FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

refactor: unify expression and operator traversal with visitors by KKould · Pull Request #372 · KipData/KiteSQL · GitHub

/ KiteSQL Public

refactor: unify expression and operator traversal with visitors - #372

Merged
KKould merged 3 commits into
mainfrom
refactor/operator-expression-visitors
Jul 11, 2026
ConversationCommits (3)ChecksFiles changed
Merged

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

Conversation

KKould commented Jul 11, 2026 •
edited
Loading

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 self-assigned this Jul 11, 2026
KKould added enhancement New feature or request test New test labels Jul 11, 2026

codecov Bot commented Jul 11, 2026 •
edited
Loading

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 force-pushed the refactor/operator-expression-visitors branch from d7018b1 to 213c8f8 Compare July 11, 2026 04:08
KKould merged commit dec3643 into main Jul 11, 2026
10 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

KKould

Labels

enhancement New feature or request test New test

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

Back | FazBrowse Home | New Git URL