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

plan, partition: support point get plan on hash partition table #14318

Merged
merged 11 commits into from Jan 10, 2020

Conversation

@imtbkcat
Copy link
Contributor

imtbkcat commented Jan 2, 2020

What problem does this PR solve?

Planner will not execute tryFastPlan on partition table, which will cause impress on partition table performance.

What is changed and how it works?

  • Support PointGet plan on hash partition table.
  • Eval partition expression using name pairs.

Check List

Tests

  • Unit test
  • Manual test (add detailed scripts or steps below)

Code changes

  • None

Side effects

  • Possible performance regression
  • Increased code complexity

Related changes

  • Need to cherry-pick to the release branch
@imtbkcat imtbkcat requested review from pingcap/co-expression as code owners Jan 2, 2020
@pull-assigner pull-assigner bot requested review from qw4990, XuHuaiyu, eurekaka and winoros and removed request for pingcap/co-expression Jan 2, 2020
@imtbkcat imtbkcat removed request for qw4990, XuHuaiyu, eurekaka and winoros Jan 2, 2020
@imtbkcat imtbkcat force-pushed the imtbkcat:HashPartitionPointGet branch from 214d24b to a2356c2 Jan 3, 2020
@imtbkcat imtbkcat removed the status/WIP label Jan 3, 2020
@imtbkcat imtbkcat requested review from cfzjywxk, jackysp and tiancaiamao Jan 3, 2020
@imtbkcat imtbkcat added the status/WIP label Jan 3, 2020
@imtbkcat imtbkcat force-pushed the imtbkcat:HashPartitionPointGet branch from a2356c2 to e7f8962 Jan 3, 2020
@imtbkcat

This comment has been minimized.

Copy link
Contributor Author

imtbkcat commented Jan 3, 2020

/run-all-tests

@cfzjywxk

This comment has been minimized.

Copy link
Contributor

cfzjywxk commented Jan 3, 2020

LGTM

@jackysp jackysp requested a review from coocood Jan 6, 2020
planner/core/point_get_plan.go Outdated Show resolved Hide resolved
switch pi := piExpr.(type) {
case *expression.Column:
for _, p := range pairs {
if p.colName == pi.OrigName {

This comment has been minimized.

Copy link
@coocood

coocood Jan 6, 2020

Member

Can pi.OrigName be upper case?

This comment has been minimized.

Copy link
@imtbkcat

imtbkcat Jan 6, 2020

Author Contributor

I add OrigName: col.Name.L in the colInfoToColumn function.

if err != nil {
return nil
}
pos, ok := locateHashPartition(exprs[0], pairs)

This comment has been minimized.

Copy link
@coocood

coocood Jan 6, 2020

Member

Is it possible that len(exprs) > 1?

This comment has been minimized.

Copy link
@imtbkcat

imtbkcat Jan 6, 2020

Author Contributor

no, expression will be checked when creating table.

@imtbkcat imtbkcat force-pushed the imtbkcat:HashPartitionPointGet branch from e7f8962 to 0afe9c1 Jan 6, 2020
planner/core/point_get_plan.go Outdated Show resolved Hide resolved
executor/batch_point_get.go Show resolved Hide resolved
planner/core/point_get_plan.go Outdated Show resolved Hide resolved
planner/core/point_get_plan.go Outdated Show resolved Hide resolved
planner/core/point_get_plan.go Outdated Show resolved Hide resolved
planner/core/rule_partition_processor.go Outdated Show resolved Hide resolved
table/tables/partition.go Outdated Show resolved Hide resolved
table/tables/partition.go Show resolved Hide resolved
table/tables/partition.go Show resolved Hide resolved
@imtbkcat imtbkcat force-pushed the imtbkcat:HashPartitionPointGet branch from aec36b3 to d89cd74 Jan 8, 2020
@imtbkcat

This comment has been minimized.

Copy link
Contributor Author

imtbkcat commented Jan 8, 2020

/run-all-tests

@imtbkcat

This comment has been minimized.

Copy link
Contributor Author

imtbkcat commented Jan 9, 2020

}, nil
}

// GetOriginPartitionExpr is used for PointGet plan.

This comment has been minimized.

Copy link
@tiancaiamao

tiancaiamao Jan 9, 2020

Contributor

This function is not used any more

@imtbkcat imtbkcat force-pushed the imtbkcat:HashPartitionPointGet branch from d89cd74 to 7a8ebe8 Jan 9, 2020
@coocood

This comment has been minimized.

Copy link
Member

coocood commented Jan 10, 2020

LGTM

1 similar comment
@tiancaiamao

This comment has been minimized.

Copy link
Contributor

tiancaiamao commented Jan 10, 2020

LGTM

@tiancaiamao

This comment has been minimized.

Copy link
Contributor

tiancaiamao commented Jan 10, 2020

/run-all-tests

@tiancaiamao tiancaiamao merged commit b6aca51 into pingcap:master Jan 10, 2020
15 checks passed
15 checks passed
idc-jenkins-ci-tidb/build Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/build_check_race Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/check_dev Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/check_dev_2 Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/common-test job succeeded
Details
idc-jenkins-ci-tidb/integration-common-test Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/integration-compatibility-test Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/integration-copr-test Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/integration-ddl-test Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/mybatis-test job succeeded
Details
idc-jenkins-ci-tidb/sqllogic-test-1 Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/sqllogic-test-2 Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/unit-test Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/wasm-build Jenkins job succeeded.
Details
license/cla Contributor License Agreement is signed.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.