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

mocktikv: avoid sorting multiple times with unstable order #15898

Merged
merged 3 commits into from
Apr 8, 2020

Conversation

fzhedu
Copy link
Contributor

@fzhedu fzhedu commented Mar 31, 2020

What problem does this PR solve?

Issue Number: close #15767

Problem Summary:
The all tuples in TopN doesn't to be sorted multiple times but resulting into unstable order, so Next() does not fetch all tuples in the same order.

What is changed and how it works?

after all tuples from its child are fetched, sort them immedrately. Then fetch data from the ordered data.

Proposal: xxx

What's Changed:

How it Works:

Related changes

  • Need to cherry-pick to the release branch

Check List

Tests

  • Unit test

Release note

@fzhedu fzhedu requested review from zhexuany, zimulala, XuHuaiyu and Reminiscent and removed request for zhexuany March 31, 2020 07:42
@fzhedu fzhedu changed the title avoid soring multiple times with unstable order mocktikv: avoid soring multiple times with unstable order Mar 31, 2020
@fzhedu fzhedu changed the title mocktikv: avoid soring multiple times with unstable order mocktikv: avoid sorting multiple times with unstable order Apr 2, 2020
@fzhedu fzhedu requested a review from a team as a code owner April 8, 2020 05:54
@ghost ghost removed their request for review April 8, 2020 05:54
@github-actions github-actions bot added the sig/execution SIG execution label Apr 8, 2020
Copy link
Contributor

@wshwsh12 wshwsh12 left a comment

Choose a reason for hiding this comment

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

LGTM

@wshwsh12 wshwsh12 added the status/LGT1 Indicates that a PR has LGTM 1. label Apr 8, 2020
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

@XuHuaiyu
Copy link
Contributor

XuHuaiyu commented Apr 8, 2020

Do we need to cherry-pick this to 2.1, 3.0, 3.1, 4.0 ?

@fzhedu
Copy link
Contributor Author

fzhedu commented Apr 8, 2020

/run-unit

Do we need to cherry-pick this to 2.1, 3.0, 3.1, 4.0 ?

yes

@fzhedu
Copy link
Contributor Author

fzhedu commented Apr 8, 2020

/run-unit-test

@fzhedu
Copy link
Contributor Author

fzhedu commented Apr 8, 2020

/run-unit-test

@fzhedu
Copy link
Contributor Author

fzhedu commented Apr 8, 2020

/run-all-test

@codecov
Copy link

codecov bot commented Apr 8, 2020

Codecov Report

Merging #15898 into master will decrease coverage by 0.1602%.
The diff coverage is 100.0000%.

@@               Coverage Diff                @@
##             master     #15898        +/-   ##
================================================
- Coverage   80.6622%   80.5019%   -0.1603%     
================================================
  Files           504        504                
  Lines        136810     135701      -1109     
================================================
- Hits         110354     109242      -1112     
+ Misses        17972      17948        -24     
- Partials       8484       8511        +27     

@fzhedu fzhedu requested a review from wshwsh12 April 8, 2020 08:00
@fzhedu
Copy link
Contributor Author

fzhedu commented Apr 8, 2020

/merge

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

sre-bot commented Apr 8, 2020

/run-all-tests

@sre-bot sre-bot merged commit 6a45a7d into pingcap:master Apr 8, 2020
@fzhedu
Copy link
Contributor Author

fzhedu commented Apr 8, 2020

/run-cherry-picker

1 similar comment
@fzhedu
Copy link
Contributor Author

fzhedu commented Apr 8, 2020

/run-cherry-picker

sre-bot pushed a commit to sre-bot/tidb that referenced this pull request Apr 8, 2020
@sre-bot
Copy link
Contributor

sre-bot commented Apr 8, 2020

cherry pick to release-2.1 in PR #16198

sre-bot pushed a commit to sre-bot/tidb that referenced this pull request Apr 8, 2020
@sre-bot
Copy link
Contributor

sre-bot commented Apr 8, 2020

cherry pick to release-3.0 in PR #16199

sre-bot pushed a commit to sre-bot/tidb that referenced this pull request Apr 8, 2020
@sre-bot
Copy link
Contributor

sre-bot commented Apr 8, 2020

cherry pick to release-3.1 in PR #16200

@sre-bot
Copy link
Contributor

sre-bot commented Apr 8, 2020

cherry pick to release-4.0 in PR #16201

@zz-jason zz-jason added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Apr 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig/execution SIG execution status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

executor: HashJoin error with limit after pushing down TopN to the coprocessor in mockTiKV
5 participants