Skip to content

refactor: exhaustively destructure dynamic filter and scalar subquery proto hooks - #25178

Open
peterxcli wants to merge 1 commit into
apache:mainfrom
peterxcli:refactor/runtime-expression-proto-hooks
Open

refactor: exhaustively destructure dynamic filter and scalar subquery proto hooks#25178
peterxcli wants to merge 1 commit into
apache:mainfrom
peterxcli:refactor/runtime-expression-proto-hooks

Conversation

@peterxcli

Copy link
Copy Markdown
Member

Which issue does this PR close?

Closes #24618.

Rationale for this change

This change makes the compiler catch fields that serialization does not handle. When a field is added to a dynamic filter, scalar subquery expression, or its protobuf payload, the corresponding hook must account for it. This helps prevent new fields from being silently dropped when expressions are encoded and decoded.

What changes are included in this PR?

All four encoding and decoding hooks for DynamicFilterPhysicalExpr and ScalarSubqueryExpr explicitly list every field without a rest pattern. Dynamic filter encoding also destructures the snapshot of its shared inner state, and decoding handles the expression ID carried by the enclosing message.

Comments explain why caches, watch channels, cached type and nullability checks, and subquery results are omitted, including how they are rebuilt or supplied.

The PR also replaces one use of unstable std::assert_matches in an existing execution test with assert!(…is_ok()), allowing the required checks to run on the pinned stable compiler.

What is the testing strategy for this PR?

Existing tests cover filter identity, shared updates, remapped children, subquery metadata, nested result scopes, and execution after serialization. The physical expression suite passed 1,739 tests with 2 ignored, and all 13 relevant protobuf integration tests passed.

The full extended workspace run passed 11,591 Rust tests, with 8 ignored, and completed all 516 SQLLogicTest files. Formatting, Clippy with all targets and features and warnings denied, and the complete ./dev/rust_lint.sh suite also passed.

Are there any user-facing changes?

No.

@github-actions github-actions Bot added physical-expr Changes to the physical-expr crates execution Related to the execution crate labels Sep 10, 2026
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.00000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.90%. Comparing base (4cee381) to head (8eaf2bb).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...ysical-expr/src/expressions/dynamic_filters/mod.rs 86.66% 0 Missing and 4 partials ⚠️
datafusion/physical-expr/src/scalar_subquery.rs 89.47% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main   #25178   +/-   ##
=======================================
  Coverage   81.90%   81.90%           
=======================================
  Files        1132     1132           
  Lines      420940   420958   +18     
  Branches   420940   420958   +18     
=======================================
+ Hits       344786   344805   +19     
+ Misses      55771    55770    -1     
  Partials    20383    20383           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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

Labels

execution Related to the execution crate physical-expr Changes to the physical-expr crates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Destructure proto hooks for dynamic filter and scalar subquery expressions

2 participants