24.8.14 Stable backport of #96405: Crash with old analyzer if JOIN and duplicated aliases

[zvonand]

Changelog category (leave one):

• Critical Bug Fix (crash, data loss, RBAC) or LOGICAL_ERROR

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

Fixed crash in old analyzer if JOIN and duplicated aliases (ClickHouse#96405 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)

[ilejn]

Regarding QA.
The backport is trivial, so I don't think that it worth special testing.
But the change itself is very tricky, if we want to carefully test it we should cover duplicated aliases in unusual circumstances like Merge engine, Distributed, whatever.

[altinity-robot]

This is an automated comment for commit a17f126 with description of existing statuses. It's updated for the latest CI running

❌ Click here to open a full report in a separate page

Check name	Description	Status
Sign aarch64	There's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS	❌ error
Sign release	There's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS	❌ error

Successful checks

Check name	Description	Status
AST fuzzer	Runs randomly generated queries to catch program errors. The build type is optionally given in parenthesis. If it fails, ask a maintainer for help	✅ success
Builds	There's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS	✅ success
Compatibility check	Checks that clickhouse binary runs on distributions with old libc versions. If it fails, ask a maintainer for help	✅ success
Docker keeper image	The check to build and optionally push the mentioned image to docker hub	✅ success
Docker server image	The check to build and optionally push the mentioned image to docker hub	✅ success
Grype Scan altinityinfra/clickhouse-keeper:1401-24.8.14.10519.altinitytest	There's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS	✅ success
Grype Scan altinityinfra/clickhouse-server:1401-24.8.14.10519.altinitytest-alpine	There's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS	✅ success
Grype Scan altinityinfra/clickhouse-server:1401-24.8.14.10519.altinitytest	There's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS	✅ success
Install packages	Checks that the built packages are installable in a clear environment	✅ success
Integration tests	The integration tests report. In parenthesis the package type is given, and in square brackets are the optional part/total tests	✅ success
Ready for release	There's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS	✅ success
Stateful tests	Runs stateful functional tests for ClickHouse binaries built in various configurations -- release, debug, with sanitizers, etc	✅ success
Stateless tests	Runs stateless functional tests for ClickHouse binaries built in various configurations -- release, debug, with sanitizers, etc	✅ success
Stress test	Runs stateless functional tests concurrently from several clients to detect concurrency-related errors	✅ success

[zvonand]

All tests are green. This PR also has a test attached.

[alsugiliazova]

Audit: PR #1401

Scope: Altinity/ClickHouse#1401.

Summary: Fixes a crash in the old (non-analyzer) path when a query has JOINs and duplicated column aliases in a subquery’s SELECT list (e.g. SELECT g, 42::Int32 AS g FROM ...). The fix runs an early normalize and renameDuplicatedColumns in TreeRewriter::analyzeSelect when remove_duplicates is true, so duplicate output names are resolved before JOIN analysis and qualified-name translation.

---

Confirmed defects

No confirmed defects in reviewed scope.

The change is minimal and correctly addresses the root cause: when subqueries are analyzed with remove_duplicates (e.g. via interpretSubquery() → removeDuplicates()), the early block ensures that the current SELECT’s expression list has unique names before translateQualifiedNames, JOIN rewriting, and downstream code that assume unique column names (e.g. in JoinToSubqueryTransformVisitor and table/column resolution) run. No new C++ lifetime, concurrency, or logic issues were found in the added code.

---

Coverage summary

• Scope reviewed: TreeRewriter::analyzeSelect early block when remove_duplicates is true (local normalize + renameDuplicatedColumns), renameDuplicatedColumns usage, and test 03903_join_alias_dups (both enable_analyzer=0 and 1).
• Categories passed: Call graph (entrypoint → normalize → renameDuplicatedColumns → rest of analyzeSelect); invariant “select list has unique names before JOIN/translation” preserved by the new block; logical branches (remove_duplicates true/false, with/without duplicates); fault category “duplicated aliases in JOIN subquery” addressed by the fix; C++ bug-type check (no use-after-move, no new races; single-threaded analysis path); rollback/partial-update N/A (no persistent state); test coverage for both analyzers and two query shapes with duplicated aliases.
• Categories failed: None.
• Assumptions/limits: Audit is static (no runtime execution); reliance on existing normalize() and renameDuplicatedColumns() behavior; “old analyzer” crash path inferred from code flow and test design, not from a reproduced crash.

[alsugiliazova]

PR #1401 CI Verification Report

CI Results

Analyzed commit: a17f1262d27c7a1b0b499fc4aaf2e5fe4b4cdcf8 (PR head commit)
Workflow run: ReleaseBranchCI #21987673096

Overall: 155 success, 1 failure, 10 skipped (166 total jobs)

Failure (resolved on re-run)

#	Job	Error	Re-run Result	Related to PR
1	IntegrationTestAarch64 / Integration tests (aarch64)-4	Process exit code 1	Passed on re-run	No

The single failure was a transient infrastructure issue that passed on re-run the same day. The failed run started at 13:58 and the successful re-run started at 16:22.

PR's Own Stateless Test

The new test 03903_join_alias_dups passed on all platforms:

• FunctionalStatelessTestRelease / Stateless tests (release) — Success
• FunctionalStatelessTestAarch64 / Stateless tests (aarch64) — Success
• FunctionalStatelessTestDebug / Stateless tests (debug) — Success

The test verifies that queries with JOIN and duplicated aliases work correctly with both enable_analyzer = 0 (old analyzer) and enable_analyzer = 1 (new analyzer).

Stateful Tests

All passed:

• FunctionalStatefulTestRelease — Success
• FunctionalStatefulTestAarch64 — Success
• FunctionalStatefulTestDebug — Success

Integration Tests

All passed (after re-run of aarch64 shard 4):

• IntegrationTestsRelease (4 shards) — All success
• IntegrationTestAarch64 (6 shards) — All success (after re-run)

Regression Tests

Regression tests were excluded by the PR author (ci_exclude_regression was checked).
Regression run in clickhouse-regression repository

• with old analyzer https://github.com/Altinity/clickhouse-regression/actions/runs/23172055646
• with new analyzer https://github.com/Altinity/clickhouse-regression/actions/runs/23172109024

clickhouse-regression

No dedicated clickhouse-regression test exists for this specific JOIN alias deduplication fix.

Conclusion

PR #1401 is verified.