Skip to content

Fix LOGICAL_ERROR exceptions from recursive convertToFullIfNeeded with LowCardinality inside compound types#97831

Merged
Algunenano merged 6 commits intoClickHouse:masterfrom
Algunenano:fix-concat-with-separator-format-variant-low-cardinality
Feb 25, 2026
Merged

Fix LOGICAL_ERROR exceptions from recursive convertToFullIfNeeded with LowCardinality inside compound types#97831
Algunenano merged 6 commits intoClickHouse:masterfrom
Algunenano:fix-concat-with-separator-format-variant-low-cardinality

Conversation

@Algunenano
Copy link
Copy Markdown
Member

@Algunenano Algunenano commented Feb 24, 2026

Follow-up to #97654 which fixed the same bug only in concat but missed other callers.

convertToFullIfNeeded (made recursive in #97493) strips LowCardinality from inside compound column types like Variant, Dynamic, and Tuple, but the corresponding types are not updated to match. This creates column/type mismatches that trigger LOGICAL_ERROR exceptions in debug/sanitizer builds.

Fixes

1. concatWithSeparator and format functions — same as concat fix in #97654: use only top-level column conversions (Const, Sparse, LowCardinality) instead of the recursive convertToFullIfNeeded, since the type is not updated.

CI report: 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

2. Set (affects IN subqueries with LowCardinality inside Tuple)Set::insertFromColumns calls convertToFullIfNeeded which recursively strips LC from inner subcolumns, but Set::setHeader only stripped top-level LC from set_elements_types. When KeyCondition::tryPrepareSetColumnsForIndex unpacked the tuple to build index conditions, castColumn tried to interpret a ColumnVector<Int8> as ColumnLowCardinality, causing the exception.

Fix: use recursiveRemoveLowCardinality for set_elements_types in both Set::setHeader and the static Set::getElementTypes, matching what convertToFullIfNeeded does to columns.

CI report: https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=97831&sha=6f56284303c16ecc5f5a437491c661d5019f2977&name_0=PR&name_1=AST%20fuzzer%20%28amd_debug%29

3. ColumnDynamic — override convertToFullIfNeeded to skip recursion into variant sub-columns. ColumnDynamic manages its own variant_info type metadata; stripping LC from variant columns without updating this metadata causes column/type mismatches in Set::appendSetElementsColumnDynamic::serializeValueIntoSharedVariant.

4. ColumnVariant — override convertToFullIfNeeded to skip recursion into variant sub-columns. DataTypeVariant sorts variants by name, so stripping LC from sub-columns (changing type names) creates position mismatches between the column data and the type. This caused LOGICAL_ERROR in KeyCondition::tryPrepareSetColumnsForIndex when casting set elements via FunctionCast::createVariantToColumnWrapper.

CI report: https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=96251&sha=e2eba2cbc9d1c15abe066a7a18da15abf93f68ca&name_0=PR&name_1=AST%20fuzzer%20%28amd_debug%29

All other convertToFullIfNeeded call sites were audited — none have the same vulnerability (they either work with scalar types, don't use the type after conversion, or already handle LC stripping properly).

Closes #97701
Closes #97847
Closes #97854

Changelog category (leave one):

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

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

Fix LOGICAL_ERROR exceptions caused by LowCardinality inside compound types (Variant, Dynamic, Tuple) in concatWithSeparator, format, IN subqueries, GLOBAL IN, and joins with runtime filters.

Documentation entry for user-facing changes

  • Documentation is written (mandatory for new features)

🤖 Generated with Claude Code

…ntaining `LowCardinality`

Same bug as fixed for `concat` in ClickHouse#97654: `convertToFullIfNeeded` recursively
strips `LowCardinality` from inside compound column types like `Variant`, but
the argument type is not updated to match, so `SerializationLowCardinality`
tries to cast a non-LC column, triggering a LOGICAL_ERROR exception in
debug builds.

Fix by using only top-level column conversions (Const, Sparse, LowCardinality)
instead of the recursive `convertToFullIfNeeded`.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh bot commented Feb 24, 2026

Workflow [PR], commit [28fb460]

Summary:

@clickhouse-gh clickhouse-gh bot added pr-critical-bugfix pr-must-backport Pull request should be backported intentionally. Use this label with great care! labels Feb 24, 2026
@Algunenano
Copy link
Copy Markdown
Member Author

The fuzzer detected yet another issue introduced in #97493

@nihalzp nihalzp self-assigned this Feb 24, 2026
…ke `Tuple`

`convertToFullIfNeeded` (made recursive in ClickHouse#97493) strips `LowCardinality`
from inner subcolumns of compound types (Tuple, Array, etc.) in
`Set::insertFromColumns`, but `set_elements_types` in `Set::setHeader`
only stripped top-level `LowCardinality`. This created a column/type
mismatch: `ColumnVector<Int8>` in the column vs `LowCardinality(Int8)`
in the type.

When `KeyCondition::tryPrepareSetColumnsForIndex` unpacked the tuple to
build index conditions for `IN` subqueries, `castColumn` tried to
interpret the plain column as `ColumnLowCardinality`, causing a
LOGICAL_ERROR exception in debug builds.

Fix by using `recursiveRemoveLowCardinality` for `set_elements_types` in
both `Set::setHeader` and the static `Set::getElementTypes`, matching
what `convertToFullIfNeeded` does to columns.

https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=97831&sha=6f56284303c16ecc5f5a437491c661d5019f2977&name_0=PR&name_1=AST%20fuzzer%20%28amd_debug%29

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@Algunenano Algunenano changed the title Fix exception in concatWithSeparator and format with Variant containing LowCardinality Fix LOGICAL_ERROR exceptions caused by recursive convertToFullIfNeeded Feb 24, 2026
Algunenano and others added 2 commits February 24, 2026 14:52
…variant

Regression test for ClickHouse#97847:
`Dynamic` column with `LowCardinality` variant and `enable_join_runtime_filters`
caused LOGICAL_ERROR "Bad cast from type DB::ColumnVector<int> to
DB::ColumnLowCardinality" because `Set` stored columns with inner
`LowCardinality` stripped but `set_elements_types` still contained it inside
the `Dynamic` type. Already fixed by the previous commit.

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

`convertToFullIfNeeded` recursively strips `LowCardinality` from subcolumns,
including variant columns inside `ColumnDynamic`. But it cannot update
`ColumnDynamic::variant_info` type metadata, creating a mismatch: variant
columns have LC stripped from data while `variant_info` still references LC
types. This caused LOGICAL_ERROR in `Set::appendSetElements` when
`ColumnDynamic::insertRangeFrom` tried to serialize values using LC
serialization against non-LC column data.

Fix: override `convertToFullIfNeeded` in `ColumnDynamic` to return itself
without recursing into subcolumns. Dynamic is a self-contained typed container
that manages its own variant types.

Also add `max_threads=1` to the test for issue ClickHouse#97847 to reliably trigger the
runtime filter code path that exercises this bug.

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

Same issue as `ColumnDynamic`: `convertToFullIfNeeded` recursively strips
`LowCardinality` from `ColumnVariant`'s internal variant sub-columns, but
cannot update the corresponding `DataTypeVariant` (which sorts variants by
name). This creates column/type position mismatches that cause LOGICAL_ERROR
in `KeyCondition::tryPrepareSetColumnsForIndex` when casting set elements
for index conditions.

Fix: override `convertToFullIfNeeded` in `ColumnVariant` to return itself
without recursing into sub-columns, matching the approach for `ColumnDynamic`.

Closes ClickHouse#97854

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@Algunenano Algunenano changed the title Fix LOGICAL_ERROR exceptions caused by recursive convertToFullIfNeeded Fix LOGICAL_ERROR exceptions from recursive convertToFullIfNeeded with LowCardinality inside compound types Feb 24, 2026
@Algunenano Algunenano added this pull request to the merge queue Feb 25, 2026
Merged via the queue into ClickHouse:master with commit 7ad028f Feb 25, 2026
291 of 292 checks passed
@Algunenano Algunenano deleted the fix-concat-with-separator-format-variant-low-cardinality branch February 25, 2026 15:54
@robot-ch-test-poll2 robot-ch-test-poll2 added the pr-synced-to-cloud The PR is synced to the cloud repo label Feb 25, 2026
@robot-clickhouse-ci-2 robot-clickhouse-ci-2 added the pr-must-backport-synced The `*-must-backport` labels are synced into the cloud Sync PR label Feb 25, 2026
robot-ch-test-poll added a commit that referenced this pull request Feb 25, 2026
Cherry pick #97831 to 25.11: Fix LOGICAL_ERROR exceptions from recursive `convertToFullIfNeeded` with `LowCardinality` inside compound types
robot-clickhouse added a commit that referenced this pull request Feb 25, 2026
… `convertToFullIfNeeded` with `LowCardinality` inside compound types
robot-ch-test-poll added a commit that referenced this pull request Feb 25, 2026
Cherry pick #97831 to 25.12: Fix LOGICAL_ERROR exceptions from recursive `convertToFullIfNeeded` with `LowCardinality` inside compound types
robot-clickhouse added a commit that referenced this pull request Feb 25, 2026
… `convertToFullIfNeeded` with `LowCardinality` inside compound types
robot-ch-test-poll added a commit that referenced this pull request Feb 25, 2026
Cherry pick #97831 to 26.1: Fix LOGICAL_ERROR exceptions from recursive `convertToFullIfNeeded` with `LowCardinality` inside compound types
robot-clickhouse added a commit that referenced this pull request Feb 25, 2026
…`convertToFullIfNeeded` with `LowCardinality` inside compound types
robot-ch-test-poll added a commit that referenced this pull request Feb 25, 2026
Cherry pick #97831 to 26.2: Fix LOGICAL_ERROR exceptions from recursive `convertToFullIfNeeded` with `LowCardinality` inside compound types
robot-clickhouse added a commit that referenced this pull request Feb 25, 2026
…`convertToFullIfNeeded` with `LowCardinality` inside compound types
@robot-ch-test-poll3 robot-ch-test-poll3 added the pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore label Feb 25, 2026
clickhouse-gh bot added a commit that referenced this pull request Feb 25, 2026
Backport #97831 to 26.1: Fix LOGICAL_ERROR exceptions from recursive `convertToFullIfNeeded` with `LowCardinality` inside compound types
Algunenano added a commit that referenced this pull request Feb 26, 2026
Backport #97831 to 26.2: Fix LOGICAL_ERROR exceptions from recursive `convertToFullIfNeeded` with `LowCardinality` inside compound types
Algunenano added a commit that referenced this pull request Feb 27, 2026
…cursive `convertToFullIfNeeded` with `LowCardinality` inside compound types"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore pr-critical-bugfix pr-must-backport Pull request should be backported intentionally. Use this label with great care! pr-must-backport-synced The `*-must-backport` labels are synced into the cloud Sync PR pr-synced-to-cloud The PR is synced to the cloud repo

Projects

None yet

7 participants