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/cascades: add implementationRule for IndexLookUpJoin #14831

Closed
wants to merge 9 commits into from

Conversation

francis0407
Copy link
Member

What problem does this PR solve?

This PR adds a new ImplemenationRule ImplIndexJoin which will implement LogicalJoin as PhysicalIndexJoin, PhysicalIndexMergeJoin and PhysicalIndexHashJoin. (Although this PR only supports PhysicalIndexJoin).

What is changed and how it works?

This PR almost copies all of the codes from planner/core/exhaust_physical_plans.go/LogicalJoin.tryToGetIndexJoin, but makes some little changes according to the cascades framework.

For the convenience of the reviewers, this PR only supports build PhysicalIndexJoin which inner child is a TableReader. In other words, I haven't implemented PhysicalIndexMergeJoin, PhysicalIndexHashJoin and IndexJoins which inner child is an IndexReader. (But this PR still changes about 500 lines T.T ).

Check List

Tests

  • Unit test
  • No code

Code changes

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

Side effects

  • Possible performance regression
  • Increased code complexity

@francis0407 francis0407 requested a review from a team as a code owner February 18, 2020 08:35
@ghost ghost requested review from eurekaka and lzmhhh123 and removed request for a team February 18, 2020 08:35
@francis0407
Copy link
Member Author

/run-unit-test

@@ -222,17 +222,24 @@ func (opt *Optimizer) fillGroupStats(g *memo.Group) (err error) {
if g.Prop.Stats != nil {
return nil
}
// 1. Recursively fill the stats of all the child groups.
Copy link
Member Author

Choose a reason for hiding this comment

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

This change blocks #14857.

@codecov
Copy link

codecov bot commented Feb 24, 2020

Codecov Report

Merging #14831 into master will decrease coverage by 0.6122%.
The diff coverage is 93.5483%.

@@               Coverage Diff                @@
##             master     #14831        +/-   ##
================================================
- Coverage   80.7843%   80.1721%   -0.6123%     
================================================
  Files           505        499         -6     
  Lines        136852     129787      -7065     
================================================
- Hits         110555     104053      -6502     
+ Misses        17821      17481       -340     
+ Partials       8476       8253       -223     

@francis0407
Copy link
Member Author

This PR is in conflict with #15090 . Not ready for review until #15090 get merged.

@lzmhhh123 lzmhhh123 removed their request for review March 17, 2020 08:46
…dex_hash_join

# Conflicts:
#	planner/cascades/testdata/integration_suite_in.json
#	planner/cascades/testdata/integration_suite_out.json
#	planner/core/explain.go
#	planner/core/logical_plans.go
francis0407 added a commit to francis0407/tidb that referenced this pull request Apr 8, 2020
francis0407 added a commit to francis0407/tidb that referenced this pull request Apr 8, 2020
@zz-jason
Copy link
Member

zz-jason commented Feb 9, 2021

I'm going to close this PR since it hasn't been updated for a long time. feel free to reopen if you want to continue with it. thank you for your contribution.

@zz-jason zz-jason closed this Feb 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig/planner SIG: Planner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants