Skip to content

planner: add AGG_TO_COP hint#12043

Merged
foreyes merged 9 commits into
pingcap:masterfrom
foreyes:dev/add_agg2cop
Sep 12, 2019
Merged

planner: add AGG_TO_COP hint#12043
foreyes merged 9 commits into
pingcap:masterfrom
foreyes:dev/add_agg2cop

Conversation

@foreyes
Copy link
Copy Markdown
Contributor

@foreyes foreyes commented Sep 5, 2019

What problem does this PR solve?

Add sql hint for enforcing push aggregation to coprocessor.

What is changed and how it works?

Check if the logical aggregation can be push down first, and throw a warning if not.

Then only cop task will be considered when enumerating task types with AGG_TO_COP hint.

Check List

Tests

  • Unit test

Code changes

  • None

Side effects

  • None

Related changes

  • None

@foreyes foreyes self-assigned this Sep 5, 2019
@foreyes foreyes requested review from eurekaka, lzmhhh123 and zz-jason and removed request for zz-jason September 5, 2019 08:07
Comment thread planner/core/exhaust_physical_plans.go Outdated
Comment thread planner/core/exhaust_physical_plans.go Outdated
@foreyes
Copy link
Copy Markdown
Contributor Author

foreyes commented Sep 11, 2019

Accept suggestion and change some test, PTAL @lzmhhh123
Address comment, PTAL @eurekaka

Copy link
Copy Markdown
Contributor

@eurekaka eurekaka left a comment

Choose a reason for hiding this comment

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

LGTM

Comment thread planner/core/exhaust_physical_plans.go Outdated
@eurekaka eurekaka added the status/LGT1 Indicates that a PR has LGTM 1. label Sep 12, 2019
Co-Authored-By: Kenan Yao <cauchy1992@gmail.com>
Copy link
Copy Markdown
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.

@lzmhhh123 lzmhhh123 added status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Sep 12, 2019
@sre-bot
Copy link
Copy Markdown
Contributor

sre-bot commented Sep 12, 2019

/run-all-tests

@sre-bot
Copy link
Copy Markdown
Contributor

sre-bot commented Sep 12, 2019

@foreyes merge failed.

@foreyes
Copy link
Copy Markdown
Contributor Author

foreyes commented Sep 12, 2019

/run-integration-ddl-test
/run-mybatis-test

@foreyes
Copy link
Copy Markdown
Contributor Author

foreyes commented Sep 12, 2019

/run-integration-ddl-test
/run-mybatis-test

@foreyes foreyes merged commit c7518de into pingcap:master Sep 12, 2019
@foreyes foreyes deleted the dev/add_agg2cop branch September 12, 2019 03:36
foreyes added a commit to foreyes/tidb that referenced this pull request Sep 16, 2019
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/new-feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants