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: fix some window specific check bug for window function. #12394

Merged
merged 6 commits into from
Sep 26, 2019

Conversation

lzmhhh123
Copy link
Contributor

What problem does this PR solve?

fix #12353

What is changed and how it works?

add some check for the situation.

Check List

Tests

  • Unit test
  • Integration test

Side effects

  • Increased code complexity

Related changes

  • Need to cherry-pick to the release branch

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

@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

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

sre-bot commented Sep 25, 2019

Your auto merge job has been accepted, waiting for 12375

@sre-bot
Copy link
Contributor

sre-bot commented Sep 25, 2019

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented Sep 25, 2019

@lzmhhh123 merge failed.

@lzmhhh123
Copy link
Contributor Author

/merge

@sre-bot
Copy link
Contributor

sre-bot commented Sep 25, 2019

Your auto merge job has been accepted, waiting for 12376

@sre-bot
Copy link
Contributor

sre-bot commented Sep 25, 2019

/run-all-tests

@lzmhhh123
Copy link
Contributor Author

/run-unit-test

@codecov
Copy link

codecov bot commented Sep 25, 2019

Codecov Report

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

@@             Coverage Diff             @@
##             master     #12394   +/-   ##
===========================================
  Coverage   80.0668%   80.0668%           
===========================================
  Files           461        461           
  Lines        103180     103180           
===========================================
  Hits          82613      82613           
  Misses        14746      14746           
  Partials       5821       5821

@sre-bot
Copy link
Contributor

sre-bot commented Sep 25, 2019

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented Sep 25, 2019

@lzmhhh123 merge failed.

@lzmhhh123
Copy link
Contributor Author

/merge

@sre-bot
Copy link
Contributor

sre-bot commented Sep 26, 2019

/run-all-tests

@sre-bot sre-bot merged commit fbcee67 into pingcap:master Sep 26, 2019
@lzmhhh123 lzmhhh123 deleted the dev/fix-12353 branch September 26, 2019 02:45
@sre-bot
Copy link
Contributor

sre-bot commented Sep 26, 2019

cherry pick to release-3.0 failed

@sre-bot
Copy link
Contributor

sre-bot commented Sep 26, 2019

cherry pick to release-3.1 failed

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. status/LGT2 Indicates that a PR has LGTM 2. type/bugfix This PR fixes a bug.
Projects
None yet
5 participants