Skip to content

Conversation

@xxchan
Copy link
Collaborator

@xxchan xxchan commented Mar 15, 2022

What's changed and what's your intention?

  • turn on HeuristicOptimizer (only filter join rule now)
  • fix binary nodes' inputs_distribution/order_required, and remove default impls (not used)
  • added one test for filter join

Checklist

  • I have written necessary docs and comments
  • I have added necessary unit tests and integration tests

Refer to a related PR or issue link (optional)

@github-actions github-actions bot added the type/feature Type: New feature. label Mar 15, 2022
@xxchan xxchan enabled auto-merge (squash) March 15, 2022 11:13
@xxchan xxchan requested a review from fuyufjh March 15, 2022 11:14
@codecov
Copy link

codecov bot commented Mar 15, 2022

Codecov Report

Merging #942 (85a0a70) into main (158b03e) will increase coverage by 0.32%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##               main     #942      +/-   ##
============================================
+ Coverage     71.49%   71.81%   +0.32%     
  Complexity     2766     2766              
============================================
  Files           970      971       +1     
  Lines         56587    56636      +49     
  Branches       1787     1787              
============================================
+ Hits          40457    40674     +217     
+ Misses        15240    15072     -168     
  Partials        890      890              
Flag Coverage Δ
java 61.23% <ø> (ø)
rust 75.90% <100.00%> (+0.44%) ⬆️

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

Impacted Files Coverage Δ
rust/frontend/src/optimizer/plan_pass/mod.rs 0.00% <ø> (ø)
rust/frontend/src/optimizer/rule/filter_join.rs 93.66% <ø> (+30.28%) ⬆️
rust/frontend/src/optimizer/mod.rs 96.15% <100.00%> (+10.43%) ⬆️
...frontend/src/optimizer/plan_node/plan_tree_node.rs 78.00% <100.00%> (+67.28%) ⬆️
rust/frontend/src/optimizer/plan_pass/heuristic.rs 92.00% <100.00%> (+92.00%) ⬆️
rust/frontend/src/optimizer/plan_node/mod.rs 36.00% <0.00%> (-0.74%) ⬇️
rust/common/src/types/ordered_float.rs 25.49% <0.00%> (-0.34%) ⬇️
rust/connector/src/kafka/source/reader.rs 0.00% <0.00%> (ø)
rust/connector/src/kafka/source/message.rs 0.00% <0.00%> (ø)
rust/connector/src/pulsar/source/reader.rs 0.00% <0.00%> (ø)
... and 25 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@xxchan xxchan requested a review from st1page March 15, 2022 11:39
Copy link
Collaborator

@fuyufjh fuyufjh left a comment

Choose a reason for hiding this comment

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

LGTM


/// `PlanPass` receive a plan tree and transform it to a new one.
trait PlanPass {
pub trait PlanPass {
Copy link
Collaborator

Choose a reason for hiding this comment

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

(unrelated to this PR) The trait PlanPass seems somehow over-designed to me. It mostly fits the volcano optimizer, but not so suitable for heuristic optimizer or visitors (like Pruning columns). Furthermore, we don't need to organize these 'phases' into a Vec or something. I would recommend removing it now and add back if necessary someday.

@xxchan xxchan merged commit 86198d7 into main Mar 15, 2022
@xxchan xxchan deleted the apply_rule branch March 15, 2022 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type/feature Type: New feature.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants