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: fold constant in projection elimination and TopN push down #14927

Merged
merged 5 commits into from
Feb 25, 2020

Conversation

eurekaka
Copy link
Contributor

What problem does this PR solve?

Fix #14926.

What is changed and how it works?

Fold constant in projection elimination. TopN pushdown check is unnecessary for this case actually, but we can enforce it to prevent potential bugs incurred by patches from future.

Check List

Tests

  • Unit test

Code changes

N/A

Side effects

N/A

Related changes

  • Need to cherry-pick to the release branch

Release note

  • Write release note for bug-fix or new feature: fix wrong result for queries with single MAX() over UNION

@eurekaka eurekaka added type/bugfix This PR fixes a bug. sig/planner SIG: Planner needs-cherry-pick-2.1 labels Feb 25, 2020
@eurekaka eurekaka requested a review from a team as a code owner February 25, 2020 04:01
@ghost ghost requested review from francis0407 and alivxxx and removed request for a team February 25, 2020 04:01
@codecov
Copy link

codecov bot commented Feb 25, 2020

Codecov Report

Merging #14927 into master will increase coverage by 0.0368%.
The diff coverage is 96.4539%.

@@               Coverage Diff                @@
##             master     #14927        +/-   ##
================================================
+ Coverage   80.2223%   80.2591%   +0.0368%     
================================================
  Files           503        503                
  Lines        130905     130911         +6     
================================================
+ Hits         105015     105068        +53     
+ Misses        17586      17552        -34     
+ Partials       8304       8291        -13

@eurekaka
Copy link
Contributor Author

/rebuild

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

Copy link
Contributor

@lzmhhh123 lzmhhh123 left a comment

Choose a reason for hiding this comment

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

LGTM

@eurekaka
Copy link
Contributor Author

/merge

@sre-bot sre-bot added the status/can-merge Indicates a PR has been approved by a committer. label Feb 25, 2020
@sre-bot
Copy link
Contributor

sre-bot commented Feb 25, 2020

Your auto merge job has been accepted, waiting for 14664

@sre-bot
Copy link
Contributor

sre-bot commented Feb 25, 2020

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented Feb 25, 2020

@eurekaka merge failed.

@eurekaka
Copy link
Contributor Author

/run-unit-test

@eurekaka eurekaka merged commit 880a9b0 into pingcap:master Feb 25, 2020
@eurekaka eurekaka deleted the topn branch February 25, 2020 12:34
@sre-bot
Copy link
Contributor

sre-bot commented Feb 25, 2020

cherry pick to release-2.1 failed

@sre-bot
Copy link
Contributor

sre-bot commented Feb 25, 2020

cherry pick to release-3.0 failed

@sre-bot
Copy link
Contributor

sre-bot commented Feb 25, 2020

cherry pick to release-4.0 failed

@sre-bot
Copy link
Contributor

sre-bot commented Apr 7, 2020

It seems that, not for sure, we failed to cherry-pick this commit to release-2.1 release-3.0. Please comment '/run-cherry-picker' to try to trigger the cherry-picker if we did fail to cherry-pick this commit before. @eurekaka PTAL.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig/planner SIG: Planner status/can-merge Indicates a PR has been approved by a committer. type/bugfix This PR fixes a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

remove constant expressions in order by items
4 participants