Reject partition manipulation between tables with a different index_granularity for non-adaptive parts
A non-adaptive data part stores no per-mark row counts on disk: its mark file holds only the two offsets per mark, so the rows per granule have to be reconstructed when the part is loaded, and they are reconstructed from the READING table's `index_granularity`. `MergeTreeData::canReplacePartition` is the only compatibility gate on the partition-manipulation path, and it compared mark adaptivity only. With both tables non-adaptive (`index_granularity_bytes = 0`) neither adaptivity branch fires, so a part written at `index_granularity = N` was admitted into a table declared with `index_granularity = M != N`. `MergeTreeDataPartWide::loadIndexGranularityImpl` then paired the source's mark offsets with the destination's rows per mark, mapping every mark to the wrong row range. Observable consequences, both measured on 8627fa22f01595b: - Release build: the part attaches and the table disagrees with itself. For 18 rows moved from `index_granularity = 8` into `index_granularity = 4`, `SELECT count() FROM t WHERE <primary key condition>` returned 10; in the other direction (4 into 8) it returned 24, while `SELECT count() FROM t` returned the correct 18. - Debug and sanitizer builds: the consistency check in `IMergeTreeDataPart::loadRowsCount` throws `LOGICAL_ERROR`, which those builds turn into a server abort. The new condition is placed outside the `enable_mixed_granularity_parts` block on purpose: a destination that accepts mixed granularity still reinterprets a non-adaptive part, because `changeGranularityIfRequired` clears `mark_type.adaptive` and `index_granularity_bytes` from the filesystem but leaves `fixed_index_granularity` at the destination's value. That case was measured to abort on master too, and it is the only arm of the new test that distinguishes this placement from putting the check inside the block. Comparing against the source part's own `fixed_index_granularity` is sound because `index_granularity` is read-only after table creation (`MergeTreeSettings::isReadonlySetting`, enforced as `READONLY_SETTING` in `MergeTreeData::checkAlterIsPossible`), so for any part the source table wrote itself the setting is the value the part was written with. `MergeTreeDataPartCompact` needs no equivalent guard: a Compact part is always adaptive and its marks record per-granule row counts, so nothing is reconstructed. Verified that a non-adaptive table cannot write a Compact part even with `min_bytes_for_wide_part` and `min_rows_for_wide_part` set to 1e9. The condition only ever adds rejections, so it cannot make a previously rejected operation succeed. Previously accepted DDL now errors with `BAD_ARGUMENTS`; the accepted cases produced a part whose marks map to the wrong rows unless the partition fitted in a single granule. The new test arms live in the existing `04150_move_partition_inconsistent_granularity`, which carries `no-shared-merge-tree`: in Cloud `ReplicatedMergeTree` is substituted by `SharedMergeTree`, whose partition-move path does not call `canReplacePartition`. Dropping that tag is what got #103881 reverted. The arms cover both directions of the mismatch and all of `REPLACE`/`MOVE`/`ATTACH PARTITION FROM`, so a one-sided comparison does not satisfy them. `r_src_nonadaptive` pins `index_granularity = 8192`, its default, because `clickhouse-test` otherwise injects a random value as the CREATE TABLE default and a draw equal to the destination's 4096 would remove that arm's expected rejection. One arm asserts an acceptance instead: an adaptive source part at a different `index_granularity` is still admitted, because its marks carry per-granule row counts and the destination's setting never enters their interpretation. It pins the scope of the new condition, so dropping the `!adaptive` conjunct would be caught by the test rather than silently rejecting a valid transfer. That arm pins its source part to the `Wide` format, which is the format whose granularity load path holds the defect, and without which restating the conjunct as a `Wide` part-type test would also pass. Reported and root-caused by Pedro Ferreira. Found by the server-side AST fuzzer under Stress test (arm_debug) on https://github.com/ClickHouse/ClickHouse/pull/116783 : https://s3.amazonaws.com/clickhouse-test-reports/praktika.html?PR=116783&sha=dd8310bb0d928a3aa50822f052cc835cf86efac0&name_0=PR&name_1=Stress%20test%20%28arm_debug%29 Closes #117524
G
Groene AI committed
91706c17efc3484fc4da04be057c86d13a8cb21b
Parent: 8627fa2