Skip to content

planner,expression,table: clean up the old partition pruning code#14834

Merged
tiancaiamao merged 12 commits intopingcap:masterfrom
tiancaiamao:clean-pruning
Feb 26, 2020
Merged

planner,expression,table: clean up the old partition pruning code#14834
tiancaiamao merged 12 commits intopingcap:masterfrom
tiancaiamao:clean-pruning

Conversation

@tiancaiamao
Copy link
Contributor

What problem does this PR solve?

Clean up unused code after #14679

What is changed and how it works?

  • Remove constraint propagator
  • Extract common code for pruneHashPartition
  • Clean up the table package as PartitionExpr.Ranges are not used

I also remove a test case as it can't be pruned anymore without constraint propagator:

create table t1 (id int, a int) partition by hash(id) ...
select * from t1 where a > 7 and a < 3

Check List

Tests

  • No code

Code changes

  • Has exported variable/fields change

@tiancaiamao tiancaiamao requested review from a team as code owners February 18, 2020 10:52
@ghost ghost requested review from SunRunAway, XuHuaiyu, alivxxx and francis0407 and removed request for a team February 18, 2020 10:52
@tiancaiamao tiancaiamao added the type/enhancement The issue or PR belongs to an enhancement. label Feb 18, 2020
@tiancaiamao
Copy link
Contributor Author

PTAL @imtbkcat

@tiancaiamao
Copy link
Contributor Author

/run-all-tests

@tiancaiamao
Copy link
Contributor Author

/rebuild

@tiancaiamao tiancaiamao requested a review from imtbkcat February 19, 2020 05:10
@tiancaiamao
Copy link
Contributor Author

PTAL @lysu @imtbkcat

Copy link

@imtbkcat imtbkcat left a comment

Choose a reason for hiding this comment

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

rest LGTM

]
},
{
"SQL": "explain select * from t1 where t1.a > 7 and t1.a < 3",

Choose a reason for hiding this comment

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

Why removing these cases?

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because I remove constraint propagate solver, now a > 7 and a < 3 can't be solved.
This plan now change to union all partitions.

I guess it's acceptable: our user should be clever enough and not writing this kind of silly queries.

@francis0407 francis0407 removed their request for review February 19, 2020 08:15
Copy link
Contributor

@lysu lysu left a comment

Choose a reason for hiding this comment

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

LGTM

@lysu lysu added the status/LGT1 Indicates that a PR has LGTM 1. label Feb 20, 2020
@codecov
Copy link

codecov bot commented Feb 25, 2020

Codecov Report

Merging #14834 into master will not change coverage by %.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master     #14834   +/-   ##
===========================================
  Coverage   80.7188%   80.7188%           
===========================================
  Files           503        503           
  Lines        133493     133493           
===========================================
  Hits         107754     107754           
  Misses        17404      17404           
  Partials       8335       8335           

Copy link

@imtbkcat imtbkcat left a comment

Choose a reason for hiding this comment

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

LGTM

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

sre-bot commented Feb 25, 2020

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented Feb 25, 2020

@tiancaiamao merge failed.

@tiancaiamao
Copy link
Contributor Author

/run-all-tests

@imtbkcat
Copy link

/run-all-tests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status/can-merge Indicates a PR has been approved by a committer. status/LGT1 Indicates that a PR has LGTM 1. type/enhancement The issue or PR belongs to an enhancement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants