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: revert optimizer hint scope #11740

Merged
merged 4 commits into from
Aug 15, 2019
Merged

planner: revert optimizer hint scope #11740

merged 4 commits into from
Aug 15, 2019

Conversation

foreyes
Copy link
Contributor

@foreyes foreyes commented Aug 14, 2019 •
edited
Loading

What problem does this PR solve?

Revert hint scope change from #11364

What is changed and how it works?

Stop always push hint into hint stack when meet a new query block.

Check List

Tests

  • Unit test

Side effects

  • Change the scope of optimizer hint.

Related changes

  • Fix TPCC Performance regression

@foreyes foreyes changed the title revert hint scope & remove test planner: revert optimizer hint scope Aug 14, 2019
@foreyes
Copy link
Contributor Author

foreyes commented Aug 14, 2019

/run-all-tests

@foreyes
Copy link
Contributor Author

foreyes commented Aug 14, 2019

PTAL. @zz-jason @XuHuaiyu

@codecov
Copy link

codecov bot commented Aug 14, 2019 •
edited
Loading

Codecov Report

Merging #11740 into master will not change coverage.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master     #11740   +/-   ##
===========================================
  Coverage   81.6782%   81.6782%           
===========================================
  Files           433        433           
  Lines         94134      94134           
===========================================
  Hits          76887      76887           
  Misses        11821      11821           
  Partials       5426       5426

@foreyes
Copy link
Contributor Author

foreyes commented Aug 14, 2019

/run-unit-test

1 similar comment
@foreyes
Copy link
Contributor Author

foreyes commented Aug 15, 2019

/run-unit-test

XuHuaiyu
XuHuaiyu reviewed Aug 15, 2019
View reviewed changes
Copy link
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

alivxxx
alivxxx approved these changes Aug 15, 2019
View reviewed changes
Copy link
Contributor

@alivxxx alivxxx left a comment

Choose a reason for hiding this comment

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

LGTM

@alivxxx alivxxx added status/LGT2 Indicates that a PR has LGTM 2. status/can-merge Indicates a PR has been approved by a committer. labels Aug 15, 2019
@sre-bot
Copy link
Contributor

sre-bot commented Aug 15, 2019

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented Aug 15, 2019

@foreyes merge failed.

@foreyes
Copy link
Contributor Author

foreyes commented Aug 15, 2019

/run-unit-test

@foreyes foreyes merged commit f866bb4 into pingcap:master Aug 15, 2019
@foreyes foreyes deleted the dev/revert_hint_scope branch August 15, 2019 05:49
foreyes added a commit to foreyes/tidb that referenced this pull request Sep 16, 2019
@foreyes
revert hint scope & remove test (pingcap#11740)
ada3a82
@foreyes foreyes mentioned this pull request Sep 16, 2019
Merged
@foreyes foreyes mentioned this pull request Sep 25, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@XuHuaiyu XuHuaiyu XuHuaiyu left review comments

@alivxxx alivxxx alivxxx approved these changes

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

Assignees
No one assigned
Labels
sig/planner SIG: Planner status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@foreyes @sre-bot @XuHuaiyu @alivxxx