feat(frontend): add LogicalMultijoin

[jon-chuang]

What's changed and what's your intention?

Adds LogicalMultiJoin PlanTreeNode.

In a follow up PR (in progress; passes e2e tests reliably but missing unit tests) we will add optimizer rules for converting a LogicalMultiJoin into a left-deep join tree with heuristic reordering to avoid cross joins (and prioritize e.g. joins with primary keys in join columns).

Refer to a related PR or issue link (optional)

#2425
#1866

[fuyufjh]

AFAIK, #1907 and #2425 are designed for two different purposes.

• frontend: support multi-way logical join #1907 is for multi-way delta join rewrite. Currently, the delta join only supports 2-way join, but we are working in progress to move the plan fragmenter from meta node to frontend, after that, it's not necessary to introduce a multi-join operator. You may talk to @skyzh and @st1page for details.
• Implement LogicalMultiJoin #2425 is for join reordering. To implement an heuristic join reordering algorithm, a common practice is to converting all inner joins to a single multi join operator and do reordering inside of it. In my opinion, perhaps we need an RFC/proposal of join reordering first. Let's have a discussion next week? @XieJiann @st1page

[jon-chuang]

Yes, agreed on the differences. This PR is meant more to solve the second issue. The LogicalMultiJoin plannode is just a placeholder and is only meant to last the lifetime of a few optimization passes. It should not be propagated beyond the frontend.

I will write up the approach I chose for the heuristic ordering in an RFC.

I think once this effort is complete we can see if this LogicalMultiJoin should be reused for delta join or there should be another abstraction.

[fuyufjh]

By the way, @XieJiann mentioned he was working on this. Did you had any communication?

#2425 (comment)

[jon-chuang]

Unfortunately not. I was enthusiastic to try out the second part, but it was blocking on the first part, and after some study, I think its not a trivial issue for a new contributor, so I decided to get started to get the ball rolling.

My apologies @XieJiann, if you had some ongoing work on it. However, I hope you can help with some of the subtasks and the RFC for heuristic join ordering #2530 if you have come up with any thoughts or solutions!

[skyzh]

I believe this representation is pretty okay even for multi-way delta join rewrite. 🤪 As far as I can see, it's easy make use of this logical plan to create a delta join plan.

Regarding join reordering, I think it's okay to have a function called .reorder(new_idx: &[usize]) on this to create a new plan node based on the new join order. What do you think? cc @fuyufjh

[codecov]

Codecov Report

Merging #2526 (408a7f9) into main (e4a0c3a) will increase coverage by 0.00%.
The diff coverage is 72.25%.

@@           Coverage Diff            @@
##             main    #2526    +/-   ##
========================================
  Coverage   71.33%   71.33%            
========================================
  Files         689      691     +2     
  Lines       88643    88816   +173     
========================================
+ Hits        63232    63357   +125     
- Misses      25411    25459    +48     

Flag	Coverage Δ
rust	71.33% <72.25%> (+<0.01%)	⬆️

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

Impacted Files	Coverage Δ
src/frontend/src/optimizer/plan_node/mod.rs	97.91% <ø> (ø)
...tend/src/optimizer/plan_node/logical_multi_join.rs	49.38% <49.38%> (ø)
src/frontend/src/optimizer/rule/multijoin_join.rs	92.39% <92.39%> (ø)

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

[fuyufjh]

I believe this representation is pretty okay even for multi-way delta join rewrite. 🤪 As far as I can see, it's easy make use of this logical plan to create a delta join plan.

Great!

Regarding join reordering, I think it's okay to have a function called .reorder(new_idx: &[usize]) on this to create a new plan node based on the new join order. What do you think? cc @fuyufjh

Agree, the join reordering rule (or else) should manipulate on the LogicalMultiJoin node

[skyzh]

LGTM from my side, and maybe other members can give some more comments.

[lmatz]

LGTM