25.3.14 Stable backport of #89367: Crash in IN function where columns have different types and many columns are involved

[zvonand]

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):

Possible crash/undefined behavior in IN function where primary key column types are different from IN function right side column types (ClickHouse#89367 by @ilejn)

CI/CD Options

Exclude tests:

• Fast test
• Integration Tests
• Stateless tests
• Stateful tests
• Performance tests
• All with ASAN
• All with TSAN
• All with MSAN
• All with UBSAN
• All with Coverage
• All with Aarch64
• All Regression
• Disable CI Cache

Regression jobs to run:

• Fast suites (mostly <1h)
• Aggregate Functions (2h)
• Alter (1.5h)
• Benchmark (30m)
• ClickHouse Keeper (1h)
• Iceberg (2h)
• LDAP (1h)
• Parquet (1.5h)
• RBAC (1.5h)
• SSL Server (1h)
• S3 (2h)
• Tiered Storage (2h)

[CarlosFelipeOR]

AI audit note: This review comment was generated by AI (gpt-5.3-codex).

Audit update for PR #1423 (Crash/UB in IN with different types and many columns):

Confirmed defects:

No confirmed defects in reviewed scope.

Coverage summary:

• Scope reviewed: KeyCondition::tryPrepareSetIndex set-element conversion/filtering path in src/Storages/MergeTree/KeyCondition.cpp, debug invariant check addition in src/Interpreters/sortBlock.cpp, and new regression coverage in tests/queries/0_stateless/03635_in_function_different_types_many_columns.{sql,reference}.
• Categories failed: None.
• Categories passed: call-graph and transition checks for IN set preparation, branch analysis for safe-cast vs accurate-or-null cast paths, null-map/filter intersection correctness across multi-column set tuples, C++ bug-class checks (lifetime/iterator/UB/signedness/RAII) in changed scope, rollback/partial-update consistency, and concurrency/locking checks (no new shared-state/locking paths introduced).
• Assumptions/limits: Static audit only (no runtime re-execution in this run); residual risk is mainly in adjacent Set construction/type-normalization paths not modified by this PR.

[CarlosFelipeOR]

QA Verification

Functional Validation

Bug was reproduced in debug build of releases/25.3.14 (without this fix, using PR #1421's debug build at commit d0db2c77) and validated as fixed in the debug build of this PR.

• Reproduced the exact same crash reported in upstream issue: SIGABRT in sortBlock → MergeTreeSetIndex::MergeTreeSetIndex → pdqsort → ColumnVector::less → PODArray::operator[] (out-of-bounds). Stack trace matches frame-by-frame the one reported in upstream PR Crash in IN function where columns have different types and many columns are involved ClickHouse/ClickHouse#89367
• After the fix, the same query (multi-column IN with mixed types where some values are not convertible to PK type) returns a count without crashing
• The crash only manifests on debug builds because release builds wrap the comparison in DebugLessComparator only under NDEBUG undefined; release builds have silent UB instead of SIGSEGV. The new assertion block.checkNumberOfRows() in sortBlock.cpp provides defense in depth

Stateless Test Issue (blocker)

The new test 03635_in_function_different_types_many_columns added by this PR fails in all 3 stateless jobs where it runs (aarch64, debug [2/2], release) due to a .reference mismatch — the reference file contains the upstream master output (Keys: id, value, Condition: ((id, value) in N-element set), with Search Algorithm and Ranges lines) but 25.3.14 produces a different output (Keys: value only, Condition: (value in N-element set), without Search Algorithm/Ranges).

The same issue was already addressed in the 25.8.12 backport (#1197) via commit 235d29d ("fix stateless test"), where the .reference was updated to match the actual output of the release branch. A similar fix is needed here, with the additional adjustment that 25.3.x does not emit Search Algorithm and Ranges lines (those were added in later versions).

CI Failure Analysis

All other CI failures are pre-existing on the releases/25.3.14 branch, confirmed by cross-referencing with PR #1421 (analyzed previously).

Category	Tests	Root Cause
PR-caused (test reference)	03635_in_function_different_types_many_columns (3 stateless jobs)	.reference needs update for 25.3.14 output (see above)
Flaky upstream	Stateless tests (release): 01455_opentelemetry_distributed	Known upstream flaky test (ClickHouse#93452, ClickHouse#67108); IO race in IOUringReader::monitorRing(), unrelated to this PR
Pre-existing flaky	Integration [3/8] test_partition_move_drop_race	Same failure on PR #1421
Pre-existing (iceberg)	Regression iceberg (x86 + aarch64): 273 errored	Identical pattern across all PRs on this branch
Infra (docker-compose startup)	Regression aarch64: Alter attach, Benchmark minio, LDAP role_mapping, S3 azure, Tiered Storage s3gcs	"could not bring up docker-compose cluster" — same category as PR #1421 (specific suites rotate per run)
Branch infra	Sign aarch64/release, GrypeScan (alpine vulns)	Consistent across all PRs on this branch

Verdict

Functionally validated — fix correctly resolves the reported crash. Blocker: stateless test .reference needs to be updated for 25.3.14 output before this PR can merge.

[zvonand]

@blau-ai

[blau-ai]

CI Triage — PR #1423

Summary: 2 PR-caused failures, rest are infra/pre-existing

Category	Checks
PR-caused (need fixing)	03635 stateless test on aarch64 (reference mismatch)
DCO (administrative)	Commit sign-off missing
Infra / pre-existing	Docker Hub rate limiting, GrypeScan botocore error, Sign error, version regression branding checks, integration test flake, debug test SIGSYS

---

❌ PR-caused: 03635_in_function_different_types_many_columns fails on aarch64

Checks affected: FunctionalStatelessTestAarch64 / Stateless tests (aarch64) (fail), Stateless tests (aarch64) (fail: 1)

The test added by this PR fails on aarch64 with a reference mismatch. Commit 2340f8575d1 updated the reference for x86_64 behavior but aarch64 still diverges on queries 3 and 4:

 # Query 3: SELECT 42, 'not a number' UNION ALL SELECT 5, toString(number) FROM numbers(5)
-          Condition: (value in 6-element set)   ← reference (expected)
+          Condition: (value in 5-element set)   ← aarch64 actual

 # Query 4: SELECT '42', 'not a number' UNION ALL SELECT 'not a number', '42' FROM numbers(5)
-          Condition: (value in 1-element set)   ← reference (expected)
+          Condition: (value in 0-element set)   ← aarch64 actual

The aarch64 values (5 and 0) are semantically correct: 'not a number' cannot be cast to UInt64, so that row should be filtered out, leaving 5 valid elements for Q3 and 0 for Q4. The reference claiming 6 and 1 likely came from generating it on x86_64 where the code path differs.

Note: FunctionalStatelessTestRelease / Stateless tests (release) has conclusion: success (GitHub Actions job passed), so x86_64 release is not blocked. The Stateless tests (release) fail: 1 artifact entry is from a different (likely known-broken) test.

Fix: Update tests/queries/0_stateless/03635_in_function_different_types_many_columns.reference to expect (value in 5-element set) and (value in 0-element set) for queries 3 and 4. These are the semantically correct values (row with 'not a number' is properly filtered). If x86_64 still produces 6 and 1 after the reference change, that suggests the fix in KeyCondition.cpp has a platform-specific behavior difference worth investigating — but the aarch64 output appears correct.

---

⚠️ Administrative: DCO check failing

The commit 2340f8575d1 ("fix stateless test for 25.3.14") is missing a Signed-off-by: line. Add Signed-off-by: Your Name <email> to that commit message.

---

✅ Pre-existing / infra (no action needed on this PR)

Regression version tests — Regression aarch64 version and Regression release version:

• Failing scenarios: /version/altinity/no clickhouse cloud url in auth error and /version/altinity/no clickhouse cloud url in binary
• These Altinity branding checks are not related to this PR's changes (KeyCondition.cpp / IN function). Pre-existing on this branch.

RegressionTestsRelease/Aarch64 Common (aggregate_functions) GitHub Actions jobs (4h+):

• Failure root cause: Docker Hub rate limiting at startup (Error response from daemon: toomanyrequests: too many failed login attempts)
• Tests themselves ran successfully for hours; infra issue, not code issue.

GrypeScan (server):

• ImportError: cannot import name 'DocumentModifiedShape' from 'botocore.docs.utils'
• Broken Python environment on the runner; infra issue.

Sign aarch64 / Sign release:

• Error: Unknown, exit_code 0 — CI infra metadata error, not code-related.

Regression aarch64 Benchmark gcs:

• Job did not complete — timeout/infra.

FunctionalStatelessTestDebug / Stateless tests (debug)-1 and Stateless tests (debug) [2/2] "Invalid check_status.tsv":

• Debug test runner exited with code 159 (signal 31, SIGSYS / seccomp violation). The test results file was never written. This is a runner seccomp policy / infra issue, not caused by the PR's checkNumberOfRows() addition (that is #ifndef NDEBUG only and wouldn't trigger SIGSYS).

Integration tests (release) [5/8] — 1 failure out of 412:

• Single failure in batch 5 of 8. Likely flaky — this PR only touches KeyCondition.cpp, sortBlock.cpp, and test files, none of which affect integration test infrastructure.

---

Next step

Push a commit that updates the aarch64 reference values for queries 3 and 4 in tests/queries/0_stateless/03635_in_function_different_types_many_columns.reference (change 6-element → 5-element and 1-element → 0-element). The x86_64 GitHub Actions job passing with conclusion: success means the current reference (6, 1) is correct for x86_64 too, so you may need platform-specific handling — or simply verify by re-running after the change. If x86_64 also accepts 5 and 0, a single reference update fixes both.

[zvonand]

@blau-ai

[blau-ai]

CI triage

Verdict: 5 failing checks — 1 is PR-related (DCO sign-off, a process fix), 4 are not (1 flaky test, 1 image-CVE pair, 1 stress-test job I couldn't read yet but that is almost certainly infra). No code defect in this backport is visible in CI. The change is the upstream ClickHouse#89367 fix verbatim (sortBlock.cpp + KeyCondition.cpp + the new 03635_... test), and that new test passed.

---

1. DCO — ❗ PR-related (process, not code)

State: ACTION_REQUIRED. The DCO app flags two non-merge commits in this branch that have no Signed-off-by trailer:

• 2340f857 — fix stateless test for 25.3.14
• 161a4db7 — Fix reference for 03635_in_function_different_types_many_columns

Both are authored by @zvonand. Fix: sign those two commits off and update the branch, e.g.

git checkout backports/releases/25.3.14/89367
git rebase releases/25.3.14 --rebase-merges \
  --exec 'git commit --amend --no-edit --signoff'
git push --force-with-lease

(or amend just those two commits with --signoff). I can't do this myself — it requires a force-push/history rewrite, which is outside my permissions. Alternatively an org admin can waive the DCO check for this release backport.

2. Stateless tests (release) — ✅ not PR-related (flaky)

fail: 1, passed: 7904. The single failure is 02922_deduplication_with_zero_copy:

Reason: having stderror:
Can't get data for node '/clickhouse/zero_copy/zero_copy_s3/.../all_0_0_0/...': node doesn't exist

This is the well-known zero-copy-replication/S3 dedup flake. This PR touches only the IN-function index path (KeyCondition/sortBlock) — nothing in replication, zero-copy, or S3 — and the PR's own new test 03635_in_function_different_types_many_columns passed. Safe to re-run.

3. StressTestDebug / Stress test (debug) — ⚠️ undetermined, almost certainly infra (not PR)

I could not read this job's log — the workflow run (35332899315) is still in progress and the stress report hasn't been uploaded to S3 yet. Indirect evidence that it is not the PR:

• The PR's only debug-build change is the assertion #ifndef NDEBUG block.checkNumberOfRows(); in sortBlock. That assertion runs in every debug build — yet Stateless tests (debug) (both shards) and all Integration test shards passed clean, so it is not misfiring.
• The job failed at ~26 min, far short of a normal debug stress run (~1.5–2 h), which is the signature of an early infra/setup failure rather than a query-driven crash.

Recommend re-running once the run finishes; ping me and I'll read the actual log then.

4 & 5. Grype Scan (keeper) and (server, -alpine) — ✅ not PR-related (base-image CVEs)

Each reports 1 high/critical vulnerability in the container's OS/base-image packages. The non-alpine clickhouse-server scan passed with 0. This PR changes only C++ source and one SQL test — it cannot introduce base-image package CVEs. These are dependency/image issues handled at the image/release level, not in this backport.

---

Bottom line: the only thing blocking that is actually about this PR is the missing DCO sign-off on 2340f857 and 161a4db7 — fix that with a signed-off force-push (or a DCO waiver). The stateless failure is a known zero-copy flake, the Grype findings are base-image CVEs, and the debug stress job needs a re-run (I'll re-check its log once CI finishes if you want).