ClickHouse / ClickHouse/ClickHouse
Inlined ALIAS body adds constants to the shipped WITH FILL header; parallel replicas throw
- Dominant language
- C++
- Stars
- 49.9k
- Forks
- 9k
- Avg merge
- 21h 32m
- Merged PRs (30d)
- 515
Description
_Found via ClickGap automated review — close or comment if this is wrong._
### Describe what's wrong
With parallel replicas enabled, `SELECT k, a_v FROM t ORDER BY k WITH FILL ... INTERPOLATE (...)` where `a_v` is an `ALIAS` column whose body is an expression fails with `Code: 20. Number of columns doesn't match (source: 5 and result: 4). (NUMBER_OF_COLUMNS_DOESNT_MATCH)`. The same query returns rows correctly without parallel replicas, and returns rows correctly under parallel replicas if the alias body is written inline in the `SELECT` list instead of declared as an `ALIAS` column.
**Root cause:** `inlineAliasColumns` at src/Interpreters/ClusterProxy/executeQuery.cpp:1048 changes the structure of the shipped query tree, but the initiator's `expected_header` is still computed from the un-inlined tree; when the shipped tree's `WithMergeableState` header gains columns (any plan step that keeps ActionsDAG intermediates, e.g. `Filling`), no reconciliation path can absorb it.
Analysis details (evidence, affected locations, impact)
**Why we believe this is a bug:** `PlannerJoinTree::buildQueryPlanForTableExpression` (src/Planner/PlannerJoinTree.cpp:2048) calls `ClusterProxy::executeQueryWithParallelReplicas`, which since this PR runs `inlineAliasColumns` on the shipped tree (src/Interpreters/ClusterProxy/executeQuery.cpp:1048) and derives the replica `header` from that inlined tree (executeQuery.cpp:1052). Back in `buildQueryPlanForTableExpression`, `expected_header` is re-planned from the ORIGINAL, un-inlined `select_query_info.query_tree` (PlannerJoinTree.cpp:2262-2268). For `ORDER BY ... WITH FILL ... INTERPOLATE` the `WithMergeableState` header is the `Filling` step header, which keeps every ActionsDAG output including the constants of the inlined body — so the inlined tree yields one more column (`2_UInt8` for a `v * 2` body) than the un-inlined tree. `buildShardCollapseFanOut` bails out because it only handles a SMALLER shard header (src/Storages/buildQueryTreeForShard.cpp:1313), and the positional `ActionsDAG::makeConvertingActions` at PlannerJoinTree.cpp:2293 then throws.
**Affected locations:**
- `src/Interpreters/ClusterProxy/executeQuery.cpp:1048` — new `inlineAliasColumns` call on the shipped tree; `header` at line 1052 is derived from it
- `src/Planner/findParallelReplicasQuery.cpp:622` — sibling new `inlineAliasColumns` call; `initial_header` at line 615 is taken from the un-inlined tree and reconciled positionally at line 650
- `src/Planner/PlannerJoinTree.cpp:2293` — positional `makeConvertingActions` that throws NUMBER_OF_COLUMNS_DOESNT_MATCH
- `src/Storages/buildQueryTreeForShard.cpp:1313` — `buildShardCollapseFanOut` returns {} when the shard header is not strictly smaller, so a LARGER shard header is unhandled
**Impact:** Any query combining an `ALIAS` column whose body is an expression with `ORDER BY ... WITH FILL ... INTERPOLATE` is rejected once `enable_parallel_replicas = 1`. Deterministic, reproduces with both `parallel_replicas_local_plan = 0` and `= 1`. It also masks the correct user error: `INTERPOLATE (k AS k)` (an ORDER BY column as interpolate target) reports `NUMBER_OF_COLUMNS_DOESNT_MATCH` instead of `INVALID_WITH_FILL_EXPRESSION`.
## Assumptions
_Unverified assumptions — tick to confirm, comment to refute:_
- [ ] **Before this PR the same query succeeded on the parallel-replicas path**
- *Why unverifiable:* no pre-PR binary is available in this environment to run the query against
- *Falsifiable test:* Build master (without this PR) and run the repro; expect `0 10 / 2 10 / 4 14 / 6 14 / 8 18`. Pre-PR `executeQueryWithParallelReplicas` shipped the un-inlined tree, so `header` and `expected_header` came from the same query node and matched structurally.
### Does it reproduce on most recent release?
Yes — confirmed on current `master` (commit `48b91073fbabef`).
### How to reproduce
```sql
CREATE TABLE t (k UInt32, v Int64, a_v Int64 ALIAS v * 2) ENGINE = MergeTree ORDER BY k; INSERT INTO t VALUES (0,5),(4,7),(8,9); then run SELECT k, a_v FROM t ORDER BY k WITH FILL FROM 0 TO 10 STEP 2 INTERPOLATE (a_v AS a_v) SETTINGS enable_parallel_replicas = 1, max_parallel_replicas = 3, cluster_for_parallel_replicas = 'test_cluster_one_shard_three_replicas_localhost', parallel_replicas_for_non_replicated_merge_tree = 1, automatic_parallel_replicas_mode = 0, serialize_query_plan = 0;
```
### Expected behavior
```
0 10
2 10
4 14
6 14
8 18
0 10
2 10
4 14
6 14
8 18
```
### Error message and/or stacktrace
```
0 10
2 10
4 14
6 14
8 18
Received exception from server (version 26.8.1):
Code: 20. DB::Exception: Received from 127.0.0.1:19020. DB::Exception: Number of columns doesn't match (source: 5 and result: 4). (NUMBER_OF_COLUMNS_DOESNT_MATCH)
```
### Additional context
**Open risks:**
- The same repro over a 2-shard `Distributed` table fails identically. That path has always inlined, so it is likely broken on master too and is out of scope for this PR — but a fix should cover both, since they share `buildQueryTreeForShard`.
- Only `WITH FILL`/`INTERPOLATE` was found to leak ActionsDAG intermediates into the `WithMergeableState` header. Other steps with the same property would fail the same way; not audited exhaustively.
**Suggested fix:** Compute the initiator-side `expected_header` from the SAME inlined tree that is shipped (or run `inlineAliasColumns` on the tree used for `expected_header` in `PlannerJoinTree::buildQueryPlanForTableExpression`), instead of relying on the shipped and expected headers happening to agree. Alternatively extend `buildShardCollapseFanOut` to drop shard columns the initiator does not expect, not only to fan out missing ones.
Found during automated review of [PR #107700](https://github.com/ClickHouse/ClickHouse/pull/107700).
---
_ClickGapAI · Severity: P2 · Finding: `h_pr107700_001`_
Contributor guide
Assessment
This issue has not been assessed yet.