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: check required order property for enforced stream aggregation (#17338) #17347

Merged
merged 2 commits into from
May 22, 2020
Merged

planner: check required order property for enforced stream aggregation (#17338) #17347

merged 2 commits into from
May 22, 2020

Conversation

sre-bot
Copy link
Contributor

@sre-bot sre-bot commented May 21, 2020

cherry-pick #17338 to release-4.0

What problem does this PR solve?

Issue Number: close #17325

Problem Summary:

Wrong result for enforced stream aggregation when order is required.

What is changed and how it works?

What's Changed:

Check if the enforced stream aggregation can satisfy the required order property.

Related changes

  • Need to cherry-pick to the release branch

Check List

Tests

  • Unit test

Side effects

N/A

Release note

  • Fix wrong result order for enforced stream aggregation.

@sre-bot sre-bot requested a review from a team as a code owner May 21, 2020 15:52
@sre-bot
Copy link
Contributor Author

sre-bot commented May 21, 2020

/run-all-tests

@ghost ghost requested review from eurekaka and removed request for a team May 21, 2020 15:52
@sre-bot sre-bot mentioned this pull request May 21, 2020
Merged
@sre-bot sre-bot added sig/planner SIG: Planner type/4.0-cherry-pick type/bugfix This PR fixes a bug. labels May 21, 2020
@sre-bot sre-bot added this to the v4.0.0-ga milestone May 21, 2020
zz-jason
zz-jason previously approved these changes May 21, 2020
View reviewed changes
Copy link
Member

@zz-jason zz-jason left a comment

Choose a reason for hiding this comment

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

LGTM

@zz-jason
Copy link
Member

please fix CI:

[2020-05-21T15:55:37.874Z] ...     "[TopN_10 1.00 root  Column#3:asc, offset:0, count:1]\n" +

[2020-05-21T15:55:37.874Z] ...     "[└─StreamAgg_17 8000.00 root  group by:test.t.a, funcs:count(1)->Column#3]\n" +

[2020-05-21T15:55:37.874Z] ...     "[  └─Sort_22 10000.00 root  test.t.a:asc]\n" +

[2020-05-21T15:55:37.874Z] ...     "[    └─TableReader_21 10000.00 root  data:TableFullScan_20]\n" +

[2020-05-21T15:55:37.874Z] ...     "[      └─TableFullScan_20 10000.00 cop[tikv] table:t keep order:false, stats:pseudo]\n"

[2020-05-21T15:55:37.874Z] ... expected string = "" +

[2020-05-21T15:55:37.874Z] ...     "[TopN_10 1.00 root  Column#3, offset:0, count:1]\n" +

[2020-05-21T15:55:37.874Z] ...     "[└─StreamAgg_17 8000.00 root  group by:test.t.a, funcs:count(1)->Column#3]\n" +

[2020-05-21T15:55:37.874Z] ...     "[  └─Sort_22 10000.00 root  test.t.a]\n" +

[2020-05-21T15:55:37.874Z] ...     "[    └─TableReader_21 10000.00 root  data:TableFullScan_20]\n" +

[2020-05-21T15:55:37.874Z] ...     "[      └─TableFullScan_20 10000.00 cop[tikv] table:t keep order:false, stats:pseudo]\n"

[2020-05-21T15:55:37.874Z] ... sql:explain select /*+ stream_agg() */ count(*) c from t group by a order by c limit 1, args:[]

@eurekaka eurekaka force-pushed the release-4.0-134e691d6f5f branch from 8318054 to 2aa891b Compare May 22, 2020 02:45
@eurekaka
Copy link
Contributor

/run-unit-test

@eurekaka
Copy link
Contributor

/run-unit-test

@eurekaka
Copy link
Contributor

/run-all-tests

SunRunAway
SunRunAway previously approved these changes May 22, 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

@eurekaka eurekaka requested a review from zz-jason May 22, 2020 04:18
@jebter
Copy link

jebter commented May 22, 2020

/run-all-tests

@eurekaka
Copy link
Contributor

/run-unit-test

2 similar comments
@eurekaka
Copy link
Contributor

/run-unit-test

@jebter
Copy link

jebter commented May 22, 2020

/run-unit-test

@SunRunAway SunRunAway dismissed their stale review May 22, 2020 05:53

need one more LGTM

@eurekaka
Copy link
Contributor

/run-unit-test

2 similar comments
@eurekaka
Copy link
Contributor

/run-unit-test

@eurekaka
Copy link
Contributor

/run-unit-test

@jebter
Copy link

jebter commented May 22, 2020

/approve

@sre-bot
Copy link
Contributor Author

sre-bot commented May 22, 2020

Approve failed.

@jebter
Copy link

jebter commented May 22, 2020

/approve

@sre-bot
Copy link
Contributor Author

sre-bot commented May 22, 2020

Approve failed.

@jebter
Copy link

jebter commented May 22, 2020

/merge

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

sre-bot commented May 22, 2020

/run-all-tests

@sre-bot sre-bot merged commit ffaae4f into pingcap:release-4.0 May 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jebter jebter jebter approved these changes

@SunRunAway SunRunAway SunRunAway left review comments

@eurekaka eurekaka Awaiting requested review from eurekaka

@lzmhhh123 lzmhhh123 Awaiting requested review from lzmhhh123

@winoros winoros Awaiting requested review from winoros

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

Assignees

@eurekaka eurekaka

Labels
sig/planner SIG: Planner status/can-merge Indicates a PR has been approved by a committer. type/bugfix This PR fixes a bug. type/4.0-cherry-pick
Projects
None yet
Milestone
v4.0.0-ga
Development

Successfully merging this pull request may close these issues.
5 participants
@sre-bot @zz-jason @eurekaka @jebter @SunRunAway