25.8: Fix crash merging _part_offset projection on row-combining engines

[mkmkme]

A projection that carries the parent _part_offset can only be safely merged when the parent merge preserves rows 1:1. Row-combining engines (Summing/Aggregating/Graphite/Coalescing) collapse rows with equal sorting keys, so the source-to-merged offset mapping becomes incomplete and the projection must be rebuilt instead.

These modes were missing from merge_may_reduce_rows, so such projections took the merge path and remapped offsets through a mapping that had no entry for the collapsed rows, reading out of bounds and aborting with "Logical error: 'page_pos < pages.size()'" (e.g. OPTIMIZE FINAL on a CoalescingMergeTree table with a _part_offset projection and deduplicate_merge_projection_mode = 'rebuild').

Add the four row-combining modes to merge_may_reduce_rows so these projections are rebuilt, and add a regression test.

Changelog category (leave one):

• Critical Bug Fix (crash, data loss, RBAC)

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

Fixes a bug where OPTIMIZE FINAL on row-combining engines could crash the server (ClickHouse#106929)

Documentation entry for user-facing changes

...

CI/CD Options

Exclude tests:

• Fast test
• Integration Tests
• Stateless tests
• Stateful tests
• Performance tests
• Aarch64 tests
• All with ASAN
• All with TSAN
• All with MSAN
• All with UBSAN
• All with Coverage
• All Regression
• Disable CI Cache

Regression jobs to run:

• Fast suites (mostly <1h)
• Aggregate Functions (2h)
• Alter (1.5h)
• Benchmark (30m)
• ClickHouse Keeper (1h)
• Iceberg (2h)
• LDAP (1h)
• OAuth (5m)
• Parquet (1.5h)
• RBAC (1.5h)
• SSL Server (1h)
• S3 (2h)
• S3 Export (2h)
• Swarms (30m)
• Tiered Storage (2h)

[github-actions]

Workflow [PR], commit [19ab052]

[ilejn]

@mkmkme , could you explain the nature of the change.
At first glance it is a backport of ClickHouse#92925 , but on the second it is not.

[mkmkme]

@codex review

[mkmkme]

@mkmkme , could you explain the nature of the change. At first glance it is a backport of ClickHouse#92925 , but on the second it is not.

Yeah, the original report contains the proper reproduction for the crash and the crash logs. However, the original report jumped into the wrong conclusion. The PR 92925 cannot be backported into 25.8, because it mostly changes the files that are missing in 25.8. The only change that is possible to backport from that PR is this one-liner mentioned by Boris:

// 25.8 (broken) — MergeTask.cpp:1430
-  if (global_ctx->merged_part_offsets && !global_ctx->projections_to_merge.empty())
// fixed by #92925
+  if (global_ctx->merged_part_offsets && !global_ctx->merged_part_offsets->isFinalized())
       global_ctx->merged_part_offsets->flush();

When I applied this change, the crash still reproduced. Turned out the root cause was in a different part of the code. Basically merge_may_reduce_rows was missing the actual engine types that can reduce the number of rows. This caused the later code to assume that remapped offsets collections was of the same size as the original one which triggered the logical error.

So the actual fix is to add those missing engine types to merge_may_reduce_rows.

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Chef's kiss.

Reviewed commit: 429e5bad16

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[ilejn]

Looks good to me