Skip to content

plan/executor: handle optimizer hint properly for cartesian join#9037

Merged
eurekaka merged 5 commits intopingcap:masterfrom
eurekaka:merge_join_cartesian
Jan 14, 2019
Merged

plan/executor: handle optimizer hint properly for cartesian join#9037
eurekaka merged 5 commits intopingcap:masterfrom
eurekaka:merge_join_cartesian

Conversation

@eurekaka
Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Fix #9032

What is changed and how it works?

  • allow merge join executor to handle cartesian product;
  • raise warning and choose other join implementations if TIDB_INLJ is specified;

Check List

Tests

  • Unit test

Code changes

N/A

Side effects

N/A

Related changes

  • Need to cherry-pick to the release branch

@eurekaka eurekaka added type/bugfix This PR fixes a bug. sig/planner SIG: Planner sig/execution SIG execution labels Jan 14, 2019
@eurekaka
Copy link
Copy Markdown
Contributor Author

/run-all-tests

Copy link
Copy Markdown
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 the status/LGT1 Indicates that a PR has LGTM 1. label Jan 14, 2019
Comment thread planner/core/exhaust_physical_plans.go Outdated
Comment thread executor/merge_join_test.go
@eurekaka
Copy link
Copy Markdown
Contributor Author

/run-unit-test

@eurekaka
Copy link
Copy Markdown
Contributor Author

Comments addressed, @XuHuaiyu PTAL

@codecov-io
Copy link
Copy Markdown

codecov-io commented Jan 14, 2019

Codecov Report

Merging #9037 into master will decrease coverage by 0.02%.
The diff coverage is 83.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9037      +/-   ##
==========================================
- Coverage   67.28%   67.26%   -0.03%     
==========================================
  Files         371      371              
  Lines       75930    75920      -10     
==========================================
- Hits        51092    51070      -22     
- Misses      20332    20346      +14     
+ Partials     4506     4504       -2
Impacted Files Coverage Δ
executor/merge_join.go 81.5% <0%> (ø) ⬆️
planner/core/exhaust_physical_plans.go 92.1% <100%> (+0.03%) ⬆️
ddl/delete_range.go 74.28% <0%> (-4.58%) ⬇️
planner/core/planbuilder.go 49.07% <0%> (-1.05%) ⬇️
executor/index_lookup_join.go 77.28% <0%> (-0.64%) ⬇️
executor/executor.go 66.66% <0%> (-0.15%) ⬇️
planner/core/logical_plan_builder.go 74.98% <0%> (+0.25%) ⬆️
executor/join.go 78.44% <0%> (+0.51%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9f346a3...836e32f. Read the comment docs.

@eurekaka eurekaka requested a review from XuHuaiyu January 14, 2019 09:55
Copy link
Copy Markdown
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 XuHuaiyu added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Jan 14, 2019
@eurekaka eurekaka merged commit 4a901db into pingcap:master Jan 14, 2019
@eurekaka eurekaka deleted the merge_join_cartesian branch January 14, 2019 10:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sig/execution SIG execution sig/planner SIG: Planner status/LGT2 Indicates that a PR has LGTM 2. type/bugfix This PR fixes a bug.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants