Workflow [PR], commit [2c546ca]

Issue related to failed test in CI run with PR - #80440

Workflow [PR], commit [bfa8ad2]

Summary: ✅

• Performance Comparison: Performance dashboard

AI Review

Summary

The PR adds a regression test for #48881 and now also fixes recursive duplicate projection alias stripping in queryNodeToDistributedSelectQuery, so nested subqueries sent to remote replicas no longer trip MULTIPLE_EXPRESSIONS_FOR_ALIAS. I did not find a code-level blocker in the current diff, but the PR metadata is stale now that the PR contains a user-visible bug fix.

PR Metadata

• The current Changelog category is Not for changelog, but the PR now changes user-visible behavior by fixing a MULTIPLE_EXPRESSIONS_FOR_ALIAS exception under parallel replicas.
• Suggested replacement:

### Changelog category (leave one):
- Bug Fix

### Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Fix `MULTIPLE_EXPRESSIONS_FOR_ALIAS` for parallel-replica queries with duplicate projection aliases inside nested subqueries.

Findings

💡 Nits

• [src/Planner/Utils.cpp:280] The recursive alias-deduplication call makes this PR a real bug fix rather than a test-only change, so the release-note metadata should be updated as shown above. Posted an inline comment on this line.

Final Verdict

✅ No code-level blockers found. Please fix the PR metadata before merge.

@devcrafter, a problem with parallel replicas.

Dear @alexey-milovidov, this PR hasn't been updated for a while. Will you continue working on it? If not, please close it. Otherwise, ignore this message.

Dear @alexey-milovidov, this PR hasn't been updated for a while. Will you continue working on it? If not, please close it. Otherwise, ignore this message.

The Stress test (arm_msan) failure is fixed by #101239, which should be merged first. After it is merged, please update the branch to include the fix.

@groeneai, the Stress test (amd_asan_ubsan) failure on b466ac19 is the Hung check on SYSTEM SYNC MERGES t_manual; from 04207_schedule_merge_basic.sql. That test was added in master commit aa5966d99cd and is unrelated to this PR's changes (which only touch tests/queries/0_stateless/03517_wrong_primary_key_analysis.{sql,reference}).

Log: https://s3.amazonaws.com/clickhouse-test-reports/PRs/80310/b466ac191f60efc6ced748be0f2ec8fedc19b661/stress_test_amd_asan_ubsan/hung_check.log

Could you investigate the failure and provide a fix in a separate PR? If a fix is already in progress, please link it here.

Updated the branch onto current `master` (was 574 commits behind), which now includes the merged #103806.

I verified the parallel-replicas situation empirically on a fresh build at `3c6ba95806e` (includes #103806), running the test queries against `test_cluster_one_shard_three_replicas_localhost` with `enable_parallel_replicas = 1, max_parallel_replicas = 3`:

• The self-shadowing alias shape in this test — `SELECT *, day + 365 AS day` over `SELECT *` — still reproduces `MULTIPLE_EXPRESSIONS_FOR_ALIAS` from the remote replica even after Fix MULTIPLE_EXPRESSIONS_FOR_ALIAS with enabled parallel replicas #103806:

  Code: 179. Multiple expressions __table3.day + 365 AS day and __table3.day AS day for alias day ... (MULTIPLE_EXPRESSIONS_FOR_ALIAS)

• With the `SET enable_parallel_replicas = 0` guard, the full test passes deterministically (4× `107`, matching the reference).

So #103806 covered the `PREWHERE`-alias and `SELECT *`-over-JOIN shapes, but not this self-shadowing alias over `SELECT *`. The explicit guard is therefore still required, and the comment in the test is accurate. Note that #74324 is currently closed as completed by #103806, yet this shape still reproduces — it may warrant reopening or a follow-up issue.

The only CI failure is the chronic `Assertion 'px != 0' failed` terminate in `QueryPlanResourceHolder.cpp` (STID 3685-4830), triggered by an unrelated fuzzer query (`SELECT DISTINCT 1025, toFixedString(...)`) and tracked under #106079 — unrelated to this test-only change.

Updated the branch onto current master (was 576 commits behind), clean merge with no conflicts. The diff remains test-only (03517_wrong_primary_key_analysis.{sql,reference}).

Re-verified the parallel-replicas situation on a fresh master build (rev 0ea13fba9e7, post-#103806), running the test queries against a three-replica cluster with enable_parallel_replicas = 1, max_parallel_replicas = 3:

• The self-shadowing alias shape — SELECT *, day + 365 AS day over SELECT * — still reproduces MULTIPLE_EXPRESSIONS_FOR_ALIAS from the remote replica:

  Code: 179. Multiple expressions __table3.day + 365 AS day and __table3.day AS day for alias day ... (MULTIPLE_EXPRESSIONS_FOR_ALIAS)

• With the SET enable_parallel_replicas = 0 guard, the full test passes deterministically (4× 107, matching the reference) both via clickhouse-local and against a real server.

So the explicit guard is still required and the comment in the test remains accurate. #74324 is still effectively open for this self-shadowing shape — #103806 covered the PREWHERE-alias and SELECT *-over-JOIN cases but not this one.

Fixed the parallel-replicas bug so the test runs with parallel replicas (the explicit enable_parallel_replicas = 0 guard is removed).

Root cause. The self-shadowing alias shape SELECT *, day + 365 AS day produces a projection __table3.day AS day, __table3.day + 365 AS day — the same alias day bound to two different bodies. When the analyzed query tree is converted back to AST and dispatched to a remote replica, the receiver re-analyzes it and throws MULTIPLE_EXPRESSIONS_FOR_ALIAS (#74324). queryNodeToDistributedSelectQuery already deduplicated such aliases — stripping the later, different-body occurrence — but only on the top-level SELECT. In SELECT count() FROM (... SELECT *, day + 365 AS day ...) the conflict lives in a nested subquery, which the dedup never reached, so #103806 left this shape failing.

Fix (f8312827231): apply the dedup recursively to every SELECT in the dispatched AST. Each subquery is its own alias scope and the whole AST is re-analyzed on the receiver, so the conflict can arise at any nesting level. Same-alias-same-body duplicates are still preserved (only different bodies are stripped) to avoid THERE_IS_NO_COLUMN, and an outer reference to the alias binds to the first occurrence regardless, so dropping the later alias is safe.

Verified locally against a three-replica cluster (default_parallel_replicas) with enable_parallel_replicas = 1, max_parallel_replicas = 10, parallel_replicas_for_non_replicated_merge_tree = 1:

• 03517_wrong_primary_key_analysis passes 4× 107 with parallel replicas;
• the original MULTIPLE_EXPRESSIONS_FOR_ALIAS with enabled Parallel Replicas #74324 repro no longer throws;
• 04145_multiple_expressions_for_alias_parallel_replicas and 03121_analyzer_filed_redefenition_in_subquery still match their references.

This effectively completes #74324 for the self-shadowing-alias-over-SELECT * shape that #103806 did not cover.

Also updated the branch onto current master (was 464 commits behind).

@groeneai, the Stateless tests (amd_debug, parallel) failures (Server died and Logical error: Virtual row boundary violated in MergingSortedAlgorithm ... (STID: 2651-3359)) are unrelated to this test-only/serialization change: the checkVirtualRowBoundary debug assertion was added in master commit 77cf854a3e2 and is currently firing across master (it appears on pull_request_number = 0 and ~30 other PRs in CIDB over the last two days). Could you investigate and provide a fix in a separate PR? If one is already in progress, please link it here.

@alexey-milovidov yes, fix is in flight: #106664.

STID 2651-3359 (Virtual row boundary violated in MergingSortedAlgorithm) is chronic across master and ~30 unrelated PRs since the checkVirtualRowBoundary debug assertion was added in commit 77cf854, matching your analysis. The Server died line in the same Stateless tests (amd_debug, parallel) is the downstream cascade of the same LOGICAL_ERROR debug abort (verified across PR #105386, PR #93462, and master).

Root cause: when apply_virtual_row_conversions = false (Union of read-in-order subtrees with per-source conversions, e.g. Distributed two-shards with PK alias columns), setVirtualRow skips the per-source rename, the pk_block columns no longer match the outer merge header, and the recorded boundary in rememberVirtualRowBoundary is a default Decimal64_'0.000000000' that the assertion trips on the first real row. Release builds were already correct because mergeImpl/mergeBatchImpl discard the virtual row via skip_last_row=true; only debug builds aborted.

Fix in #106664: gate rememberVirtualRowBoundary and checkVirtualRowBoundary on apply_virtual_row_conversions = true in MergingSortedAlgorithm.cpp. Regression test 04323_virtual_row_two_shards_pk_alias.sql aborts on master and passes with the fix.

Session: cron:clickhouse-ci-task-worker:20260607-080600

LLVM Coverage Report

Changed lines: Changed C/C++ lines covered by tests: 23/25 (92.00%) | Lost baseline coverage: none · Uncovered code

Full report · Diff report