Baike.dev
Log in
Back to tool/Back to issues
#120649·ClickHouse

Collapsing merge changes `sum(sign)`: level>0 part's key pair never re-collapsed

Author: clickgapaiCreated Sep 17, 2026Updated Sep 17, 2026
Labelscomp-mergetree

Describe what's wrong

A merge of a CollapsingMergeTree table changes sum(sign) for a key from 1 to 0, so the live row silently disappears from the engine's documented query pattern (GROUP BY key ... HAVING sum(sign) > 0). The stale cancel row survives forever and no later merge can restore the count. SELECT ... FINAL still returns the state row, so the two supported read paths disagree about the same part. Trigger: the oldest part holding the key is a merged part (level > 0) that kept a cancel/state pair, and a newer part holds one more state row than cancel rows for that key.

  • Root cause: src/Processors/Merges/Algorithms/IMergingAlgorithmWithSharedChunks.h:86 - the 'a part with non-zero level has no duplicate keys' premise (spelled out at ReplacingSortedAlgorithm.cpp:282) is true for ReplacingMergeTree but false for CollapsingMergeTree, whose own merge rule emits up to two rows per key. The shortcut is applied unconditionally to every algorithm derived from IMergingAlgorithmWithSharedChunks.
Analysis details (evidence, affected locations, impact)

Why we believe this is a bug: CollapsingSortedAlgorithm::merge derives key-group boundaries from rowsHaveDifferentSortColumns (src/Processors/Merges/Algorithms/CollapsingSortedAlgorithm.cpp:210), which returns 'different key' for any two rows read from the same source when that source part's level > 0 (src/Processors/Merges/Algorithms/IMergingAlgorithmWithSharedChunks.h:86). A merged CollapsingMergeTree part legitimately holds two rows per key - first_negative and last_positive, exactly as the class contract in CollapsingSortedAlgorithm.h:20-24 prescribes - so that pair is split into two key groups: the first group emits the stale cancel row, the second collapses the remaining rows on its own.

Affected locations:

Impact: Silent wrong results for the documented CollapsingMergeTree query pattern, permanent once the merge has run: the object reads as deleted (sum(sign) = 0) although its state row is still on disk and FINAL still returns it. The docs state the opposite guarantee - 'collapsing should not change the results of calculating statistics' - and the engine's own rule 2 ('the last state row, if there are more state rows than cancel rows') prescribes a single row for the repro's key. No setting is involved. The shortcut is present on 26.3, 26.6, 26.7, 26.8 (code index: IMergingAlgorithmWithSharedChunks.h:61 on each) and on this 26.10.1 build. Scope verified: CollapsingMergeTree only - VersionedCollapsingMergeTree compares keys directly and ReplacingMergeTree/SummingMergeTree/AggregatingMergeTree keep one row per key in a merged part. NOT introduced by this PR; the PR rewrites the same merge loop.

Does it reproduce on most recent release?

Yes — confirmed on current master (commit 2d19d1b6e073).

How to reproduce

-- Test: merging a CollapsingMergeTree part that already keeps a cancel/state pair for a key
-- must not change sum(sign) for that key.

DROP TABLE IF EXISTS t_05223_merged_pair;
DROP TABLE IF EXISTS t_05223_unmerged_pair;
SET optimize_on_insert = 0;
SET async_insert = 0;

DROP TABLE IF EXISTS t_05223_merged_pair;
DROP TABLE IF EXISTS t_05223_unmerged_pair;

CREATE TABLE t_05223_merged_pair (id UInt64, sign Int8, s String) ENGINE = CollapsingMergeTree(sign) ORDER BY id;

INSERT INTO t_05223_merged_pair VALUES (1, -1, 'a'), (1, 1, 'b');
OPTIMIZE TABLE t_05223_merged_pair FINAL;

SYSTEM STOP MERGES t_05223_merged_pair;
INSERT INTO t_05223_merged_pair VALUES (1, 1, 'c');
SELECT 'merged_pair before', count(), sum(sign) FROM t_05223_merged_pair;
SYSTEM START MERGES t_05223_merged_pair;

OPTIMIZE TABLE t_05223_merged_pair FINAL;
SELECT 'merged_pair after', count(), sum(sign) FROM t_05223_merged_pair;
SELECT 'merged_pair rows', id, sign, s FROM t_05223_merged_pair ORDER BY sign, s;

-- Control: the same rows, with the pair never merged on its own.
CREATE TABLE t_05223_unmerged_pair (id UInt64, sign Int8, s String) ENGINE = CollapsingMergeTree(sign) ORDER BY id;

SYSTEM STOP MERGES t_05223_unmerged_pair;
INSERT INTO t_05223_unmerged_pair VALUES (1, -1, 'a'), (1, 1, 'b');
INSERT INTO t_05223_unmerged_pair VALUES (1, 1, 'c');
SELECT 'unmerged_pair before', count(), sum(sign) FROM t_05223_unmerged_pair;
SYSTEM START MERGES t_05223_unmerged_pair;

OPTIMIZE TABLE t_05223_unmerged_pair FINAL;
SELECT 'unmerged_pair after', count(), sum(sign) FROM t_05223_unmerged_pair;
SELECT 'unmerged_pair rows', id, sign, s FROM t_05223_unmerged_pair ORDER BY sign, s;

DROP TABLE t_05223_merged_pair;
DROP TABLE t_05223_unmerged_pair;

Expected behavior

Expected output of the reproducer above:

merged_pair before	3	1
merged_pair after	1	1
merged_pair rows	1	1	c
unmerged_pair before	3	1
unmerged_pair after	1	1
unmerged_pair rows	1	1	c

Error message and/or stacktrace

Actual output of the reproducer above on master (2d19d1b6e073):

merged_pair before	3	1
merged_pair after	2	0
merged_pair rows	1	-1	a
merged_pair rows	1	1	c
unmerged_pair before	3	1
unmerged_pair after	1	1
unmerged_pair rows	1	1	c
Suggested fix

Stop applying the level>0 shortcut in algorithms whose merges can leave several rows per key in one part: add a flag to IMergingAlgorithmWithSharedChunks that CollapsingSortedAlgorithm sets to false (keep it true for ReplacingSortedAlgorithm), or compare the sort columns unconditionally in the CollapsingSortedAlgorithm path. Trade-off: CollapsingMergeTree merges then pay one key comparison per row from level>0 sources, losing the comparison-skipping optimization for that engine.

Additional context

Open risks:

  • src/Processors/Merges/Algorithms/IMergingAlgorithmWithDelayedChunk.h:38 carries the same shortcut for the delayed-chunk algorithms. SummingSortedAlgorithm (SummingSortedAlgorithm.cpp:1025) and AggregatingSortedAlgorithm (AggregatingSortedAlgorithm.cpp:331) are safe because a merged part keeps one row per key, but I did not test GraphiteRollupSortedAlgorithm: whether a rollup can leave several rows per sorting key in a merged part, and therefore whether re-merging such a part loses rows, still needs a dedicated Graphite test.
  • I did not check whether an upstream issue already tracks this; the level>0 shortcut predates the shallow git history available in this checkout.

Found during automated review of PR #119900; the bug predates that PR (it reproduces on the master build just before it merged), so the introducing change is not identified yet. Severity P1 · Finding h_pr119900_001

Source: ClickHouse/ClickHouse

View original on GitHubView discussion on GitHub