Skip to content

executor: disable runtime filter on join key type mismatch#10698

Open
ChangRui-Ryan wants to merge 1 commit intopingcap:masterfrom
ChangRui-Ryan:runtime_filter
Open

executor: disable runtime filter on join key type mismatch#10698
ChangRui-Ryan wants to merge 1 commit intopingcap:masterfrom
ChangRui-Ryan:runtime_filter

Conversation

@ChangRui-Ryan
Copy link
Contributor

@ChangRui-Ryan ChangRui-Ryan commented Feb 2, 2026

What problem does this PR solve?

Issue Number: close #10699

Problem Summary:

What is changed and how it works?


Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No code

Side effects

  • Performance regression: Consumes more CPU
  • Performance regression: Consumes more Memory
  • Breaking backward compatibility

Documentation

  • Affects user behaviors
  • Contains syntax changes
  • Contains variable changes
  • Contains experimental features
  • Changes MySQL compatibility

Release note

None

Summary by CodeRabbit

  • Bug Fixes

    • Runtime filters are now conservatively disabled when join key types, signed/unsigned flags, or decimal precision/scale differ; an informational log records the reason and empty filter lists are used to avoid incorrect pruning and preserve correct join results.
  • Tests

    • Added tests verifying runtime filters are disabled for various type/precision mismatch scenarios and that execution plans and results remain unchanged.

@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Feb 2, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign schrodingerzhu for approval. For more information see the Code Review Process.
Please ensure that each of them provides their approval before proceeding.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ti-chi-bot ti-chi-bot bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Feb 2, 2026
@coderabbitai
Copy link

coderabbitai bot commented Feb 2, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds a conservative runtime-filter compatibility guard in the join planner that disables RF generation when join key protobuf field types, integer signed/unsigned flags, or decimal precision/scale differ. Adds GoogleTest cases verifying RF is not used/pruned when such mismatches occur.

Changes

Cohort / File(s) Summary
Runtime Filter Type Guard
dbms/src/Flash/Planner/Plans/PhysicalJoin.cpp
Add is_join_key_field_type_compatible cheap check (field type, signed/unsigned, decimal precision/scale) to decide whether to call genRuntimeFilterList; substitute an empty runtime_filter_list and emit an informational log when incompatible. Add #include <Flash/Coprocessor/DAGUtils.h> and explanatory comments.
Runtime Filter Type Mismatch Tests
dbms/src/Flash/tests/gtest_runtime_filter_disable_on_type_mismatch.cpp
Add RuntimeFilterDisableOnTypeMismatchTestRunner (derives from ExecutorTest) and tests covering: baseline, Int32 vs Int64 mismatch, Int32 vs UInt32 signed/unsigned mismatch, and Decimal precision/scale mismatch. Includes macros to run across pipeline modes, DeltaMerge/right-exchange setups, and uses a mock runtime filter to assert no pruning and identical plans/row counts when RF disabled.

Sequence Diagram(s)

(Skipped)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐇 I sniffed the left key, then the right,

When types or scales sparked doubt in sight,
I folded my filter, soft and light,
Let every row hop through just right,
A cautious twitch keeps joins polite.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'executor: disable runtime filter on join key type mismatch' clearly and specifically describes the main change: disabling runtime filters when join key types don't match.
Description check ✅ Passed The PR description follows the required template with Issue Number (#10699), Test checklist (Unit test and Integration test marked), and Release note sections completed.
Linked Issues check ✅ Passed The code changes directly address issue #10699 by implementing a runtime filter enablement guard that disables RF when join key types are incompatible (signed/unsigned mismatches and decimal precision mismatches), preventing incorrect query results.
Out of Scope Changes check ✅ Passed All changes are in-scope: PhysicalJoin.cpp adds type compatibility checks to conditionally disable runtime filters, and the new test file validates the feature works correctly for type mismatch scenarios.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@dbms/src/Flash/Planner/Plans/PhysicalJoin.cpp`:
- Around line 173-212: Rename the snake_case local symbols to camelCase: change
the lambda is_join_key_field_type_compatible to isJoinKeyFieldTypeCompatible,
the bool enable_runtime_filter to enableRuntimeFilter, and runtime_filter_list
to runtimeFilterList; update the lambda invocation and its use sites (the const
bool assignment and the ternary that builds runtimeFilterList using
tiflash_join.genRuntimeFilterList) so all references match the new camelCase
names and keep behavior unchanged.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@dbms/src/Flash/tests/gtest_runtime_filter_disable_on_type_mismatch.cpp`:
- Around line 43-49: The loop variable name in the WrapForRuntimeFilterTestBegin
macro is declared as enablePipeline but the test code uses enable_pipeline,
causing a compile error; fix by making the identifier consistent—either change
the macro's loop variable to enable_pipeline or update all references (the
usages currently named enable_pipeline) to enablePipeline so the symbol matches,
ensuring WrapForRuntimeFilterTestBegin and all occurrences use the same
identifier.
🧹 Nitpick comments (2)
dbms/src/Flash/Planner/Plans/PhysicalJoin.cpp (1)

161-199: Conservative guard logic looks correct; minor style nit on redundant continue.

The lambda correctly checks:

  1. Equal number of join keys
  2. Both keys have valid field types
  3. Same protobuf type (tp())
  4. Same unsigned flag

One minor observation: the continue; on line 196 is redundant as it's at the end of the for-loop body.

🧹 Optional: remove redundant continue
             // Otherwise keep conservative: if we got here (same tp + same unsigned flag), treat as compatible.
-            continue;
         }
         return true;
     };
dbms/src/Flash/tests/gtest_runtime_filter_disable_on_type_mismatch.cpp (1)

43-49: Use the existing WRAP_FOR_TEST_BEGIN and WRAP_FOR_TEST_END macros from ExecutorTestUtils.h.

The file already imports ExecutorTestUtils.h and extends ExecutorTest, making it an executor test that should follow the established pattern. The custom WrapForRuntimeFilterTestBegin/End macros duplicate the existing WRAP_FOR_TEST_BEGIN/END macros and are already used consistently throughout other executor tests in dbms/src/Flash/tests/. Replace lines 43-49 with the standard macros to maintain consistency with the codebase pattern.

@ChangRui-Ryan ChangRui-Ryan force-pushed the runtime_filter branch 4 times, most recently from 12f0add to 8dfcf74 Compare February 3, 2026 03:10
@ChangRui-Ryan
Copy link
Contributor Author

/retest

1 similar comment
@ChangRui-Ryan
Copy link
Contributor Author

/retest

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

Labels

do-not-merge/needs-triage-completed release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

May miss rows when joining different key types with the Runtime Filter.

1 participant