Type IN (subquery) over LowCardinality consistently in the old analyzer

[fm4v]

Related: #114229
Related: #100226

Changelog category (leave one):

• Bug Fix (user-visible misbehavior in an official stable release)

Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):

Fixed LOGICAL_ERROR: Unexpected return type from tuple. Expected Tuple(..., UInt8). Got Tuple(..., LowCardinality(UInt8)) when a LowCardinality column compared with IN (subquery) is read through tuple(...) across a subquery boundary and enable_analyzer = 0. With the old analyzer, x IN (subquery) over a LowCardinality column now returns LowCardinality(UInt8), the same type as x IN (literal list) and the same as with enable_analyzer = 1.

Description

The old-analyzer half of #114229, whose test is tagged no-old-analyzer.

Reproducer, broken since 26.6 and still broken on master:

CREATE TABLE t (s LowCardinality(String)) ENGINE = MergeTree ORDER BY s;
INSERT INTO t VALUES ('a');
SET enable_analyzer = 0;
SELECT tuple(*) FROM (SELECT s, s IN (SELECT 'a') AS f FROM t);
-- Code: 49. Unexpected return type from tuple. Expected Tuple(LowCardinality(String), UInt8).
--           Got Tuple(LowCardinality(String), LowCardinality(UInt8)). (LOGICAL_ERROR)

Root cause. The general typing rule wraps a result in LowCardinality only when at most one argument is a full LowCardinality column and no argument is a full ordinary column (IFunctionOverloadResolver::getReturnType, src/Functions/IFunction.cpp:797). The two passes over the same expression disagree:

• during execution the set is a constant column (ColumnSet is always wrapped in ColumnConst since Enforce invariant that ActionsDAG::Node::column is always ColumnConst #100226) and a Set-typed argument counts as constant since Type IN (subquery) over LowCardinality the same as IN (literal list) #114229, so in returns LowCardinality(UInt8);
• during analysis the set from a subquery is not built yet, so in is replaced with inIgnoreSet and the left operand is passed twice as the stand-in for the missing set (src/Interpreters/ActionsVisitor.cpp). Two full LowCardinality arguments fail the num_full_low_cardinality_columns <= 1 condition, so the result types as plain UInt8.

The outer query records UInt8 for that column and the pipeline delivers LowCardinality(UInt8), so columnMatchesType rejects the tuple result in executeActionForPartialResult during ActionsDAG::updateHeader. DESCRIBE (subquery) and EXPLAIN header = 1 disagreeing on the type is the quick diagnostic. In a release build this is a query error; in debug and sanitizer builds it aborts the server.

Change. The stand-in for the missing set is a constant UInt8 instead of the left operand. It is indistinguishable from the real Set argument in every dimension the typing rule inspects — it is constant, so it is skipped by both LowCardinality counters; it is not Nullable, so it does not affect the null wrapper; and FunctionIn::getReturnTypeImpl only reads arguments[0]. Analysis and execution therefore agree by construction, for all eight in / notIn / nullIn / notNullIn × global variants and for every operand shape, not only the one in the reproducer. The IgnoreSet variants never look at their second argument.

A Set-typed stand-in would mirror execution more literally, but getRootActionsNoMakeSet feeds window-definition expressions into an executed plan (src/Interpreters/ExpressionAnalyzer.cpp:1562), which can be serialized for distributed execution, and a ColumnSet with no set behind it would be dereferenced there. A UInt8 constant cannot be.

Visible type change, mirroring what #114229 did for the analyzer: with enable_analyzer = 0, toTypeName(x IN (subquery)), DESCRIBE of such a subquery, EXPLAIN headers, and column types of CREATE ... AS SELECT over such an expression now show LowCardinality(UInt8) instead of UInt8. Values do not change.

Validation. New stateless test 04894_in_subquery_lowcardinality_type_old_analyzer asserts, for both analyzers, that the literal-set and subquery-set forms have the same type, that the type recorded for the subquery column is LowCardinality(UInt8), and that tuple(*) over that column across a subquery boundary works, with IN / NOT IN count() controls. Its first two arms fail and the tuple(*) arm aborts a debug server without the change.

---

Workflow [PR]
Sync PR [sync-upstream/pr/114856]

Version info

• Merged into: 26.8.1.1958 (included in 26.8 and later)

[clickhouse-gh]

Workflow [PR], commit [4d1214f]

Summary: ✅

• Performance Comparison: Performance dashboard

---

AI Review

Summary

This PR fixes the old-analyzer LowCardinality type mismatch for IN (subquery) across subquery boundaries by routing in*IgnoreSet through a one-argument placeholder, and it adds good regression coverage for the original reproducer plus the lambda capture case. The reproducer itself looks covered, but the same placeholder still flows into an executed window-expression chain, so enable_analyzer = 0 window keys containing IN (subquery) remain incorrect.

Findings

⚠️ Majors

• [src/Interpreters/ActionsVisitor.cpp:1331] appendWindowFunctionsArguments() builds executable pre-window actions with getRootActionsNoMakeSet(), and WindowStep then consumes those produced columns as the actual PARTITION BY / ORDER BY keys. FunctionIn::executeImpl still returns an all-zero column for every *IgnoreSet call, so row_number() OVER (PARTITION BY s IN (SELECT 'a') ORDER BY s) under enable_analyzer = 0 still computes the wrong partition key instead of the real IN result. The new one-argument placeholder fixes the header/type mismatch, but it leaves this executed path semantically wrong.

Final Verdict

⚠️ Needs changes before merge. The old-analyzer window-function path still executes IN (subquery) placeholders as zero-valued keys, so this fix does not yet cover all executed no_makeset consumers of the invariant it changes.

LLVM Coverage Report

Metric	Baseline	Current	Δ
Lines	87.10%	87.10%	+0.00%
Functions	91.90%	91.90%	+0.00%
Branches	79.40%	79.40%	+0.00%

Changed lines: Changed C/C++ lines covered: 12/12 (100.00%) · Uncovered code

Full report · Diff report

[clickhouse-gh]

Build profile diff (arm_release)

Comparing 4d1214fe5 with master c583f98a0 (stripped binary size, per-symbol sizes and ThinLTO time; compile times per translation unit against the most recent warmup build that recompiled it).

✅ No significant changes.

Binary sizes

Binary	Master	PR	Δ
programs/clickhouse-stripped	705.78 MiB	702.75 MiB	-3.03 MiB (-0.43%)

Only the stripped binary is compared: the official master build keeps debug symbols while PR builds strip them, so the other binaries differ by construction.

Compile time of recompiled translation units

8 translation units recompiled, 15 s compile time in total, 8 of them have a recent master baseline.

Job report

[clickhouse-gh]

appendWindowFunctionsArguments() executes the no_makeset DAG before WindowStep, so this in*IgnoreSet node is not limited to type inference. FunctionIn::executeImpl still returns an all-zero UInt8 column whenever ignore_set is true (src/Functions/in.cpp:101-102), which means enable_analyzer = 0 queries such as SELECT row_number() OVER (PARTITION BY s IN (SELECT 'a') ORDER BY s) ... still collapse every row into the same partition instead of evaluating the real IN key. The one-argument placeholder fixes the LowCardinality type mismatch, but it does not make the executed window path semantically correct. This path needs either a real key computation or a way to keep IgnoreSet out of executable window-expression DAGs.