ClickHouse / ClickHouse/ClickHouse

WATERMARK expression over an ALIAS column passes validation, then throws UNKNOWN_IDENTIFIER

Open
#116,308 0 comments 0 reactions 2 assignees Claimed by @Michicosun View on GitHub
bug comp-mergetree comp-query-analyzer
Dominant language
C++
Stars
49.9k
Forks
9k
Avg merge
21h 32m
Merged PRs (30d)
515

Description

### Describe what's wrong

**`SELECT ... FROM t STREAM WATERMARK FOR ts AS ts_alias` where `ts_alias` is an `ALIAS` column is accepted by the analyzer (resolved and type-checked) and then dies at execution with `Code 47. Unknown expression identifier 'ts_alias' in scope ts_alias: While executing MergeTreeCommitOrderSource`.**

- **Root cause:** The analyzer-side validator and the execution-side resolvers use different column namespaces for the same expression: `getColumns()` (all, incl. `ALIAS`) at [`src/Analyzer/Resolve/QueryAnalyzer.cpp:827`](https://github.com/ClickHouse/ClickHouse/blob/04971e144f779e/src/Analyzer/Resolve/QueryAnalyzer.cpp#L827) versus `getColumns().getAllPhysical()` at buildReadRoundPipeline.cpp:69 and MergeTreeCommitOrderSource.cpp:100.

Analysis details (evidence, affected locations, impact)

**Why we believe this is a bug:** `validateWatermarkSettings` builds its `StorageDummy` from `storage_snapshot->metadata->getColumns()` ([`src/Analyzer/Resolve/QueryAnalyzer.cpp:827`](https://github.com/ClickHouse/ClickHouse/blob/04971e144f779e/src/Analyzer/Resolve/QueryAnalyzer.cpp#L827)), i.e. the full `ColumnsDescription` including `ALIAS` columns, so resolution and the result-type equality check both succeed. Every execution-side resolution uses a strictly smaller namespace: `extendWithAuxiliaryColumns` calls `collectWatermarkSourceColumns(..., metadata->getColumns().getAllPhysical(), ...)` ([`src/Storages/MergeTree/Streaming/ReadingPlan/buildReadRoundPipeline.cpp:69`](https://github.com/ClickHouse/ClickHouse/blob/04971e144f779e/src/Storages/MergeTree/Streaming/ReadingPlan/buildReadRoundPipeline.cpp#L69)), `restoreStreamingAuxiliaryColumns` does the same ([`src/Storages/MergeTree/Streaming/MergeTreeCommitOrderSource.cpp:100`](https://github.com/ClickHouse/ClickHouse/blob/04971e144f779e/src/Storages/MergeTree/Streaming/MergeTreeCommitOrderSource.cpp#L100)), and `CalculateWatermarksStep::transformPipeline` re-resolves the expression against the read header. `getAllPhysical()` excludes `ALIAS` columns, so the identifier cannot be resolved and `buildWatermarkActionsDAG` throws inside the source.

**Affected locations:**
- [`src/Analyzer/Resolve/QueryAnalyzer.cpp:827`](https://github.com/ClickHouse/ClickHouse/blob/04971e144f779e/src/Analyzer/Resolve/QueryAnalyzer.cpp#L827) — validateWatermarkSettings resolves against the full ColumnsDescription
- [`src/Storages/MergeTree/Streaming/ReadingPlan/buildReadRoundPipeline.cpp:69`](https://github.com/ClickHouse/ClickHouse/blob/04971e144f779e/src/Storages/MergeTree/Streaming/ReadingPlan/buildReadRoundPipeline.cpp#L69) — extendWithAuxiliaryColumns resolves against getAllPhysical()
- [`src/Storages/MergeTree/Streaming/MergeTreeCommitOrderSource.cpp:100`](https://github.com/ClickHouse/ClickHouse/blob/04971e144f779e/src/Storages/MergeTree/Streaming/MergeTreeCommitOrderSource.cpp#L100) — restoreStreamingAuxiliaryColumns resolves against getAllPhysical()

**Impact:** A watermark expression naming an `ALIAS` column is accepted at parse/analysis time and then fails mid-execution with a raw `UNKNOWN_IDENTIFIER`. Every other invalid `WATERMARK` input (missing column, non-temporal column type, result-type mismatch) is rejected up front with `ILLEGAL_STREAM` and an explanatory message; this one is not, so a long-running unbounded stream is accepted and then dies on its first read round with an error that does not mention `WATERMARK`.

### Does it reproduce on most recent release?

Yes — confirmed on current `master` (commit `04971e144f779e`).

### How to reproduce

[▶ Run on ClickHouse Fiddle](https://fiddle.clickhouse.com/eacabd9e-159b-4676-b514-1474d13ed381)

Reproducer

```sql
-- no-old-analyzer: streaming queries require the analyzer.

DROP TABLE IF EXISTS t_stream_wm_alias;
SET enable_streaming_queries = 1;
SET enable_analyzer = 1;
SET enable_parallel_replicas = 0;

DROP TABLE IF EXISTS t_stream_wm_alias;

CREATE TABLE t_stream_wm_alias
(
ts DateTime64(3),
ts_alias DateTime64(3) ALIAS ts - INTERVAL 5 SECOND,
x UInt64
)
ENGINE = MergeTree ORDER BY tuple()
SETTINGS enable_block_number_column = 1, enable_block_offset_column = 1;

INSERT INTO t_stream_wm_alias (ts, x) VALUES ('2020-01-01 00:00:10.000', 1);

-- A MATERIALIZED/physical column works.
SELECT x, _watermark FROM t_stream_wm_alias STREAM BOUNDED WATERMARK FOR ts AS ts - INTERVAL 5 SECOND;

-- An ALIAS column resolves during analysis but not during execution; the query must be
-- rejected with the same error the other WATERMARK validations use, not mid-execution.
SELECT x, _watermark FROM t_stream_wm_alias STREAM BOUNDED WATERMARK FOR ts AS ts_alias; -- { serverError ILLEGAL_STREAM }

DROP TABLE IF EXISTS t_stream_wm_alias;
```

### Expected behavior

```
1 2020-01-01 00:00:05.000
```

### Error message and/or stacktrace

```
1 2020-01-01 00:00:05.000
Expected server error code: 1007 but got: 47
Code: 47. DB::Exception: Unknown expression identifier `ts_alias` in scope ts_alias: While executing MergeTreeCommitOrderSource. (UNKNOWN_IDENTIFIER)
```

Suggested fix

Either (a) make `validateWatermarkSettings` resolve against `storage_snapshot->metadata->getColumns().getAllPhysical()` so the two namespaces agree and the query is rejected with `ILLEGAL_STREAM` at analysis time, or (b) expand `ALIAS` columns before `collectWatermarkSourceColumns` / `buildWatermarkActionsDAG` so the expression executes. Pick one; today the two sides disagree.

Additional context

**Open risks:**
- The same namespace mismatch would hit any other non-physical column form the analyzer can resolve but `getAllPhysical()` omits.

Found during automated review of [PR #106169](https://github.com/ClickHouse/ClickHouse/pull/106169). Severity P2 · Finding `h_pr106169_002`

cc @Michicosun @CheSema (from #106169)

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.