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: avoid using index_merge when there are multiple table filters #22122

Merged
merged 1 commit into from
Dec 31, 2020

Conversation

XuHuaiyu
Copy link
Contributor

What problem does this PR solve?

Issue Number: close #22105

Problem Summary:

What is changed and how it works?

Proposal: xxx

What's Changed:
Avoid using index merge when there are multiple table filters.

How it Works:

Related changes

  • Need to cherry-pick to the release branch

Check List

Tests

  • Integration test

Side effects

N/A

Release note

  • Avoid using index merge when there are multiple table filters.

@XuHuaiyu XuHuaiyu added type/bugfix This PR fixes a bug. sig/planner SIG: Planner needs-cherry-pick-4.0 labels Dec 31, 2020
@XuHuaiyu XuHuaiyu requested a review from a team as a code owner December 31, 2020 04:14
@XuHuaiyu XuHuaiyu requested review from eurekaka and lzmhhh123 and removed request for a team December 31, 2020 04:14
@XuHuaiyu
Copy link
Contributor Author

/run-all-tests

for _, path := range partialPaths {
// IndexMerge should not be used when the SQL is like 'select x from t WHERE (key1=1 AND key2=2) OR (key1=4 AND key3=6);'.
// Check issue https://github.com/pingcap/tidb/issues/22105 for details.
Copy link
Contributor

@eurekaka eurekaka Dec 31, 2020

Choose a reason for hiding this comment

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

Actually we can use IndexMerge for this case, as long as we keep the whole filter (key1=1 and key2=2) or (key1=4 and key3=6) in the selection, that should be the original intention of #20425.

Copy link
Contributor

@eurekaka eurekaka left a comment

Choose a reason for hiding this comment

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

LGTM. We can fix the bug quickly by this PR and optimize the plan later.

@eurekaka eurekaka added the status/LGT1 Indicates that a PR has LGTM 1. label Dec 31, 2020
@XuHuaiyu
Copy link
Contributor Author

/run-all-tests

Copy link
Contributor

@qw4990 qw4990 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 Dec 31, 2020
@XuHuaiyu XuHuaiyu merged commit f1805f3 into pingcap:master Dec 31, 2020
@XuHuaiyu XuHuaiyu deleted the issue/22105 branch December 31, 2020 05:39
ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Dec 31, 2020
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot
Copy link
Contributor

cherry pick to release-4.0 in PR #22124

ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Dec 31, 2020
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot
Copy link
Contributor

cherry pick to release-5.0-rc in PR #22125

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/bugfix This PR fixes a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The result is not as expected when using the hint of use_index_merge
4 participants