plan,table: fix select null value for table partition #7452

tiancaiamao · 2018-08-21T12:02:39Z

What problem does this PR solve?

set @@session.tidb_enable_table_partition = 1;
create table t1 (a int unsigned) partition by range (a)
(partition pnull values less than (0),
 partition p0 values less than (1),
 partition p1 values less than(2));

insert into t1 values (null),(0),(1);

select * from t1 where a is null;

should get

mysql> select * from t1 where a is null;
+------+
| a    |
+------+
| NULL |

But we get an empty result.

mysql> select * from t1 where a is null;
Empty set (0.00 sec)

The reason is that partition pruning works incorrectly, it prunes all the partitions.

What is changed and how it works?

The partition pruning condition of partition[0] is changed from

expr < value

to

expr < value or expr is null

Before this change, the partition pruning conditions in the failed example are:

a < 0 and a is null
0 < a < 1 and a is null
1 < a < 2 and a is null

The pruning result is an empty set.

After this change, the partition pruning conditions are:

(a < 0 or a is null) and a is null
0 < a < 1 and a is null
1 < a < 2 and a is null

The first partition remains, so we get the correct result.

Check List

Tests

Unit test

@zz-jason @coocood

tiancaiamao · 2018-08-22T05:16:33Z

/run-all-tests tidb-test=pr/606

coocood · 2018-08-22T05:17:37Z

table/tables/partition.go

+				}
+			}
+			if column == nil {
+				log.Warnf("partition pruning won't work on this expr:%s", pi.Expr)


I think we don't need to log a warning.

Without partition pruning, a query need to scan all partitions, we'd better warn the user about this.
For example,

create table t (id int) partition by range (mod(id, 5)) (partition ... )

The warning log is not frequent, it's happen when creating table, and normal expression will not print the warning.

tiancaiamao · 2018-08-22T05:27:32Z

Check if the expression is null is more complex, the expression may be anything:

select * from t where a is null
select * from t where a is null or a > 6
select * from t where a is null and a > 6
select * from t where cast(a, as integer) > 3
select * from t where to_days(a) = 2016000;
...

Change partition expression range is much easier, for partition 1 and others:

expr < lower bound and expr < upper bound

while partition 0 condition is actually

expr < upper bound or expr is null

@coocood

tiancaiamao · 2018-08-22T06:12:52Z

/run-common-test tidb-test=pr/606
/run-integration-common-test tidb-test=pr/606

coocood · 2018-08-22T07:30:47Z

table/tables/partition.go

+			fmt.Fprintf(&buf, " or ((%s) is null)", pi.Expr)
+
+			// Extracts the column of the partition expression, it will be used by partition prunning.
+			if tmp, err1 := expression.ParseSimpleExprWithTableInfo(ctx, pi.Expr, tblInfo); err1 == nil {


err1 is not logged.

If expression.ParseSimpleExprWithTableInfo(ctx, pi.Expr, tblInfo) meets error,
expression.ParseSimpleExprWithTableInfo(ctx, pi.Expr < xxx, tblInfo) will also meets error, and the error will be logged.

The err1 is not important, we just need to get a *expression.Column from a (maybe) string.

coocood · 2018-08-22T07:49:02Z

LGTM

tiancaiamao · 2018-08-22T07:50:22Z

PTAL @zz-jason

tiancaiamao · 2018-08-23T02:35:01Z

PTAL @zz-jason

zz-jason · 2018-08-23T02:39:34Z

plan/logical_plan_test.go

@@ -538,6 +538,17 @@ func (s *testPlanSuite) TestTablePartition(c *C) {
 			best: "Dual->Projection",
 			is:   is1,
 		},
+		{


these kind of tests can not show the detail about the filters, it's better to add some explain tests.

I'll port this one https://github.com/twitter/mysql/blob/master/mysql-test/t/partition_pruning.test
It will certainly cover more cases, but it's a file and I could not do it in this PR.

zz-jason · 2018-08-23T02:40:19Z

table/tables/partition.go

 // UpperBounds: (x < y1); (x < y2); (x < y3)
 type PartitionExpr struct {
+	Column      *expression.Column
 	Ranges      []expression.Expression
 	UpperBounds []expression.Expression
 }

 func generatePartitionExpr(tblInfo *model.TableInfo) (*PartitionExpr, error) {


It's better to add some ut for this function.

shenli · 2018-08-23T02:44:47Z

table/tables/partition.go

@@ -129,6 +131,19 @@ func generatePartitionExpr(tblInfo *model.TableInfo) (*PartitionExpr, error) {

 		if i > 0 {
 			fmt.Fprintf(&buf, " and ((%s) >= (%s))", pi.Expr, pi.Definitions[i-1].LessThan[0])
+		} else {
+			// NULL will locates in the first partition, so its expression is (expr < value or expr is null).


s/locates/locate/

shenli · 2018-08-23T02:45:54Z

table/tables/partition.go

 // UpperBounds: (x < y1); (x < y2); (x < y3)
 type PartitionExpr struct {
+	Column      *expression.Column


Add a comment for the newly added attribute.

tiancaiamao · 2018-08-23T03:43:32Z

PTAL @zz-jason

zz-jason

LGTM

tiancaiamao · 2018-08-23T05:39:07Z

/run-all-tests

plan,table: fix select null value for table partition

dbeffb5

tiancaiamao added the sig/planner SIG: Planner label Aug 21, 2018

tiancaiamao added this to In Progress in Table Partitioning via automation Aug 21, 2018

Merge branch 'master' into select-null

583eba8

coocood reviewed Aug 22, 2018

View reviewed changes

coocood added the status/LGT1 Indicates that a PR has LGTM 1. label Aug 22, 2018

coocood added the status/all tests passed label Aug 22, 2018

zz-jason reviewed Aug 23, 2018

View reviewed changes

shenli reviewed Aug 23, 2018

View reviewed changes

tiancaiamao added 2 commits August 23, 2018 11:39

address comment

73e1fb1

Merge branch 'master' into select-null

78d356f

tiancaiamao force-pushed the select-null branch from ee42cde to 78d356f Compare August 23, 2018 03:41

zz-jason reviewed Aug 23, 2018

View reviewed changes

tiancaiamao added 2 commits August 23, 2018 13:37

fix ci

4fc1d76

Merge branch 'master' into select-null

f79a6f6

tiancaiamao added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Aug 23, 2018

coocood approved these changes Aug 23, 2018

View reviewed changes

coocood merged commit 3ff237c into pingcap:master Aug 23, 2018

Table Partitioning automation moved this from In Progress to Done Aug 23, 2018

tiancaiamao deleted the select-null branch August 23, 2018 06:16

zz-jason moved this from Issue: Done to PR: Donw in Table Partitioning Apr 21, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

plan,table: fix select null value for table partition #7452

plan,table: fix select null value for table partition #7452

tiancaiamao commented Aug 21, 2018

tiancaiamao commented Aug 22, 2018

coocood Aug 22, 2018

tiancaiamao Aug 22, 2018 •

edited

tiancaiamao commented Aug 22, 2018

tiancaiamao commented Aug 22, 2018

coocood Aug 22, 2018

tiancaiamao Aug 22, 2018

coocood commented Aug 22, 2018

tiancaiamao commented Aug 22, 2018

tiancaiamao commented Aug 23, 2018

zz-jason Aug 23, 2018

tiancaiamao Aug 23, 2018

zz-jason Aug 23, 2018

tiancaiamao Aug 23, 2018

shenli Aug 23, 2018

shenli Aug 23, 2018

tiancaiamao commented Aug 23, 2018

zz-jason left a comment

tiancaiamao commented Aug 23, 2018

plan,table: fix select null value for table partition #7452

plan,table: fix select null value for table partition #7452

Conversation

tiancaiamao commented Aug 21, 2018

What problem does this PR solve?

What is changed and how it works?

Check List

tiancaiamao commented Aug 22, 2018

Choose a reason for hiding this comment

tiancaiamao Aug 22, 2018 • edited

Choose a reason for hiding this comment

tiancaiamao commented Aug 22, 2018

tiancaiamao commented Aug 22, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coocood commented Aug 22, 2018

tiancaiamao commented Aug 22, 2018

tiancaiamao commented Aug 23, 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tiancaiamao commented Aug 23, 2018

zz-jason left a comment

Choose a reason for hiding this comment

tiancaiamao commented Aug 23, 2018

tiancaiamao Aug 22, 2018 •

edited