Skip to content

Cherry pick #97493 to 25.12: Make convertToFullIfNeeded recursive for compound column types#97628

Merged
robot-ch-test-poll4 merged 7 commits intobackport/25.12/97493from
cherrypick/25.12/97493
Feb 22, 2026
Merged

Cherry pick #97493 to 25.12: Make convertToFullIfNeeded recursive for compound column types#97628
robot-ch-test-poll4 merged 7 commits intobackport/25.12/97493from
cherrypick/25.12/97493

Conversation

@robot-ch-test-poll4
Copy link
Copy Markdown
Contributor

Original pull-request #97493

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-pick locally.
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:

  • delete the pr-cherrypick label from the PR
  • delete this branch from the repository

You also need to check the Original pull-request for pr-backports-created label, and delete if it's presented there

The PR source

The PR is created in the CI job

alexey-milovidov and others added 7 commits February 20, 2026 16:49
`IColumn::convertToFullIfNeeded` only converted the outermost column
(const, replicated, sparse, low cardinality) but did not recurse into
subcolumns of compound types like `ColumnTuple`, `ColumnArray`,
`ColumnNullable`, etc. This caused assertion failures in debug/sanitizer
builds when `Set::appendSetElements` or `MergeTreeIndexAggregatorSet::update`
processed batches from different MergeTree parts that had different sparse
serialization profiles for inner tuple elements.

The fix uses `forEachSubcolumn` to recursively convert all inner columns,
which generically handles all compound column types.

Closes #97474
Closes #97335

https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=97431&sha=6fe0e8c44dd5c6a12583390b4f8fc45e226a3cb4&name_0=PR&name_1=Stateless%20tests%20%28arm_asan%2C%20azure%2C%20parallel%29

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The test creates a MergeTree table with a Tuple column where different
parts have different sparse serialization profiles for inner elements,
then uses `IN` with a subquery to trigger `Set::appendSetElements` which
previously failed with an assertion because `ColumnTuple` inner columns
were not recursively converted from sparse to full.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The assertion in `ColumnTuple::doInsertRangeFrom` only fires when the
destination is non-sparse (`ColumnVector`) and the source is sparse
(`ColumnSparse`), because `ColumnSparse::doInsertRangeFrom` handles
both cases. Swap insert order so the non-sparse part (all_1_1_0) is
read first into `set_elements`, and the sparse part comes second.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
`Set::appendSetElements` is only called when `fill_set_elements` is true,
which happens when `KeyCondition` calls `buildOrderedSetInplace` for index
evaluation. The IN column must be part of the ORDER BY key for this to happen.

Changed `ORDER BY key` to `ORDER BY val` so that the tuple column is in the
primary key and `KeyCondition` triggers the `buildOrderedSetInplace` path.

Verified: aborts on unpatched debug build (exit code 134), passes on patched.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ecursive

Make convertToFullIfNeeded recursive for compound column types
@robot-ch-test-poll4 robot-ch-test-poll4 added pr-cherrypick Cherry-pick of merge-commit before backporting. Do not use manually - automated use only! do not test disable testing on pull request pr-critical-bugfix labels Feb 22, 2026
@robot-ch-test-poll4 robot-ch-test-poll4 merged commit a227ad0 into backport/25.12/97493 Feb 22, 2026
11 checks passed
@robot-ch-test-poll4 robot-ch-test-poll4 deleted the cherrypick/25.12/97493 branch February 22, 2026 16:17
@clickhouse-gh clickhouse-gh bot added the ready-for-backport PR is eligible for backporting (merged 7+ days ago, not reverted) label Mar 27, 2026
@maxknv maxknv removed the ready-for-backport PR is eligible for backporting (merged 7+ days ago, not reverted) label Mar 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do not test disable testing on pull request pr-cherrypick Cherry-pick of merge-commit before backporting. Do not use manually - automated use only! pr-critical-bugfix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants