Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

planner: support join elimination rule #8021

Merged
merged 29 commits into from
Nov 9, 2018

Conversation

lzmhhh123
Copy link
Contributor

What problem does this PR solve?

support join elimination rule #7704

What is changed and how it works?

Add a logical optimize rule:

  1. outer join elimination: For example left outer join, if the parent only use the columns from left table and the join key of right table(the inner table) is a unique key of the right table. the left outer join can be eliminated.
  2. outer join elimination with distinct: For example left outer join. If the parent only use the columns from left table with 'distinct' label. The left outer join can be eliminated.

Check List

Tests

  • Unit test
  • Integration test

Code changes

  • Has exported function/method change
  • Has exported variable/fields change

Side effects

  • Increased code complexity

@lzmhhh123 lzmhhh123 added type/enhancement The issue or PR belongs to an enhancement. status/WIP sig/planner SIG: Planner labels Oct 23, 2018
@lzmhhh123
Copy link
Contributor Author

/run-all-tests

planner/core/optimizer.go Outdated Show resolved Hide resolved
planner/core/rule_join_elimination.go Outdated Show resolved Hide resolved
planner/core/rule_join_elimination.go Outdated Show resolved Hide resolved
planner/core/rule_join_elimination.go Outdated Show resolved Hide resolved
planner/core/rule_join_elimination.go Outdated Show resolved Hide resolved
planner/core/rule_join_elimination.go Outdated Show resolved Hide resolved
@lzmhhh123
Copy link
Contributor Author

@zz-jason @winoros PTAL.

planner/core/rule_join_elimination.go Outdated Show resolved Hide resolved
planner/core/rule_join_elimination.go Outdated Show resolved Hide resolved
planner/core/rule_join_elimination.go Outdated Show resolved Hide resolved
planner/core/rule_join_elimination.go Outdated Show resolved Hide resolved
planner/core/optimizer.go Outdated Show resolved Hide resolved
@zz-jason
Copy link
Member

zz-jason commented Nov 2, 2018

LGTM

@zz-jason zz-jason added the status/LGT1 Indicates that a PR has LGTM 1. label Nov 2, 2018
@zz-jason
Copy link
Member

zz-jason commented Nov 2, 2018

@winoros @lamxTyler @XuHuaiyu @eurekaka PTAL

planner/core/rule_join_elimination.go Outdated Show resolved Hide resolved
planner/core/rule_join_elimination.go Outdated Show resolved Hide resolved
planner/core/rule_join_elimination.go Outdated Show resolved Hide resolved
planner/core/rule_join_elimination.go Outdated Show resolved Hide resolved
planner/core/rule_join_elimination.go Outdated Show resolved Hide resolved
planner/core/rule_join_elimination.go Outdated Show resolved Hide resolved
@XuHuaiyu
Copy link
Contributor

XuHuaiyu commented Nov 5, 2018

Will it be better to not store o.cols and o.schemas,
it seems we can put them as parameters?

planner/core/optimizer.go Outdated Show resolved Hide resolved
planner/core/rule_join_elimination.go Outdated Show resolved Hide resolved
planner/core/rule_join_elimination.go Outdated Show resolved Hide resolved
planner/core/rule_join_elimination.go Outdated Show resolved Hide resolved
planner/core/rule_join_elimination.go Show resolved Hide resolved
planner/core/rule_join_elimination.go Outdated Show resolved Hide resolved
planner/core/rule_join_elimination.go Outdated Show resolved Hide resolved
planner/core/rule_join_elimination.go Outdated Show resolved Hide resolved
}
}
if joinKeysContainIndex {
return p.children[1^innerChildIdx]
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we remove other possible paths in this case?

planner/core/logical_plan_test.go Show resolved Hide resolved
@XuHuaiyu
Copy link
Contributor

XuHuaiyu commented Nov 6, 2018

Please check whether this would be more readable?

rule_join_elimination.txt

planner/core/rule_join_elimination.go Show resolved Hide resolved
planner/core/rule_join_elimination.go Outdated Show resolved Hide resolved
planner/core/rule_join_elimination.go Outdated Show resolved Hide resolved
Copy link
Contributor

@XuHuaiyu XuHuaiyu left a comment

Choose a reason for hiding this comment

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

LGTM

@XuHuaiyu
Copy link
Contributor

XuHuaiyu commented Nov 9, 2018

PTAL @zz-jason @eurekaka

joinPlan.JoinType = LeftOuterJoin
resetNotNullFlag(joinPlan.schema, leftPlan.Schema().Len(), joinPlan.schema.Len())
case ast.RightJoin:
// right outer join need to be checked elimination
b.optFlag = b.optFlag | flagEliminateOuterJoin
joinPlan.JoinType = RightOuterJoin
resetNotNullFlag(joinPlan.schema, 0, leftPlan.Schema().Len())
default:
Copy link
Member

Choose a reason for hiding this comment

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

In the optimization phase, the semi joins are also considered, I think we should add the optimization flag for these semi joins in here.

Copy link
Member

Choose a reason for hiding this comment

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

and please add some explain tests to cover these optimizations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After my seriously considering. I think semi-joins can't be eliminated in any case. So I will remove semi-join in the eliminating rule.

Copy link
Member

@zz-jason zz-jason left a comment

Choose a reason for hiding this comment

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

LGTM

@zz-jason zz-jason added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Nov 9, 2018
@zz-jason
Copy link
Member

zz-jason commented Nov 9, 2018

/run-all-tests

@zz-jason zz-jason merged commit cd5ffb3 into pingcap:master Nov 9, 2018
@lzmhhh123 lzmhhh123 assigned lzmhhh123 and unassigned lzmhhh123 Nov 9, 2018
@lzmhhh123 lzmhhh123 deleted the dev/join_elimination branch November 13, 2018 06:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig/planner SIG: Planner status/LGT2 Indicates that a PR has LGTM 2. type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants