ClickHouse / ClickHouse/ClickHouse

Aggregate projection column types depend on type-affecting session settings (e.g. `cast_keep_nullable`), diverging between declaration, write and read

Open
#116,824 1 comment 0 reactions 0 assignees View on GitHub
bug comp-projections
Dominant language
C++
Stars
49.9k
Forks
9k
Avg merge
21h 32m
Merged PRs (30d)
515

Description

🕵 Filed as the tracking issue for the aggregate-projection carrier found during the review of https://github.com/ClickHouse/ClickHouse/pull/109196 (analysis and measurements by @groeneai in that PR's review thread). That PR pins `MergeTree` key and skip-index expression types to the server settings baseline, but deliberately does not touch projections, because the projection side needs its own compatibility decision.

### Summary

A projection's column types are derived by re-analyzing its `query_ast` at three stages with three different contexts, and nothing forces them to agree:

| stage | site | context |
|---|---|---|
| declared type | `ProjectionsDescription.cpp` `fillProjectionDescriptionByQuery` | global context |
| write | `ProjectionsDescription.cpp` `calculateByQuery` | the inserting query's context |
| read | `optimizeUseAggregateProjection.cpp` | the reading query's context |

Any type-affecting session setting (for example `cast_keep_nullable`) therefore produces a divergence.

### Reproducer

With `PROJECTION p (SELECT CAST(x AS UInt32) AS k, count() GROUP BY k)` over a `Nullable(UInt32)` column `x`:

- setting on at `INSERT`: `Bad cast from type DB::ColumnNullable to DB::ColumnVector` — logical error (exception in release builds, abort in debug/sanitizer builds) in `writeProjectionPartImpl`;
- setting on at read, projection declared `UInt32`: `Bad cast from type DB::ColumnVector to DB::ColumnNullable` in `Aggregator::mergeStreamsImpl`. Reproduced on 26.8.1.1 unmodified. `EXPLAIN header = 1` shows it without any fault: `Aggregating` claims `Nullable(UInt32)` while `ReadFromMergeTree (p)` emits `UInt32`.

Also, `CREATE TABLE` under `cast_keep_nullable = 1` with such a projection succeeds and the first `INSERT` under default settings then fails; and an unrelated `ALTER ... SETTINGS cast_keep_nullable = 1` on such a table is rejected with `Cannot apply ALTER because it breaks projection` (fails closed, no metadata corruption).

### Why the narrow fixes do not work

1. Pinning the setting on the projection `SELECT` (in `ASTProjectionSelectQuery::cloneToASTSelect`, next to the existing `aggregate_functions_null_for_empty` and `transform_null_in` pins) fixes the `INSERT` abort, but a table created while the setting was a profile default already has a projection part whose `columns.txt` records `Nullable(UInt32)`; the pinned derivation then yields `UInt32` and the read fails on a table that worked before (measured: same data directory, old binary reads it correctly, pinned binary fails).
2. Converting the projection header at read time, the way the `DISTINCT` branch already does in `optimizeUseAggregateProjection.cpp`, does not transplant to the aggregating branch: it feeds intermediate aggregate states through `requestOnlyMergeForAggregateProjection`, which rewrites the key column to the expected output type with no conversion, and the `Nullable` to non-`Nullable` direction has no lossless answer.

The proper fix is what https://github.com/ClickHouse/ClickHouse/pull/109196 does for keys and skip indexes: resolve the projection expression against a stable settings baseline, applied consistently to declaration, calculation and projection analysis — plus a compatibility decision (and likely a `SettingsChangesHistory` entry) for tables that already persisted setting-dependent projection column types, so that canonicalizing at load does not convert a failed write into a failed `ATTACH`.

Related: https://github.com/ClickHouse/ClickHouse/pull/109196
Related: https://github.com/ClickHouse/ClickHouse/issues/109181
Related: https://github.com/ClickHouse/ClickHouse/issues/115237

Contributor guide

Open the contributing guide

Research direction

Start with ProjectionsDescription.cpp, especially fillProjectionDescriptionByQuery and calculateByQuery, then trace projection analysis in optimizeUseAggregateProjection.cpp. Reproduce the cast_keep_nullable cases and compare declaration, write, and read types. Done means a decided compatibility approach and consistent handling for new and already persisted projection types, with regression coverage.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp, sql
Domain
databases
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.