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.

Sign up for GitHub

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

Jump to bottom

planner: disable index merge join temporarily #20599

Merged
merged 5 commits into from
Oct 27, 2020
Merged

planner: disable index merge join temporarily #20599

merged 5 commits into from
Oct 27, 2020

Conversation

lzmhhh123
Copy link
Contributor

@lzmhhh123 lzmhhh123 commented Oct 23, 2020 •
edited
Loading

What problem does this PR solve?

Problem Summary:

What is changed and how it works?

How it Works:

Related changes

  • PR to update pingcap/docs/pingcap/docs-cn:
  • Need to cherry-pick to the release branch

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No code

Side effects

  • Performance regression
    • Consumes more CPU
    • Consumes more MEM
  • Breaking backward compatibility

Release note

  • disable the index merge join.

@lzmhhh123 lzmhhh123 added sig/planner SIG: Planner require-LGT3 Indicates that the PR requires three LGTM. type/4.0-cherry-pick labels Oct 23, 2020
@lzmhhh123 lzmhhh123 added this to the v4.0.8 milestone Oct 23, 2020
@lzmhhh123 lzmhhh123 self-assigned this Oct 23, 2020
@XuHuaiyu
Copy link
Contributor

Should the doc be updated?
e.g. The INL_MERGE_JOIN hint

@XuHuaiyu
Copy link
Contributor

What will the warning msg be if we execute select /*+ INL_MERGE_JOIN(t) */ from ?

@zz-jason
Copy link
Member

what if there is a SQL binding that uses index merge join hint?

@SunRunAway SunRunAway changed the title release-4.0: close index merge join temporarily planner: close index merge join temporarily Oct 26, 2020
@SunRunAway SunRunAway changed the title planner: close index merge join temporarily planner: disable index merge join temporarily Oct 26, 2020
@lzmhhh123
Copy link
Contributor Author

what if there is a SQL binding that uses index merge join hint?

I redirect the index merge join hint to index join.

@lzmhhh123
Copy link
Contributor Author

/run-all-tests

@XuHuaiyu
Copy link
Contributor

LGTM

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Oct 26, 2020
@lzmhhh123
Copy link
Contributor Author

/run-sqllogic-test-1

qw4990
qw4990 previously approved these changes Oct 27, 2020
View reviewed changes
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

@ti-srebot ti-srebot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Oct 27, 2020
@qw4990
Copy link
Contributor

qw4990 commented Oct 27, 2020

/merge

@ti-srebot
Copy link
Contributor

Sorry @qw4990, you don't have permission to trigger auto merge event on this branch.

SunRunAway
SunRunAway reviewed Oct 27, 2020
View reviewed changes
executor/join_test.go Outdated Show resolved Hide resolved
@SunRunAway
Copy link
Contributor

Could you add an integration test for hints of INL_MERGE_JOIN and SQL binding for INL_MERGE_JOIN?

SunRunAway
SunRunAway approved these changes Oct 27, 2020
View reviewed changes
Copy link
Contributor

@SunRunAway SunRunAway left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot ti-srebot removed the status/LGT2 Indicates that a PR has LGTM 2. label Oct 27, 2020
@ti-srebot ti-srebot added the status/LGT3 The PR has already had 3 LGTM. label Oct 27, 2020
@jebter
Copy link

jebter commented Oct 27, 2020

/merge

@ti-srebot ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Oct 27, 2020
@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot
Copy link
Contributor

@lzmhhh123 merge failed.

@jebter
Copy link

jebter commented Oct 27, 2020

/merge

@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot ti-srebot merged commit 66ac9fc into pingcap:release-4.0 Oct 27, 2020
@lzmhhh123 lzmhhh123 deleted the dev/close_index_merge_join branch October 29, 2020 02:59
@time-and-fate time-and-fate mentioned this pull request Oct 13, 2021
Open
@qw4990 qw4990 mentioned this pull request Dec 22, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SunRunAway SunRunAway SunRunAway approved these changes

@ti-srebot ti-srebot ti-srebot approved these changes

@qw4990 qw4990 qw4990 left review comments

@XuHuaiyu XuHuaiyu Awaiting requested review from XuHuaiyu

@zz-jason zz-jason Awaiting requested review from zz-jason

Assignees

@lzmhhh123 lzmhhh123

Labels
component/expression require-LGT3 Indicates that the PR requires three LGTM. sig/execution SIG execution sig/planner SIG: Planner status/can-merge Indicates a PR has been approved by a committer. status/LGT3 The PR has already had 3 LGTM. type/4.0-cherry-pick
Projects
None yet
Milestone
v4.0.8
Development

Successfully merging this pull request may close these issues.
7 participants
@lzmhhh123 @XuHuaiyu @zz-jason @qw4990 @ti-srebot @SunRunAway @jebter