Repository navigation
Conversation
Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
- switch PlannerJoinTree header conversion from position to name matching when column names are unique and sets match - add stateless regression 03922 for alias-marker column swap across distributed reads Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
This reverts commit b6cbb20. Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Add a planner utility that first tries header conversion by column name and falls back to position when name matching is not possible.\n\nUse it in PlannerJoinTree and parallel replicas query planning to handle duplicate alias-heavy schemas more robustly, while emitting trace diagnostics when position fallback is used.\n\nUpdate 03921 distributed-over-distributed alias tests and pin parallel-replica settings for deterministic results. Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Nested ALIAS columns (a2 contains a1's subexpression) over a single-shard Distributed table, matrix over prefer_localhost_replica x serialize_query_plan, with the single-node result as oracle. Already green on this branch (the PlannerJoinTree name-first conversion from b0ce0c9 covers it) - kept as a regression lock. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
…licasQuery) Nested ALIAS columns over a Distributed table read with parallel replicas, AST and serialized-plan transport, single-node result as oracle. Green on this branch (findParallelReplicasQuery name-first conversion covers it) - kept as a regression lock. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Rewrites 03928 as a black-box test: nested ALIAS columns read through a Merge over a Distributed table, no explicit __aliasMarker, single-node result as oracle. Currently FAILS - alias columns come back as 0 through StorageMerge reconciliation. Probe .stderr/.stdout artifacts dropped. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
…r names When a Merge table reads an underlying Distributed table with ALIAS columns, the alias expressions in convertAndFilterSourceStream were emitted under their plain logical name (e.g. `a`), while the Merge reconciliation expects analyzer column identifiers (e.g. `__table1.a`). The identifier columns were therefore treated as missing and filled with defaults, yielding wrong results (zeros). Map each alias's plain logical name to the unambiguous target-header identifier and emit the alias output under that identifier, so the downstream header reconciliation matches by name with no positional fallback. Makes the header correct by construction; any extra expression columns from the child are simply dropped by name matching. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Reorders two nested ALIAS columns and adds a computed expression over them, read through a Distributed table (AST and serialized-plan transport), with the single-node result as oracle. Passes via name-first header reconciliation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Now that alias outputs are emitted under their target identifier, the child header matches the Merge header by full name, so the short-name lookup helpers (get_short_name, short_name_count, the short-name branch in find_header_column) and the unreachable duplicate header.has() branch are dead. Revert convertAndFilterSourceStream to the upstream name -> smallest -> position -> unneeded shape. The position fallback in the converting step is kept (load-bearing for normal distributed aggregation). Verified: 03928/03930/03931/03932/03921/03842/03843/03923 + the Merge regression set (type cast, virtual columns, alias-merge, structure unification, sample factor) all pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
…3926) These tests pinned enable_alias_marker=0 yet asserted non-swapped output. The __aliasMarker is the mechanism that prevents the column swap; with it disabled the swap is expected by design (a String alias routed to a UInt64 column -> CANNOT_PARSE_TEXT), so the references were aspirational and the tests could never pass. Marker-on coverage (03921/03928/03930/03931/03932) already validates the supported configuration. Copies kept under _local_files_and_notes/dropped_tests/ for the record. Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…changes - Planner::makeConvertingActionsPreferNameThenPosition: only fall back to positional matching for THERE_IS_NO_COLUMN / NUMBER_OF_COLUMNS_DOESNT_MATCH (the cases name matching legitimately cannot handle, e.g. a remote aggregate-state column matched by ordinal); rethrow any other name-mode error instead of silently masking a real schema/type problem. Fallback stays at TRACE (it is a common, expected path). - finalizeAliasMarkersForDistributedSerialization: do not descend into lambda bodies. A marker inside a lambda (e.g. arrayMap(x -> __aliasMarker(x, x), ...)) over a Distributed table resolved its column argument to the lambda parameter, which has no table source, and raised a user-triggerable LOGICAL_ERROR (server abort under abort_on_logical_error). Covered by 03933. - StorageDistributed: clone the alias expression before removeAlias() in the marker branch; getExpression() may return a node shared elsewhere in the tree. - Clarify enable_alias_marker setting description (it is a correctness toggle). - Remove dead/unused code: unused includes (base/hex.h, ClientInfo.h), unused serialize_query_plan extern, a stray blank line, and make logPositionConversionMismatch file-local with a TRACE-level guard. Refresh a stale comment in findParallelReplicasQuery. Tests: 03933 (direct/lambda __aliasMarker use must not crash) and 03934 (enable_alias_marker on=correct vs off=reintroduces the column swap, proving the setting's effect). Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…traction
Replace the two hand-rolled rfind('.')-and-substr blocks in convertAndFilterSourceStream
with Nested::splitName(name, /*reverse=*/true), the canonical helper the analyzer itself
uses to derive a column's logical name from its `__tableN.col` identifier. Behavior is
unchanged (fall back to the full name when there is no dot).
Note: the short-name alias-injection loop is retained - it is load-bearing for resolving
alias-of-alias expressions (e.g. `b ALIAS a + 1`); removing it breaks 03928 with
NOT_FOUND_COLUMN_IN_BLOCK, so the reviewers' "possibly redundant" hypothesis does not hold.
Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous form (SELECT a, b FROM merge ORDER BY x over a two-shard cluster) relied on the distributed merge ordering of duplicate rows, which is not guaranteed and differed on arm_binary CI (correct values, wrong row order). Rewrite as SELECT x, a, b ... GROUP BY x, a, b ORDER BY x: GROUP BY keeps x in the required columns (the ALIAS expansion needs it) and deduplicates the per-shard duplicates, and ORDER BY over the distinct x values is a total order independent of the merge. The test still fails if the alias columns are swapped or zeroed. Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Both tests target AST-path behavior; the "distributed plan" CI flavor forces serialize_query_plan=1 globally, which exposed two plan-path differences unrelated to what these tests assert: - 03934: on the serialized-plan path the header is reconciled by name without the marker, so the marker_off query does NOT swap/error there - the serverError expectation only holds on the AST path. - 03923: hybrid + IN-subquery on the plan path hits a separate header-reconciliation gap (THERE_IS_NO_COLUMN __table1.string_col), orthogonal to the unknown-table issues this test covers. Pin serialize_query_plan=0 so both run on the intended AST path under any CI flavor. Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The marker travels to shards as the __aliasMarker function in distributed SQL. If a shard does not understand it (older/forked build), set enable_alias_marker=0 on the initiator to disable injection and fall back to previous behavior. No version negotiation is performed by design; document this in the setting description. Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Parallel replicas over a small non-replicated table can read the same rows on several replicas under randomized settings, duplicating output (observed ×3 on arm CI). Switch to `SELECT x, a1, a2 ... GROUP BY x, a1, a2 ORDER BY x` to deduplicate while keeping `x` in the required columns for the ALIAS expansion. The test still fails if `a1`/`a2` are swapped or wrong. Mirrors the earlier 03928 fix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Six alias-marker regression tests dropped from the upstream port branch (alias_marker3) had their column-order scenarios already covered by upstream PR ClickHouse#94644. Preserves the original SQL and reference files (under 26.3 slot numbers) plus a dated rationale Markdown so future contributors don't reinvent the same shapes.
makeConvertingActionsPreferNameThenPosition previously tried `MatchColumnsMode::Name`
inside a try/catch and fell back to `MatchColumnsMode::Position` on
`THERE_IS_NO_COLUMN` / `NUMBER_OF_COLUMNS_DOESNT_MATCH`. That works functionally but
throws a C++ exception on every call site where Name cannot match by construction --
most notably on every `SELECT count() FROM mt_table` because `Optimized trivial count`
emits its output column under the source-table column name (e.g. `x`) while the
analyzer-expected header uses the aggregate function name (`count()`). The exception
travelled the full stack-unwind path before Position rescued the conversion.
Restore the predicate-based selector originally introduced in `b6cbb20e7f6` (reverted
by `39a235d405c` for unclear reasons -- not a defect). `canMatchByNameWithoutAmbiguity`
checks set equality of source and result column names with no duplicates in O(n) hash
work and routes to `Name` only when the rename can be done by name; otherwise routes
straight to `Position` and emits a `LOG_TEST` diagnostic.
Behaviour is identical to the prior form on every distributed-ALIAS scenario the
suite covers (Name path), and identical to upstream Position-blind behaviour on the
non-alias paths the prior form was paying try/catch for. Verified locally on:
- alias suite: 03921, 03928, 03930, 03931, 03932, 03933, 03228
- non-alias load-bearing: 00059_shard_global_in_mergetree (count over remote),
00754_distributed_optimize_skip_select_on_unused_shards,
00028, 00111, 00124, 01099, 02184, 02402
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
…alya-26.3/alias_marker_fixes
- StorageDistributed::ReplaseAliasColumnsVisitor: capture function_node.getArguments().getNodes() in a local reference before the move-assign + index-access pair. Two separate accessor calls are safe but the original code read as indexing a moved-from container. - StorageMerge::convertAndFilterSourceStream: document the suffix-vs-prefix concern in Nested::splitName(reverse=true) as a latent-only issue. Investigated 2026-05-29: the bug does not reproduce today because Nested subcolumns appear in the child stream with their dotted name verbatim, so the alias expression resolves directly without going through the suffix-stripping bridge. Comment in code points to the proper fix (stripAnalyzerTablePrefix) if reachability changes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com> (cherry picked from commit f323e68)
…ookup) `ReadFromMerge::convertAndFilterSourceStream` mapped logical alias names to analyzer identifiers via `Nested::splitName(name, /*reverse=*/true)`, which splits on the LAST dot. For backtick-quoted dotted column names (e.g. `\`n.a\`` -> analyzer identifier `__table1.\`n.a\``) this yields the wrong logical name (`a` instead of `n.a`); the alias output was then emitted under the wrong name, the downstream `addMissingDefaults` saw the expected `__tableN.\`n.a\`` column as missing and filled it with type defaults -- silent wrong data. Replace the analyzer-naming-convention regex with a schema lookup: take the candidate set from the actual declared Merge schema (via `merge_storage_snapshot->metadata->getColumns()`) and suffix-match each declared column name against the `header` / `pipe_columns` identifier names. A header column named `<prefix>.<C>` or `<prefix>.\`<C>\`` is known to correspond to Merge column `C` because `C` is in the candidate set. This is robust to dotted column names (Nested-style and backtick-quoted) and does not hardcode the `__tableN` prefix shape. The same lookup also drives the input-side bridge that exposes child inputs under their plain names so `buildQueryTree(alias.expression)` resolves references like `a` or `n.a` against `__table1.a` / `__table1.\`n.a\``-named inputs -- required for alias-of-alias resolution such as `b ALIAS a + 1`. Pass `merge_storage_snapshot->metadata->getColumns()` to the (static) helper so it has access to the Merge schema. Add a long TODO documenting the structural awkwardness of the two-step design (rewrite the child query in `getModifiedQueryInfo`, recompute alias values here) and a potential future unification via `__aliasMarker` -- noting that the marker function call disappears at plan-build time so MergeTree index analysis on predicates over ALIAS columns is unaffected. Tests: - 04281_storage_merge_over_distributed_alias: Merge over Distributed with nested ALIAS columns (`b ALIAS a + 1` where `a ALIAS x + 1`). - 04286_dotted_alias_merge_over_distributed: Merge with explicit backtick-quoted dotted column names declared as ALIAS in the underlying storage, over a two-shard Distributed cluster -- the shape that triggers `__tableN.\`n.a\`` identifiers and uncovers the suffix-vs-prefix splitName bug. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
…nostic (cherry picked from commit bfbf06fa04a07daafc115801ef81ae7deb66ff57)
The previous commit assumed analyzer identifiers for dotted Merge columns are backtick-quoted (e.g. `__tableN.\`n.a\``). The 26.3 analyzer emits them raw (`__tableN.n.a`). 04286_dotted_alias_merge_over_distributed exposed this: header matching only the backtick-quoted form missed the raw-dot identifier, the alias output was emitted under the wrong name and `addMissingDefaults` filled the expected column with type defaults (silent wrong data: `n.a` came back as 0). Try both forms in the suffix match: `<prefix>.<C>` and (only for dotted column names) `<prefix>.\`<C>\``. Either match accepts the analyzer identifier as corresponding to the Merge column `C`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
mkmkme
added a commit
that referenced
this pull request
Sep 19, 2026
Tests only, no source changes. Purpose is to establish which of the `__aliasMarker` regressions from #1844 still reproduce on `antalya-26.6`, which already carries the upstream fixes that landed after that PR was written: * ClickHouse#94644 (ALIAS column order) * ClickHouse#107675 (distributed ALIAS column order) * ClickHouse#107913 (`buildShardCollapseFanOut`) All 22 distinct test files from #1844 are staged: the 18 active ones plus the 4 unique files kept under `_local_files_and_notes/dropped_tests` there. The six dropped-test files that duplicate active ones byte-for-byte are staged once. Slot numbers from the 26.3 branch that are already taken on 26.6 are reassigned to the 05053-05063 range: 03843 -> 05053 03923 -> 05056 04286 -> 05059 03926 -> 05062 03920 -> 05054 03928 -> 05057 03924 -> 05060 03927 -> 05063 03921 -> 05055 03930 -> 05058 03925 -> 05061 `03648_alias_marker_with_mergeable_state.sh` is updated to the #1844 revision. It is a contract test for the reworked marker, not a bug reproducer, so it is expected to fail until the source change lands. Related: #1844 Related: ClickHouse#106402
mkmkme
added a commit
that referenced
this pull request
Sep 19, 2026
…ALIAS columns
A `Distributed` query that reads the same-named `ALIAS` column from two
`JOIN` sources fails on the shard:
Multiple expressions __aliasMarker(__table2.x, '__table4.foo') AS foo
and __aliasMarker(__table1.x, '__table3.foo') AS foo
for alias foo
`ReplaseAliasColumnsVisitor` stamped `setAlias(column_name)` on every marker it
created, so two markers carrying different ids both ended up named `foo` in the
same scope. It also aliased `column_node->getExpression()` in place, which is
shared between occurrences, so aliasing one occurrence changed the others.
Set the alias only where it is read. A top-level projection item keeps it so
the mergeable-state output column keeps its name, and a `JOIN USING` key side
keeps it because the shipped SQL renders the side rather than the key and
`rejectUnshippableJoinUsingKeys` reads that shape. Everywhere else -- `WHERE`,
`GROUP BY`, `ORDER BY`, `HAVING`, `JOIN ON` -- no alias is set, which is what
removes the collision. Clone the defining expression per occurrence so setting
an alias on one no longer reaches another.
The pass moves from `StorageDistributed` to `buildQueryTreeForShard` as
`inlineAliasColumns`, matching the name, file and semantics of the shared
inliner upstream introduced in
ClickHouse#107700, which fixes the same
class of failure (ClickHouse#107990)
without markers. Writing it there keeps the next version bump a merge rather
than a rewrite. The signature takes a `ContextPtr` that upstream's does not,
because marker injection needs the `enable_alias_marker` setting and function
resolution; with the setting off, the pass behaves exactly as upstream's.
Not ported here: the parallel-replicas call sites ClickHouse#107700 adds. No staged test
needs them yet.
Verified against the 20 staged regression tests from
#1844:
`05054_distributed_global_alias_marker_matrix` now passes, and the other 19
are byte-identical to the run before this change -- same error codes, same
messages. `05061` covers `enable_alias_marker = 0`, and `03844` / `05055`
cover nested marker chains; all three still pass.
Related: #1844
Related: ClickHouse#107700
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
mkmkme
added a commit
that referenced
this pull request
Sep 19, 2026
Reading an `ALIAS` column from a `Merge` table whose child is `Distributed`
fails on the initiator:
Not found column x: in block __table1.x UInt64, plus(__table1.x, 1_UInt8) UInt64
`ReadFromMerge::convertAndFilterSourceStream` recomputes alias values at the
Merge level and emitted each one under its plain logical name, while the Merge
target header names its columns with analyzer identifiers. The later
`addMissingDefaults` then found no `__tableN.<alias>` column and filled it with
type defaults. The final header reconciliation matched by position, so a child
that returned the same columns in a different order -- which a `Distributed`
child does, because it inlines alias expressions on the shard and returns them
in whatever order the shard's `ActionsDAG` produced -- silently paired values
with the wrong columns.
Emit each alias output under its analyzer identifier instead, and reconcile the
child header by name. The plain-name to identifier mapping is built by matching
each column declared on the Merge table against the suffixes of the child
header names, rather than by pattern-matching the analyzer's `__tableN.` naming
convention. Driving it from the declared schema is what makes dotted names
work: a `Nested` subcolumn arrives as ``__table1.`n.a` `` on some branches and
`__table1.n.a` on others, and splitting on the last dot mangles both.
Alias expressions name their inputs in plain logical names, so every mapped
column is also exposed under its plain name in the per-alias `ActionsDAG`. An
`ALIAS` defined over another `ALIAS` needs that to resolve.
`makeConvertingActionsPreferNameThenPosition` in `Planner/Utils` performs the
reconciliation. It matches by name only when both headers carry the same set of
names with no duplicates, which makes the rename a pure reordering, and matches
by position otherwise. The position branch is load-bearing rather than a safety
net: a source header can legitimately carry names the result never mentions,
such as the synthetic aggregate-state column behind an optimized trivial
`count()`. A `LOG_TEST` line reports which call site fell back and why.
`convertAndFilterSourceStream` takes the Merge table's `ColumnsDescription` for
this; the planner's `TableExpressionData` is not populated yet at that point,
so the mapping cannot be looked up through `PlannerContext`.
The `TODO(storage-merge-alias)` comment records why this recomputation exists
at all and what would remove it. Its two steps are internal to `StorageMerge`
and unrelated to the commits around it.
Verified against the 20 staged regression tests from
#1844:
`04281_storage_merge_over_distributed_alias`,
`05057_merge_over_distributed_alias_marker_column_swap` and
`05059_dotted_alias_merge_over_distributed` now pass, and the four remaining
failures are byte-identical to the previous run.
Related: #1844
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
mkmkme
added a commit
that referenced
this pull request
Sep 19, 2026
Reading an `ALIAS` column of a Hybrid table from inside a subquery fails on
the initiator:
Cannot find column `max(__table1.computed)` in source stream,
there are only columns: [max(__table2.computed)]
The marker id was built when the marker was injected, from the table alias the
initiator had assigned at that moment. `buildQueryTreeForShard` then runs
`createUniqueAliasesIfNecessary`, which renumbers the `__tableN` aliases of the
shard tree from 1. The header the initiator matches against is built from the
renumbered tree, but the id inside the marker is a `String` constant and keeps
the old number. The shard duly emits a column under the stale name and nothing
matches it.
Keep the id as a live `ColumnNode` from injection onward, so the analyzer
passes transform it along with the rest of the tree, and materialize it into a
`String` constant only at a serialization boundary, in
`finalizeAliasMarkersForDistributedSerialization`. There are two such
boundaries: `buildQueryTreeDistributed`, right after `buildQueryTreeForShard`,
and `executeSubqueryNode`, where a `GLOBAL IN` / `GLOBAL JOIN` subquery is
materialized and shipped as a temporary table. Finalizing in the second place
also stabilizes the tree hash that names the temporary table, which otherwise
hashes a `ColumnNode` whose identifier can still change.
The finalize pass walks bottom-up, so a nested marker chain materializes from
the inside out, and it does not descend into lambda bodies: a marker written by
hand as `arrayMap(x -> __aliasMarker(x, x), ...)` resolves to a lambda
parameter that has no table source to build an id from.
`FunctionAliasMarker` accepts any second argument now, because for most of its
life that argument is a `ColumnNode` rather than a `String`. The rejection it
replaced bought nothing: the function is a pass-through identity, so a marker a
user writes by hand is harmless, and turning it into a server-side error only
made valid SQL fail. For the same reason the id argument is no longer declared
always-constant. Both planner sites fall back to naming the node after the
expression the marker wraps when no finalized id is present, which is the name
it would have had with no marker at all.
Two deliberate differences from the original change in
#1844. `NormalizeAliasMarkerVisitor`
is kept: it flattens nested marker chains at SQL-render time, which happens
after finalization, and the nested-chain tests pass with it in place. And the
finalize pass never throws: a marker it cannot build an id for is left alone
rather than raising a `LOGICAL_ERROR` that valid user SQL can reach.
Verified against the 20 staged regression tests from
#1844:
`03842_hybrid_alias_issue_1424` and
`03933_alias_marker_direct_use_no_logical_error` now pass, nothing regressed,
and the two remaining failures are tests asserting a `serverError` that
upstream fixes no longer raise.
Related: #1844
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
mkmkme
added a commit
that referenced
this pull request
Sep 19, 2026
Three of the staged regression tests expect a `serverError` that upstream fixes stopped raising, so each was asserting a bug that no longer exists. Replace the assertion with the result the query now returns, and fix the framing that went with it. `03934` claimed `enable_alias_marker` was a correctness toggle: with the marker off, a distributed-over-distributed read of a `String` ALIAS and a `UInt64` ALIAS swapped the two columns and failed with `CANNOT_PARSE_TEXT`. Upstream fixed the column ordering, so the setting is what it was always meant to be -- a way for an initiator to stop emitting `__aliasMarker` for a mixed-version cluster whose shards do not understand it. The test now runs the same query with the marker on and off and requires both to return the same rows. `05053` expected `NUMBER_OF_COLUMNS_DOESNT_MATCH` for two ALIAS columns over the same expression with the marker off. `buildShardCollapseFanOut` from ClickHouse#107913 reconstructs the column the shard deduplicated away whether or not a marker is present, so that variant returns the same single row as the other six. `05060` expected `UNKNOWN_IDENTIFIER` from a Hybrid/MergeTree/Iceberg join under `object_storage_cluster_join_mode = 'local'`. It now returns the same rows as the `'allow'` variant beside it, and is held to them. Each reference was taken from a sibling query in the same test that must produce identical output, so a wrong reference here would mean the test was already wrong. `05060` also gained a header note: it needs the stateless S3 mock on localhost:11111, it needs a server listening on `127.0.0.3` for `test_cluster_one_shard_three_replicas_localhost`, and it needs `enable_parallel_blocks_marshalling = 0` until the `DISTINCT` over `ColumnBLOB` abort is fixed. Related: #1844 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
mkmkme
added a commit
that referenced
this pull request
Sep 19, 2026
A review of the three preceding commits turned up four things worth
changing.
`finalizeAliasMarkersForDistributedSerialization` refused to descend into
a lambda body, but the inliner has no such limit: `inlineExpression`
recurses through every child, so `arrayMap(x -> x + computed, dist)`
wrapped `computed` in a marker that nothing then materialized. Its id
stayed a `ColumnNode` and was rendered straight back into the shipped SQL
as `__table1.computed`, leaving the shard to resolve the very `ALIAS`
column the inlining exists to remove. Where the column is declared on the
`Distributed` table and not on the local one, the shard cannot.
The guard is gone. The case it was written for -- a hand-written
`__aliasMarker(x, x)` inside a lambda, whose id is the lambda parameter
-- is now handled by checking the id's column source instead. A lambda
parameter's source is the `LambdaNode`, which `createUniqueAliasesIfNecessary`
never aliases, so it is recognisable without refusing to look.
`makeConvertingActionsPreferNameThenPosition` is deleted. Its only call
site builds `converted_columns[i]` out of `current_step_columns[i]`, and
every branch of that loop either copies the source name through or --
in the one branch reachable only when the source name is absent from
`header` -- substitutes a name that cannot then appear anywhere in the
result set. So either every name matches its own position, in which case
`MatchColumnsMode::Name` and `::Position` pair the same columns, or the
two name sets differ and the helper falls back to position anyway. The
column swap in `05057` is fixed by emitting alias outputs under analyzer
identifiers, not by this. `src/Planner/Utils.{h,cpp}` are back to their
previous contents.
Finalization moved from `buildQueryTreeDistributed` into the end of
`buildQueryTreeForShard`. Four call sites build a shard tree and only one
finalized it; nothing is broken today, because only the `Distributed`
path injects markers at all, but wiring a second injection site would
have shipped markers nobody materializes. Last in the function rather
than straight after `createUniqueAliasesIfNecessary`, so a long id still
passes `ReplaceLongConstWithScalarVisitor` as a `ColumnNode` rather than
as a `String` that visitor could replace with a scalar.
The plain-name to analyzer-identifier mapping in
`ReadFromMerge::convertAndFilterSourceStream` now takes the longest
matching declared column name rather than the first. This is not a bug
fix: reintroducing the first-match behaviour leaves `05065` passing,
because an analyzer identifier backquotes a dotted name and the child
stream never emits a dotted identifier. It removes the mapping's
dependence on an invariant enforced two layers away.
Two review findings were not acted on. An assertion for an unfinalized
marker id cannot be written: nothing at the planner can tell an injected
marker that missed its boundary from one a user typed, and `03933` runs
exactly that shape on purpose. And `NormalizeAliasMarkerVisitor` does not
discard nested marker chains -- it never descends into a marker's own
arguments and collapses one only when the payload is itself a marker, so
the only id it drops names an intermediate action node. Both are recorded
as comments where a future reader will look.
Upstream ClickHouse#107700 also calls
`inlineAliasColumns` from `ClusterProxy/executeQuery.cpp` and
`findParallelReplicasQuery.cpp`, fixing `NO_SUCH_COLUMN_IN_TABLE` for an
`ALIAS` column on the shipped side of a parallel-replicas query. That bug
is real on this branch's base and predates this work. It is left alone:
this `inlineAliasColumns` injects markers when `enable_alias_marker` is
on, which is the default, so wiring those sites would put markers on two
paths that have never carried one.
New tests: `05064_alias_marker_in_lambda_over_distributed` covers the
lambda case, and `05065_shadowed_dotted_alias_merge_over_distributed`
guards the values for a `Merge` schema declaring both `b` and `` `a.b` ``.
Related: #1844
Related: ClickHouse#107700
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
mkmkme
added a commit
that referenced
this pull request
Sep 19, 2026
A `GLOBAL JOIN` over a `Distributed` table whose `ALIAS` column is not
declared on the local table failed on the shard:
Identifier '__table1.foo' cannot be resolved from table with name __table1.
In scope SELECT __table1.x AS x, __table1.foo AS foo, __table1.id AS id
FROM t_right_local AS __table1
Nothing in that query references `foo` any more -- inlining replaced it
with its defining expression. What still names it is the marker's id,
which became a live `ColumnNode` so that `createUniqueAliasesIfNecessary`
could renumber it. `CollectColumnSourceToColumnsVisitor` records every
`ColumnNode` it meets, keyed by source, keeping only the name and type,
so the id was gathered as if it were a column the query reads.
Those gathered columns become the projection of the subquery
`getSubqueryFromTableExpression` builds for the shipped temporary table,
rebuilt as bare column nodes with no expression. The alias body is lost
on the way, so the shard is asked for a column its storage does not
declare. `GLOBAL IN` on a table is unaffected -- it reads the storage's
ordinary columns instead of the gathered map.
Skip the marker's second argument when collecting. Traversal is top-down,
so the marker is always seen before the argument it records.
Reachable through `GLOBAL JOIN` and through a `CROSS JOIN` under
`find_cross_join`, and through a plain `JOIN` once
`prefer_global_in_and_join` promotes it.
`05066_global_join_alias_only_on_distributed` covers the alias in the
projection and the alias referenced only in a clause, each with the
marker on and off, plus the promoted plain `JOIN`. It fails on the
previous commit with the error above and passes here.
Related: #1844
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
mkmkme
added a commit
that referenced
this pull request
Sep 19, 2026
Both were argued from cases that cannot occur, and both sit in a path every `Merge` table with child `ALIAS` columns goes through. `use_column_identifier_as_action_node_name` goes back to `false`. It was set to `true` so that an `ALIAS` defined over another one would resolve against the child stream rather than be expanded again, because `false` makes `PlannerActionsVisitor` replace a column node that has an expression with a visit of that expression. No such column node reaches it. `ApplyAliasColumnExpressionsVisitor` runs over each alias before it is recorded, replacing every column node that has an expression with the expression itself, and the visitor descends into what it substituted -- so `b ALIAS a + 1` over `a ALIAS x + 1` is already flattened to physical columns by the time `convertAndFilterSourceStream` re-analyzes it. The naming half of the setting is inert too: no column identifiers are registered for that table expression, so the action node name falls back to the plain column name whichever value is passed. The plain-name to identifier mapping goes back to taking the first matching declared column instead of the longest. The longest match was meant to stop a table declaring both `b` and `` `a.b` `` from letting `b` claim `__table1.a.b`. Nothing produces that name: `buildColumnIdentifier` always backquotes a dotted column name, so the identifier is ``__table1.`a.b` `` and does not end in `.b`. Reintroducing the first-match behaviour leaves `05065` passing, which is what settled it. The clause accepting an unquoted dotted name stays, now described as defensive rather than as something a branch emits. `05065` stays as a values guard for a schema shape nothing else covers, with its header cut down to say that and nothing more. Everything else from the `Merge` commit stands: the plain-name aliasing loop, the identifier-named alias outputs and the declared-schema mapping are the actual fix, and `04281`, `05057` and `05059` cover them. Related: #1844 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
mkmkme
added a commit
that referenced
this pull request
Sep 19, 2026
`__aliasMarker` stopped requiring a `String` second argument when the id became a live `ColumnNode`, but its registration still declared one. #1844 made the same relaxation and edited only the one-line description, and this branch inherited that, so the documented contract has been wrong since the marker gained its second phase. The argument is an identity rather than a name, so call it `alias_id`, and say what it is for: recording which `ALIAS` column an expression was inlined from, so that a shard names the result the way the initiator expects. Direct use is still not intended, but it is harmless -- the function returns its first argument unchanged. `03648_alias_marker_with_mergeable_state.sh` gains a note on why every invocation passes `--query_kind secondary_query`. The test prints the header a shard leg produces, and a query marked as secondary skips AST-level optimizations and splits aggregation differently, neither of which a real shard leg would have applied. It came from #1844 unchanged and nothing recorded the reason. Related: #1844 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Sep 19, 2026
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):
Fix wrong results / errors for
ALIAScolumns read throughDistributed(andMerge-over-Distributed, distributed-over-distributed) by reconciling shard headers by name via a reworked__aliasMarker.Documentation entry for user-facing changes
Forward-port of #1463 onto
antalya-26.3.What:
__aliasMarkernow works in two phases — inject a marker carrying a live column reference during distributed rewrite, then materialize it to the column identifier (__tableN.col) right before serializing to the shard — so an inlinedALIASexpression keeps a name-matchable identity across the initiator/shard boundary. Header reconciliation prefers matching by name (with a position fallback) in the planner andStorageMerge. See the design comment inidentity.h.Why: without it, distributed
ALIAS-column queries could return swapped columns or fail with a type mismatch (CANNOT_PARSE_TEXT/THERE_IS_NO_COLUMN).Notes:
enable_alias_marker(default on); set to0on the initiator for mixed-version clusters whose shards don't understand__aliasMarker.03842/03843/03921/03923/03928/03930-03934(plainDistributed, parallel replicas,Merge, lambda-misuse guard, setting-effect).Related: #1424, #1335 (closed via #1463).