Skip to content

Fix join runtime filter with Variant type#100182

Merged
alexey-milovidov merged 1 commit intomasterfrom
fix-join-runtime-filter-variant
Mar 20, 2026
Merged

Fix join runtime filter with Variant type#100182
alexey-milovidov merged 1 commit intomasterfrom
fix-join-runtime-filter-variant

Conversation

@novikd
Copy link
Copy Markdown
Member

@novikd novikd commented Mar 20, 2026

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

Changelog category (leave one):

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

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

Fix join runtime filters on top of the Variant data type.

Documentation entry for user-facing changes

  • Documentation is written (mandatory for new features)

@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh bot commented Mar 20, 2026 •
edited
Loading

Workflow [PR], commit [6452f44]

Summary: ✅

AI Review

Summary

This PR fixes runtime filter behavior for JOIN keys of type Variant by treating Variant as potentially nullable in RuntimeFilterBase, which avoids the incorrect single-value fast path (ValuesCount::ONE) and aligns the filter result type behavior. The fix is narrow, targeted, and accompanied by a reproducer test; I did not find blocker or major correctness/safety issues in the diff.

Missing context

  • ⚠️ No CI results for this exact PR run were provided in the review inputs.

ClickHouse Rules

Item Status Notes
Deletion logging ➖
Serialization versioning ➖
Core-area scrutiny ✅
No test removal ✅
Experimental gate ➖
No magic constants ✅
Backward compatibility ✅
SettingsChangesHistory.cpp ➖
Safe rollout ✅
Compilation time ✅

Final Verdict

  • Status: ✅ Approve

@clickhouse-gh clickhouse-gh bot added the pr-bugfix Pull request with bugfix, not backported by default label Mar 20, 2026
@novikd novikd mentioned this pull request Mar 20, 2026
Merged
1 task
@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh bot commented Mar 20, 2026

LLVM Coverage Report

Metric Baseline Current Δ
Lines 83.80% 83.80% +0.00%
Functions 24.00% 24.00% +0.00%
Branches 76.40% 76.30% -0.10%

PR changed lines: PR changed-lines coverage: 100.00% (7/7, 0 noise lines excluded)
Diff coverage report
Uncovered code

@alexey-milovidov alexey-milovidov self-assigned this Mar 20, 2026
@alexey-milovidov alexey-milovidov merged commit b79843b into master Mar 20, 2026
151 of 152 checks passed
@alexey-milovidov alexey-milovidov deleted the fix-join-runtime-filter-variant branch March 20, 2026 23:29
@robot-ch-test-poll4 robot-ch-test-poll4 added the pr-synced-to-cloud The PR is synced to the cloud repo label Mar 20, 2026
@clickgapai clickgapai mentioned this pull request Mar 31, 2026
Open
@clickgapai
Copy link
Copy Markdown
Contributor

clickgapai commented Mar 31, 2026

Hi @novikd @alexey-milovidov — while reviewing this PR I found the following:

Happy to discuss — close anything that's wrong or already addressed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alexey-milovidov alexey-milovidov alexey-milovidov approved these changes

Assignees

@alexey-milovidov alexey-milovidov

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@novikd @clickgapai @alexey-milovidov @robot-ch-test-poll4