ddl: check only the range type partition #7599

ciscoxll · 2018-09-04T03:41:50Z

What problem does this PR solve?

Fix issue Create Range columns type partition error #7607

What is changed and how it works?

Only range type partition is now supported.
Other partition types only implement the parser so they will not be checked.
Add some tests for different partition types.

Check List

Tests

Unit test

@tiancaiamao @zimulala @crazycs520 PTAL .

winkyao · 2018-09-04T08:01:52Z

Please fix conlicts。

winkyao

reset LGTM

…ition

ciscoxll · 2018-09-04T09:14:51Z

@winkyao PTAL.

ciscoxll · 2018-09-05T02:22:44Z

@tiancaiamao @zimulala @crazycs520 PTAL .

shenli · 2018-09-05T02:35:10Z

ddl/db_test.go

@@ -1711,6 +1711,54 @@ func (s *testDBSuite) TestCreateTableWithPartition(c *C) {
 		  partition p3 values less than (18446744073709551000 + 1),
 		  partition p4 values less than (18446744073709551000 + 10)
 		);`)
+
+	// Support partitioned by range columns later "ERROR HY000: Field 'a' is of a not allowed type for this type of partitioning".


What is the relationship between the comment and the following case?

shenli · 2018-09-05T02:35:35Z

ddl/db_test.go

+	(partition p0 values less than (0));`)
+}
+
+func (s *testDBSuite) TestCreateTableWithHashPartition(c *C) {


What is the purpose of this test case?

Check grammar compatibility for different table partition type? @shenli

shenli · 2018-09-05T02:38:39Z

ddl/ddl_api.go

-		}
+	if pi != nil && pi.Type == model.PartitionTypeRange {
+		// Check partition by range.
+		if s.Partition.ColumnNames == nil {


From the title, I can tell there is only one constraint on the checking "restrict the check partition function to check only the rang partition". But I find another constraint here.

tiancaiamao · 2018-09-05T08:32:57Z

LGTM

tiancaiamao · 2018-09-05T08:33:42Z

/run-all-tests

ciscoxll · 2018-09-06T06:37:45Z

@CaitinChen PTAL.

CaitinChen · 2018-09-06T07:04:29Z

ddl/db_test.go

@@ -1711,6 +1711,55 @@ func (s *testDBSuite) TestCreateTableWithPartition(c *C) {
 		  partition p3 values less than (18446744073709551000 + 1),
 		  partition p4 values less than (18446744073709551000 + 10)
 		);`)
+
+	// Only range type partition are now supported, range columns partition only implement parser so do not check.


Only range type partition is now supported. Range columns partition only implements the parser, so it will not be checked.

CaitinChen · 2018-09-06T07:09:27Z

ddl/db_test.go

@@ -1711,6 +1711,55 @@ func (s *testDBSuite) TestCreateTableWithPartition(c *C) {
 		  partition p3 values less than (18446744073709551000 + 1),
 		  partition p4 values less than (18446744073709551000 + 10)
 		);`)
+
+	// Only range type partition are now supported, range columns partition only implement parser so do not check.
+	// So executing the following sql can be executed successfully.


So the following SQL statements can be executed successfully.

CaitinChen · 2018-09-06T07:10:12Z

ddl/ddl_api.go

-			return errors.Trace(err)
-		}
+	if pi != nil && pi.Type == model.PartitionTypeRange {
+		// Only range type partition are now supported.


Only range type partition is now supported.

CaitinChen · 2018-09-06T07:10:28Z

ddl/ddl_api.go

-		}
+	if pi != nil && pi.Type == model.PartitionTypeRange {
+		// Only range type partition are now supported.
+		// Range columns partition only implement parser so do not check.


Range columns partition only implements the parser, so it will not be checked.

CaitinChen · 2018-09-06T07:13:03Z

What is changed and how it works?

Only range type partition is now supported.
Other partition types only implement the parser so they will not be checked.
Add some tests for different partition types.

ciscoxll · 2018-09-06T07:20:40Z

@CaitinChen Thanks for your guidance.

CaitinChen · 2018-09-06T07:22:30Z

@ciscoxll My pleasure

ciscoxll · 2018-09-06T08:11:24Z

@shenli PTAL.

coocood · 2018-09-06T08:20:27Z

LGTM

shenli · 2018-09-06T08:23:07Z

/run-all-tests

shenli

LGTM

ciscoxll added component/DDL-need-LGT3 and removed status/WIP labels Sep 4, 2018

winkyao reviewed Sep 4, 2018

View reviewed changes

ddl:restrict the check partition function to check only the rang part…

0a586a8

…ition

ciscoxll force-pushed the check-range-partition branch from 19d3082 to 0a586a8 Compare September 4, 2018 09:13

ciscoxll added 2 commits September 4, 2018 17:21

fix ci error

5dd23fa

fix ci error

8ae695a

ciscoxll changed the title ~~ddl:restrict the check partition function to check only the rang partition~~ ddl:restrict the check partition function to check only the range partition Sep 5, 2018

shenli reviewed Sep 5, 2018

View reviewed changes

ciscoxll added the status/WIP label Sep 5, 2018

address comment

7d3ff8c

tiancaiamao added status/LGT1 Indicates that a PR has LGTM 1. and removed status/WIP labels Sep 5, 2018

ciscoxll added the status/WIP label Sep 6, 2018

ciscoxll changed the title ~~ddl:restrict the check partition function to check only the range partition~~ ddl:checks only the range type partition Sep 6, 2018

Update function comment

3a48f55

ciscoxll force-pushed the check-range-partition branch from bed6b62 to 3a48f55 Compare September 6, 2018 06:28

ciscoxll removed the status/WIP label Sep 6, 2018

ciscoxll changed the title ~~ddl:checks only the range type partition~~ ddl:check only the range type partition Sep 6, 2018

CaitinChen reviewed Sep 6, 2018

View reviewed changes

Address comment

acd6c29

coocood added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Sep 6, 2018

Merge branch 'master' into check-range-partition

30b5c8c

shenli approved these changes Sep 6, 2018

View reviewed changes

ciscoxll added status/LGT3 The PR has already had 3 LGTM. and removed status/LGT2 Indicates that a PR has LGTM 2. labels Sep 6, 2018

shenli changed the title ~~ddl:check only the range type partition~~ ddl: check only the range type partition Sep 6, 2018

ciscoxll added the status/all tests passed label Sep 6, 2018

ciscoxll merged commit 7c6c279 into pingcap:master Sep 6, 2018

ciscoxll deleted the check-range-partition branch September 6, 2018 08:55

sre-bot added the contribution This PR is from a community contributor. label Dec 18, 2019

you06 added the sig/sql-infra SIG: SQL Infra label Mar 4, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ddl: check only the range type partition #7599

ddl: check only the range type partition #7599

ciscoxll commented Sep 4, 2018 •

edited

Loading

winkyao commented Sep 4, 2018

winkyao left a comment

ciscoxll commented Sep 4, 2018

ciscoxll commented Sep 5, 2018

shenli Sep 5, 2018

shenli Sep 5, 2018

tiancaiamao Sep 5, 2018

shenli Sep 5, 2018

tiancaiamao commented Sep 5, 2018

tiancaiamao commented Sep 5, 2018

ciscoxll commented Sep 6, 2018

CaitinChen Sep 6, 2018

CaitinChen Sep 6, 2018

CaitinChen Sep 6, 2018

CaitinChen Sep 6, 2018

CaitinChen commented Sep 6, 2018 •

edited

Loading

ciscoxll commented Sep 6, 2018

CaitinChen commented Sep 6, 2018

ciscoxll commented Sep 6, 2018

coocood commented Sep 6, 2018

shenli commented Sep 6, 2018

shenli left a comment

ddl: check only the range type partition #7599

ddl: check only the range type partition #7599

Conversation

ciscoxll commented Sep 4, 2018 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

winkyao commented Sep 4, 2018

winkyao left a comment

Choose a reason for hiding this comment

ciscoxll commented Sep 4, 2018

ciscoxll commented Sep 5, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tiancaiamao commented Sep 5, 2018

tiancaiamao commented Sep 5, 2018

ciscoxll commented Sep 6, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CaitinChen commented Sep 6, 2018 • edited Loading

What is changed and how it works?

ciscoxll commented Sep 6, 2018

CaitinChen commented Sep 6, 2018

ciscoxll commented Sep 6, 2018

coocood commented Sep 6, 2018

shenli commented Sep 6, 2018

shenli left a comment

Choose a reason for hiding this comment

ciscoxll commented Sep 4, 2018 •

edited

Loading

CaitinChen commented Sep 6, 2018 •

edited

Loading