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: enhance the rule max_min_eliminate to support multiple agg (#12083) #14410

Merged
merged 6 commits into from
Jan 9, 2020

Conversation

francis0407
Copy link
Member

@francis0407 francis0407 commented Jan 9, 2020

What problem does this PR solve?

cherry-pick #12083

conflicts:
planner/core/rule_max_min_eliminate.go
planner/core/logical_plans.go
planner/core/testdata/plan_suite_out.json

The major difference between release-3.0 and master branch which this PR encounters is:
In master,

// For index paths, we have to check:
// 1. whether all of the conditions can be pushed down as accessConds.
// 2. whether the AccessPath can satisfy the order property of `col` with these accessConds.
result, err := ranger.DetachCondAndBuildRangeForIndex(p.ctx, conditions, path.FullIdxCols, path.FullIdxColLens)
if err != nil || len(result.RemainedConds) != 0 {
	continue
}

But in release-3.0, AccessPath does not have fullIdxCols, so I use expression.IndexInfo2Cols(p.schema.Columns, path.index) to generate idxCols instead.

// For index paths, we have to check:
// 1. whether all of the conditions can be pushed down as accessConds.
// 2. whether the accessPath can satisfy the order property of `col` with these accessConds.
idxCols, idxColLens := expression.IndexInfo2Cols(p.schema.Columns, path.index)
if len(idxCols) == 0 {
	continue
}
result, err := ranger.DetachCondAndBuildRangeForIndex(p.ctx, conditions, idxCols, idxColLens)
if err != nil || len(result.RemainedConds) != 0 {
	continue
}

What is changed and how it works?

Check List

Tests

  • Unit test

Side effects

  • Possible performance regression
  • Increased code complexity

Release note

  • Optimize max/min queries which contain more than one max/min.
    For example,
select max(a), min(a) from t

will be rewritten as

select * from
  (select a from t order by a desc limit 1),
  (select a from t order by a limit 1)

@francis0407 francis0407 added type/enhancement The issue or PR belongs to an enhancement. sig/planner SIG: Planner type/3.0 cherry-pick labels Jan 9, 2020
@francis0407 francis0407 requested review from eurekaka and winoros and removed request for eurekaka January 9, 2020 05:56
Copy link
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

@francis0407
Copy link
Member Author

/run-unit-test

Copy link
Member

@winoros winoros left a comment

Choose a reason for hiding this comment

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

lgtm

@francis0407 francis0407 added the status/LGT2 Indicates that a PR has LGTM 2. label Jan 9, 2020
@eurekaka
Copy link
Contributor

eurekaka commented Jan 9, 2020

/merge

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

sre-bot commented Jan 9, 2020

/run-all-tests

@lonng lonng merged commit 948b5c6 into pingcap:release-3.0 Jan 9, 2020
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/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants