ddl: add the `not` expression check when creating a partition table #21497

Reminiscent · 2020-12-04T07:32:34Z

What problem does this PR solve?

Issue Number: close #17808

Problem Summary:
When TiDB uses HASH(not c) as the partition condition, it will convert not c to UnaryOperationExpr instead of FuncCallExpr to check whether the expression is valid.

What is changed and how it works?

What's Changed:
When we use checkPartitionExprArgs for checking whether the expression is valid. , we add UnaryOperationExpr into consideration. And if the operand is not, we will report an error.

Before this PR:

mysql> CREATE TABLE t1(c0 INT) PARTITION BY HASH((NOT c0)) PARTITIONS 2;
Query OK, 0 rows affected (0.00 sec)

After this PR:

mysql> CREATE TABLE t1(c0 INT) PARTITION BY HASH((NOT c0)) PARTITIONS 2;
ERROR 1564 (HY000): This partition function is not allowed

How it Works:

Tests

Unit test
Integration test

Release note

add the not expression check when creating a partition table

clean the code

Reminiscent · 2020-12-04T07:33:27Z

/label type/bug-fix, sig/ddl

Reminiscent · 2020-12-04T07:33:48Z

/cc @wjhuang2016

Reminiscent · 2020-12-04T07:34:41Z

/cc @tiancaiamao

ddl/partition.go

…o `checkPartitionExprFuncAllowed`

ddl/db_partition_test.go

ddl/partition.go

� Conflicts: � ddl/db_partition_test.go

xiongjiwei · 2020-12-18T08:30:01Z

a white-list of allowed expression of partition table will be much easier to compatible with MySQL because MySQL uses white-list too. we will always not sure whether we miss some expressions check if we use a black-list.

wjhuang2016 · 2020-12-23T06:42:37Z

/run-all-tests

Reminiscent · 2020-12-23T07:17:51Z

/run-check_dev

xiongjiwei · 2020-12-23T07:52:13Z

ddl/partition.go

-		ast.DateDiff: {}, ast.Floor: {}, ast.Mod: {},
-	}
-	if _, ok := allowedFuncMap[expr.FnName.L]; ok {
+	case *ast.FuncCastExpr, *ast.CaseExpr, *ast.SubqueryExpr, *ast.WindowFuncExpr, *ast.RowExpr, *ast.DefaultExpr, *ast.ValuesExpr,


we should explicitly list the expr which is allowed in partition expression. Although there are many expr, only a few of them are allowed.

Reminiscent · 2020-12-24T04:03:06Z

/run-all-tests

Reminiscent · 2020-12-24T04:08:06Z

/run-all-tests

xiongjiwei · 2020-12-24T04:51:29Z

/run-e2e-tests

wjhuang2016

LGTM

xiongjiwei

LGTM

ti-srebot · 2020-12-24T06:11:46Z

@xiongjiwei, Thanks for your review. The bot only counts LGTMs from Reviewers and higher roles, but you're still welcome to leave your comments. See the corresponding SIG page for more information. Related SIG: execution(slack).

XuHuaiyu · 2020-12-24T08:04:49Z

/merge

ti-srebot · 2020-12-24T08:04:52Z

Your auto merge job has been accepted, waiting for:

21953

ti-srebot · 2020-12-24T08:11:58Z

/run-all-tests

ti-srebot · 2020-12-24T08:14:02Z

@Reminiscent merge failed.

Reminiscent · 2020-12-24T08:17:47Z

/run-e2e-tests

…ingcap#21497)

Reminiscent added 2 commits December 4, 2020 15:07

ddl: do not use not expression when create partition table

be4f2db

change the error information

7a5958d

clean the code

ti-srebot added sig/sql-infra SIG: SQL Infra type/bugfix This PR fixes a bug. labels Dec 4, 2020

ti-srebot requested a review from wjhuang2016 December 4, 2020 07:33

ti-srebot requested a review from tiancaiamao December 4, 2020 07:34

fix test

3a6ed27

xiongjiwei reviewed Dec 4, 2020

View reviewed changes

ddl/partition.go Outdated Show resolved Hide resolved

ichn-hu mentioned this pull request Dec 4, 2020

Welcome to contribute #20804

Closed

Reminiscent added 2 commits December 7, 2020 10:22

move the ast.UnaryOperationExpr check from checkPartitionExprArgs t…

def1e65

…o `checkPartitionExprFuncAllowed`

Merge branch 'master' of https://github.com/pingcap/tidb into issue17808

56399fd

Reminiscent requested a review from xiongjiwei December 7, 2020 02:33

xiongjiwei reviewed Dec 7, 2020

View reviewed changes

ddl/db_partition_test.go Outdated Show resolved Hide resolved

address comments

710791d

Reminiscent requested a review from xiongjiwei December 7, 2020 08:16

xiongjiwei reviewed Dec 10, 2020

View reviewed changes

ddl/partition.go Show resolved Hide resolved

Reminiscent added 2 commits December 18, 2020 15:04

Merge branch 'master' of https://github.com/pingcap/tidb into issue17808

691eee2

� Conflicts: � ddl/db_partition_test.go

address comments

a3d5b3a

Reminiscent requested a review from a team as a code owner December 18, 2020 07:24

Reminiscent requested review from SunRunAway and removed request for a team December 18, 2020 07:24

github-actions bot added the component/expression label Dec 18, 2020

fix tests

e6afbec

Reminiscent added 3 commits December 22, 2020 14:48

Merge branch 'master' of https://github.com/pingcap/tidb into issue17808

cd7ae63

address comments

4dfda74

fix test

9c8463d

Merge branch 'master' into issue17808

4d459e3

xiongjiwei reviewed Dec 23, 2020

View reviewed changes

Reminiscent added 3 commits December 24, 2020 11:34

address comments

658b177

Merge remote-tracking branch 'origin/issue17808' into issue17808

bcfe5e5

FIX UT

0a0fdbb

Merge branch 'master' into issue17808

769344a

wjhuang2016 reviewed Dec 24, 2020

View reviewed changes

ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Dec 24, 2020

xiongjiwei reviewed Dec 24, 2020

View reviewed changes

Merge branch 'master' into issue17808

7443060

XuHuaiyu approved these changes Dec 24, 2020

View reviewed changes

ti-srebot removed the status/LGT1 Indicates that a PR has LGTM 1. label Dec 24, 2020

ti-srebot approved these changes Dec 24, 2020

View reviewed changes

ti-srebot added the status/LGT2 Indicates that a PR has LGTM 2. label Dec 24, 2020

ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Dec 24, 2020

Merge branch 'master' into issue17808

d3af1c9

XuHuaiyu merged commit db31b29 into pingcap:master Dec 24, 2020

Tjianke pushed a commit to Tjianke/tidb that referenced this pull request Dec 25, 2020

ddl: add the not expression check when creating a partition table (p…

a9a2d2a

…ingcap#21497)

Reminiscent deleted the issue17808 branch August 5, 2021 01:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ddl: add the `not` expression check when creating a partition table #21497

ddl: add the `not` expression check when creating a partition table #21497

Reminiscent commented Dec 4, 2020

Reminiscent commented Dec 4, 2020

Reminiscent commented Dec 4, 2020

Reminiscent commented Dec 4, 2020

xiongjiwei commented Dec 18, 2020 •

edited

Loading

wjhuang2016 commented Dec 23, 2020

Reminiscent commented Dec 23, 2020

xiongjiwei Dec 23, 2020

Reminiscent commented Dec 24, 2020

Reminiscent commented Dec 24, 2020

xiongjiwei commented Dec 24, 2020

wjhuang2016 left a comment

xiongjiwei left a comment

ti-srebot commented Dec 24, 2020

XuHuaiyu commented Dec 24, 2020

ti-srebot commented Dec 24, 2020

ti-srebot commented Dec 24, 2020

ti-srebot commented Dec 24, 2020

Reminiscent commented Dec 24, 2020

ddl: add the not expression check when creating a partition table #21497

ddl: add the not expression check when creating a partition table #21497

Conversation

Reminiscent commented Dec 4, 2020

What problem does this PR solve?

What is changed and how it works?

Release note

Reminiscent commented Dec 4, 2020

Reminiscent commented Dec 4, 2020

Reminiscent commented Dec 4, 2020

xiongjiwei commented Dec 18, 2020 • edited Loading

wjhuang2016 commented Dec 23, 2020

Reminiscent commented Dec 23, 2020

xiongjiwei Dec 23, 2020

Choose a reason for hiding this comment

Reminiscent commented Dec 24, 2020

Reminiscent commented Dec 24, 2020

xiongjiwei commented Dec 24, 2020

wjhuang2016 left a comment

Choose a reason for hiding this comment

xiongjiwei left a comment

Choose a reason for hiding this comment

ti-srebot commented Dec 24, 2020

XuHuaiyu commented Dec 24, 2020

ti-srebot commented Dec 24, 2020

ti-srebot commented Dec 24, 2020

ti-srebot commented Dec 24, 2020

Reminiscent commented Dec 24, 2020

ddl: add the `not` expression check when creating a partition table #21497

ddl: add the `not` expression check when creating a partition table #21497

xiongjiwei commented Dec 18, 2020 •

edited

Loading