Optimizer: Refactor and simplify outer to inner join conversion rule #52941 | tidb-test=pr/2321 #52941
Conversation
ti-chi-bot
bot
added
do-not-merge/invalid-title
release-note-none
|
Hi @ghazalfamilyusa. Thanks for your PR. PRs from untrusted users cannot be marked as trusted with I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
ghazalfamilyusa
changed the title
ti-chi-bot
bot
removed
do-not-merge/invalid-title
needs-rebase
ghazalfamilyusa
force-pushed
the
outer2inner
branch
2 times, most recently
from
a3969db to
b43e840
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #52941 +/- ##
================================================
+ Coverage 72.4943% 73.5124% +1.0180%
================================================
Files 1493 1498 +5
Lines 429359 434713 +5354
================================================
+ Hits 311261 319568 +8307
+ Misses 98889 95083 -3806
- Partials 19209 20062 +853
Flags with carried forward coverage won't be shown. Click here to find out more.
|
ghazalfamilyusa
force-pushed
the
outer2inner
branch
2 times, most recently
from
3d6981a to
6dbbc8c
Compare
|
/retest |
|
@ghazalfamilyusa: Cannot trigger testing until a trusted user reviews the PR and leaves an In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
ghazalfamilyusa
force-pushed
the
outer2inner
branch
from
6dbbc8c to
cf495b1
Compare
|
/retest |
|
@ghazalfamilyusa: Cannot trigger testing until a trusted user reviews the PR and leaves an In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
ghazalfamilyusa
force-pushed
the
outer2inner
branch
2 times, most recently
from
6833fad to
ecba825
Compare
|
/retest |
|
@ghazalfamilyusa: Cannot trigger testing until a trusted user reviews the PR and leaves an In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
ghazalfamilyusa
force-pushed
the
outer2inner
branch
from
ecba825 to
e60ffff
Compare
ti-chi-bot
bot
added
the
do-not-merge/work-in-progress
ghazalfamilyusa
force-pushed
the
outer2inner
branch
from
e60ffff to
4d45c03
Compare
ti-chi-bot
bot
removed
the
do-not-merge/work-in-progress
ghazalfamilyusa
force-pushed
the
outer2inner
branch
3 times, most recently
from
588bde2 to
c2978ba
Compare
ghazalfamilyusa
force-pushed
the
outer2inner
branch
from
c2978ba to
400174a
Compare
| @@ -73,16 +73,16 @@ | |||
| "Join{DataScan(t1)->DataScan(t2)}(test.t.a,test.t.b)(test.t.d,test.t.d)->Projection", | |||
| "Join{DataScan(ta)->DataScan(tb)}(test.t.d,test.t.d)->Projection", | |||
| "Join{DataScan(ta)->DataScan(tb)}(test.t.d,test.t.d)->Projection", | |||
| "Join{DataScan(ta)->DataScan(tb)}(test.t.d,test.t.d)->Sel([eq(test.t.a, 0)])->Projection", | |||
There was a problem hiding this comment.
Why this output changed a lot ? I saw you didn't change any case of this in.json ?
There was a problem hiding this comment.
The change is probably of more chances to predicate push down after outer to inner join rule.
There was a problem hiding this comment.
@elsa0520 : actually some tests are missing flagConvertOuterToInnerJoin whcih cause some joins not converted and different push down. Will fix the tests by adding flagConvertOuterToInnerJoin to the plan tests.
ghazalfamilyusa
force-pushed
the
outer2inner
branch
from
400174a to
15a2ba2
Compare
ti-chi-bot
bot
added
lgtm
and removed
needs-1-more-lgtm
|
/retest |
|
@qw4990: Cannot trigger testing until a trusted user reviews the PR and leaves an In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: elsa0520, qw4990, XuHuaiyu The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What problem does this PR solve?
Issue Number: Ref #53278
Problem Summary:
Currently, the outer to inner join rewrite is part of the predicate push down. This is causing confusion and difficulty in extending of both rewrites. Also, the current implementation of outer to inner join rewrite has unnecessary special cases.
What changed and how does it work?
A new logical optimization is added to the list of rules. Also, a simple solution of null filtering (or NF) predicate check is used.
The solution is simply:
The scope covers previous solution plus some more with the generalized solution. mysql tests has two diffs and both are OK to promote. A separate PR will be used for that.
Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.