{{ message }}
Cherry pick #101036 to 26.1: Fix ColumnReplicated type mismatch crash in merge algorithms#103278
Merged
Merged
Conversation
When MergedData::initialize() runs, it wraps destination columns as ColumnReplicated only if initial inputs have ColumnReplicated columns. However, inputs that are null (no chunk) at initialization time are skipped. When those inputs later arrive via consume(), the previous code only materialized sort columns via removeReplicatedFromSortingColumns(), leaving non-sort ColumnReplicated columns intact. This caused a type mismatch when insertRow/insertRows tried to insert from ColumnReplicated source into a regular destination column, triggering the assertTypeEquality assertion at IColumn.h:862. The fix has two layers: 1. Primary: All merge algorithm consume() methods now call removeReplicatedFromAllColumns() which materializes ALL ColumnReplicated columns, not just sort columns. This prevents the mismatch from occurring. 2. Defense-in-depth: MergedData::insertRow, insertRows, and insertChunk now check for ColumnReplicated/regular mismatches and materialize the source when needed. This addresses STID 2508 family crashes (2508-37b9, 2508-35cc, 2508-394e, 2508-32f6, 2508-35de, 2508-33f1, 2508-2c31) which collectively account for 47+ hits across 40+ PRs and 4+ master hits in 30 days, accelerating to 15+ hits in the last 3 days. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Move ColumnReplicated materialization to the top of the per-column loop in insertChunk, before hasDynamicStructure/hasStatistics branches. This prevents ColumnDynamic::insertRangeFrom from receiving a ColumnReplicated source (causing assert_cast crash in debug/sanitizer, UB in release). The previous code had the replicated guard after dynamic/statistics branches, so ColumnReplicated(ColumnDynamic) would enter the hasDynamicStructure path first and crash. Also adds a regression test with ColumnReplicated(ColumnDynamic) in insertChunk to cover this specific code path. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…stination Based on @Avogar's review feedback on PR #101036, this commit redesigns the fix for the `STID 2508` family `MergedData` type mismatch exception (`isConst() || isSparse() || isReplicated()` assertion at `IColumn.h:862`). Previous fix (reverted here) materialized all `ColumnReplicated` columns in the merge algorithms' `consume` methods, which killed the lazy replication optimization for every late-arriving chunk. It also materialized sources in `MergedData::insertRow` / `insertRows`, losing the optimization on every row insertion. New approach: detect the mismatch in `MergedData` and WRAP the destination in `ColumnReplicated` using the identity-indexes constructor. After wrapping, `ColumnReplicated::insertFrom` / `insertRangeFrom` consume both regular and replicated sources through the same optimized path, preserving the lazy replication optimization for the entire merged output. Changes: 1. Revert `removeReplicatedFromAllColumns` in all 5 merge algorithm `consume` methods. Restore `removeReplicatedFromSortingColumns`, which only materializes sort columns — non-sort `ColumnReplicated` columns now flow through to `MergedData` unchanged. 2. In `MergedData::insertRow` and `insertRows`: when the source is `ColumnReplicated` but the destination is regular, wrap the destination in `ColumnReplicated` via `ColumnReplicated::create(std::move(columns[i]))`. The identity-indexes constructor is a structural wrap — existing data is preserved in `nested_column` with indexes `[0, 1, ..., N-1]`. 3. In `MergedData::insertChunk`: drop the eager up-front materialization. Handle each branch on its own terms: - `hasDynamicStructure` wraps the empty destination in `ColumnReplicated` when the chunk is replicated, so `ColumnReplicated::insertRangeFrom` handles the source (not `ColumnDynamic::insertRangeFrom`, which would `assert_cast` a `ColumnReplicated` source). - `hasStatistics` works unchanged — `takeOrCalculateStatisticsFrom` is delegated through `ColumnReplicated` to the nested column. - `columns[i]->isReplicated()` handles a replicated chunk via `std::move` instead of re-wrapping. - The simple else branch lets `ColumnReplicated` chunks flow through, so the merged destination inherits the optimization. 4. Rewrite `gtest_merged_data_replicated.cpp`: - Keep regression tests for `insertRow` / `insertRows` / `insertChunk` covering regular destination + `ColumnReplicated` source, including `ColumnReplicated(ColumnDynamic)`. - Add `InsertRowsWrapsDestinationPreservingOptimization` which asserts the merged destination is `ColumnReplicated` after mismatch handling (not materialized as regular). Reproduction note: over ~3 weeks of attempts I could not reproduce the assertion failure through SQL or integration tests — it requires the stress-test-scale parallelism + sanitizer timing that CI provides. The gtests exercise the exact conditions that arrive at `MergedData` under that environment.
…ted-merge-crash Fix ColumnReplicated type mismatch crash in merge algorithms
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Original pull-request #101036
Do not merge this PR manually
This pull-request is a first step of an automated backporting.
It contains changes similar to calling
git cherry-picklocally.If you intend to continue backporting the changes, then resolve all conflicts if any.
Otherwise, if you do not want to backport them, then just close this pull-request.
The check results does not matter at this step - you can safely ignore them.
Troubleshooting
If the conflicts were resolved in a wrong way
If this cherry-pick PR is completely screwed by a wrong conflicts resolution, and you want to recreate it:
pr-cherrypicklabel from the PRYou also need to check the Original pull-request for
pr-backports-createdlabel, and delete if it's presented thereThe PR source
The PR is created in the CI job