Skip to content

refactor: unify expression and operator traversal with visitors#372

Merged
KKould merged 3 commits into
mainfrom
refactor/operator-expression-visitors
Jul 11, 2026
Merged

refactor: unify expression and operator traversal with visitors#372
KKould merged 3 commits into
mainfrom
refactor/operator-expression-visitors

Conversation

@KKould

@KKould KKould commented Jul 11, 2026

Copy link
Copy Markdown
Member

What problem does this PR solve?

Expression and operator traversal was duplicated across binders and optimizer rules, making new operator variants easy to miss and error propagation inconsistent.

Issue link: N/A

What is changed and how it works?

  • Rename the expression visitor traits to ExprVisitor and ExprVisitorMut.
  • Add decoupled OperatorVisitor and OperatorVisitorMut traits for operator dispatch.
  • Add concrete OperatorExprVisitor and OperatorExprVisitorMut adapters for traversing expressions embedded in operators.
  • Replace manual operator-expression traversal in evaluator binding, constant calculation, column pruning, and related binder/optimizer paths.
  • Propagate DatabaseError through referenced-column and expression inspection APIs.
  • Simplify predicate pushdown classification without intermediate partition vectors.
  • Add edge-case, visitor, parameterized-index, operator, and binder E2E coverage tests.

Code changes

  • Has Rust code change
  • Has CI related scripts change

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (make test-all, make codecov, and strict Clippy)
  • No code

Side effects

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

@KKould KKould self-assigned this Jul 11, 2026
@KKould KKould added enhancement New feature or request test New test labels Jul 11, 2026
@codecov

codecov Bot commented Jul 11, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.93815% with 50 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.74%. Comparing base (a28b4ab) to head (498d051).

Files with missing lines Patch % Lines
src/optimizer/rule/normalization/column_pruning.rs 84.54% 17 Missing ⚠️
src/binder/select.rs 84.31% 8 Missing ⚠️
src/planner/operator/visitor.rs 98.87% 5 Missing ⚠️
...mizer/rule/normalization/compilation_in_advance.rs 96.72% 4 Missing ⚠️
src/planner/operator/mod.rs 97.20% 4 Missing ⚠️
...ptimizer/rule/normalization/pushdown_predicates.rs 97.16% 3 Missing ⚠️
src/db.rs 75.00% 2 Missing ⚠️
src/optimizer/rule/normalization/elimination.rs 93.33% 2 Missing ⚠️
...ptimizer/rule/normalization/parameterized_index.rs 93.54% 2 Missing ⚠️
src/serdes/data_value.rs 96.66% 2 Missing ⚠️
... and 1 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #372      +/-   ##
==========================================
+ Coverage   92.09%   92.74%   +0.64%     
==========================================
  Files         241      243       +2     
  Lines       39870    41025    +1155     
==========================================
+ Hits        36720    38049    +1329     
+ Misses       3150     2976     -174     
Flag Coverage Δ
rust 92.74% <96.93%> (+0.64%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/binder/aggregate.rs 97.59% <100.00%> (ø)
src/binder/distinct.rs 98.47% <ø> (ø)
src/binder/parser.rs 86.96% <100.00%> (ø)
src/catalog/column.rs 95.70% <100.00%> (ø)
src/catalog/view.rs 94.44% <100.00%> (+1.58%) ⬆️
src/expression/mod.rs 84.93% <100.00%> (ø)
src/expression/range_detacher.rs 93.64% <100.00%> (+0.29%) ⬆️
src/expression/simplify.rs 87.44% <ø> (ø)
src/expression/visitor.rs 92.65% <100.00%> (ø)
src/expression/visitor_mut.rs 93.64% <100.00%> (ø)
... and 18 more
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@KKould KKould force-pushed the refactor/operator-expression-visitors branch from d7018b1 to 213c8f8 Compare July 11, 2026 04:08
@KKould KKould merged commit dec3643 into main Jul 11, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request test New test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant