Skip to content

fix(clickhouse): FinalMerge counts only rows that are not all-zero on SummingMergeTree - #22

Merged
fm4v merged 1 commit into
ClickHouse:mainfrom
groeneai:fix-final-merge-summing-zero-rows
Oct 12, 2026
Merged

fm4v merged 1 commit into
ClickHouse:mainfrom
groeneai:fix-final-merge-summing-zero-rows

Conversation

@groeneai

Copy link
Copy Markdown

After ClickHouse/ClickHouse#115032, SELECT ... FINAL from SummingMergeTree keeps the rows whose summed columns are all zero, while background merges and OPTIMIZE TABLE ... FINAL still delete them. FinalMerge compares count() under FINAL before and after OPTIMIZE ... FINAL, so on that build it reports every all-zero group as a mismatch.

For a SummingMergeTree table, both reads now use countIf(NOT (c1 = 0 AND ...)) over the integer and Decimal columns that the merge always sums: not in the sorting or partition key, stored (not ALIAS or EPHEMERAL), and in the engine's columns argument when there is one. A row the merge deletes has all of them at zero, so it drops out of both counts, and the check holds with and without #115032 (on master, FINAL deletes only rows where every summed column it reads is zero, which this predicate also excludes). Float columns are left out, because a float sum can be zero in one summation order and not in another. Tables with no such column, or with an unfinished mutation, are skipped. countIf instead of WHERE keeps the predicate away from optimize_move_to_prewhere_if_final. Other engines keep the plain count().

Checked: mvn verify -DskipTests=true passes. I ran --oracle FinalMerge (6 threads) against the CI amd_release builds of #115032 (b3ef481b) and master (a347fefd), with optimize_on_insert=0 so that zero rows survive the insert, and with a local generator tweak (not in this PR) that makes every eligible table a SummingMergeTree:

Server Before this PR With this PR
#115032 38 mismatches in 4 min 0 in 15 min
master 10 mismatches in 4 min (the column-dependent deletion that #115032 removes) 0 in 10 min

As a negative control, the patched oracle with FINAL removed from the first read reports 18 mismatches in 3 min, so the filtered count still catches a read that does not merge.

Unrelated, seen only with the generator tweak: SummingMergeTree(<col>) can name an EPHEMERAL column, and that CREATE TABLE fails with NO_SUCH_COLUMN_IN_TABLE. Not changed here.

Related: ClickHouse/ClickHouse#115032

🤖 Generated with Claude Code

… SummingMergeTree

ClickHouse/ClickHouse#115032 makes `SELECT ... FINAL` from `SummingMergeTree`
keep the rows whose summed columns are all zero, while background merges and
`OPTIMIZE TABLE ... FINAL` still delete them. The FinalMerge oracle compared
`count()` under `FINAL` before and after `OPTIMIZE ... FINAL`, so on a server
with that change it reports every all-zero group as a mismatch.

For a SummingMergeTree table both reads now count only the rows where at least
one summed column is non-zero: `countIf(NOT (c1 = 0 AND ...))`. The columns are
the integer and Decimal columns that the merge sums for certain: not in the
sorting or partition key, physically stored, and listed in the engine's
`columns` argument when there is one. Any row the merge deletes has all of them
equal to zero, so it is excluded from both counts and the comparison holds with
and without #115032. Float columns are left out because a float sum can be zero
in one summation order and not in another. A table without such a column is
skipped, and so is a table with an unfinished mutation, whose columns may not
be readable. `countIf` rather than `WHERE` keeps the predicate from being moved
before the merge under `optimize_move_to_prewhere_if_final`.

Other engines keep the plain `count()` comparison.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@groeneai

Copy link
Copy Markdown
Author

@fm4v I found another false positive from this provider, in the 2026-10-13 master nightly (SQLancer (arm_release), sqlancer b82d56d72663). TLPWhere asserted on Code: 47 UNKNOWN_IDENTIFIER for SELECT * FROM v7, t0 (reproducer log).

The generator ran CREATE VIEW v7 AS SELECT * FROM t1 while t1 was (c0, c1, c2), then ALTER TABLE t1 DROP COLUMN IF EXISTS c2. A plain view keeps the columns it was created with (expected, see ClickHouse/ClickHouse#19087), so every SELECT * from v7 fails with Unknown expression identifier c2. I reproduced it on master 26.10. RENAME COLUMN c2 TO c3 breaks the view in the same way. The server is right here, the generator leaves an unreadable view in the schema.

A predicate in isStableForRepeatedReads() like #18 would not cover this case. TLPWhere is the common TLPWhereOracle, which picks its tables with TestOracleUtils.getRandomTableNonEmptyTables(schema), i.e. straight from getDatabaseTables().

The fix I would send touches one function, ClickHouseSchema.fromConnection: probe each view with SELECT * FROM <db>.<view> LIMIT 0, then either (a) leave a view that throws out of the loaded schema, or (b) DROP VIEW it, which also frees its slot under the 3-view cap. On master the probe throws Code 47 after a dropped or renamed column and succeeds on a healthy view.

May I send that as a PR here? If so, (a) or (b)? Or would you rather handle it yourself?

@fm4v

fm4v commented Oct 13, 2026

Copy link
Copy Markdown
Member

@groeneai open fix PR

@groeneai

Copy link
Copy Markdown
Author

Opened #23. It takes option (b) without touching fromConnection: checkViewsAreValid already drops unreadable views after generation, but its SELECT 1 FROM <view> LIMIT 1 probe passed because ClickHouse prunes the view's columns. For ClickHouse the probe is now SELECT * FROM <view> LIMIT 0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants