Skip to content

Fix null-safe comparison exception with const Dynamic/Variant columns#98553

Merged
alexey-milovidov merged 4 commits intomasterfrom
fix-null-safe-cmp-dynamic-const
Mar 3, 2026
Merged

Fix null-safe comparison exception with const Dynamic/Variant columns#98553
alexey-milovidov merged 4 commits intomasterfrom
fix-null-safe-cmp-dynamic-const

Conversation

@alexey-milovidov
Copy link
Copy Markdown
Member

Summary

  • Fixed exception Bad cast from type ColumnConst to ColumnDynamic when using <=> / IS NOT DISTINCT FROM with a const Dynamic or Variant column vs NULL. The column is now unwrapped via convertToFullColumnIfConst before casting.
  • Fixed a pre-existing bug where IS DISTINCT FROM with Dynamic/Variant vs NULL always returned 0 due to ele_is_null && false always evaluating to false.

Found by AST fuzzer (MSan): https://s3.amazonaws.com/clickhouse-test-reports/json.html?REF=master&sha=337832154ed77a0f80ca69c428a5932fb7212c77&name_0=MasterCI&name_1=AST%20fuzzer%20%28amd_msan%29

Test plan

  • Added regression test 04010_null_safe_cmp_dynamic_const
  • Verified fix with debug build locally

Changelog category:

  • Bug Fix (user-visible misbehavior in an official stable release)

Changelog entry:

Fixed exception Bad cast from type ColumnConst to ColumnDynamic in null-safe comparison (<=> / IS NOT DISTINCT FROM) with const Dynamic or Variant columns and NULL. Also fixed IS DISTINCT FROM with Dynamic/Variant vs NULL always incorrectly returning 0.

🤖 Generated with Claude Code

The `executeForVariantOrDynamicAndNull` function tried to cast
columns directly to `ColumnDynamic`/`ColumnVariant`, but the column
could be wrapped in `ColumnConst` when only one argument is constant.
Fixed by unwrapping via `convertToFullColumnIfConst` before the cast.

Also fixed a pre-existing bug where `IS DISTINCT FROM` with
Dynamic/Variant vs NULL always returned 0 due to `ele_is_null && false`
always evaluating to `false`.

https://s3.amazonaws.com/clickhouse-test-reports/json.html?REF=master&sha=337832154ed77a0f80ca69c428a5932fb7212c77&name_0=MasterCI&name_1=AST%20fuzzer%20%28amd_msan%29

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

clickhouse-gh bot commented Mar 2, 2026

Workflow [PR], commit [b7342a2]

Summary:

@clickhouse-gh clickhouse-gh bot added the pr-bugfix Pull request with bugfix, not backported by default label Mar 2, 2026
{
bool ele_is_null = column_variant_or_dynamic.isNullAt(i);
data[i] = is_equal_mode ? ele_is_null && true : ele_is_null && false;
data[i] = is_equal_mode ? ele_is_null : !ele_is_null;
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Who writes "element" as "ele"? 🤢

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No wonder this code has a bug.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
alexey-milovidov and others added 2 commits March 2, 2026 22:06
The fix for `IS DISTINCT FROM` with `Dynamic`/`Variant` vs `NULL`
changes the result from always 0 to correctly returning 1 for non-null
values. Update the existing test reference to reflect the correct
behavior.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@antaljanosbenjamin antaljanosbenjamin self-assigned this Mar 3, 2026
@alexey-milovidov alexey-milovidov merged commit 54b5870 into master Mar 3, 2026
148 of 149 checks passed
@alexey-milovidov alexey-milovidov deleted the fix-null-safe-cmp-dynamic-const branch March 3, 2026 17:57
@robot-clickhouse-ci-1 robot-clickhouse-ci-1 added the pr-synced-to-cloud The PR is synced to the cloud repo label Mar 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-bugfix Pull request with bugfix, not backported by default pr-synced-to-cloud The PR is synced to the cloud repo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants