Skip to content

Commit

Permalink
planner: regard NULL as point when accessing composite index (pingcap…
Browse files Browse the repository at this point in the history
  • Loading branch information
qw4990 committed Dec 10, 2021
1 parent cc52a0a commit a4a63ee
Show file tree
Hide file tree
Showing 15 changed files with 354 additions and 9 deletions.
2 changes: 2 additions & 0 deletions distsql/request_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -683,6 +683,8 @@ func encodeIndexKey(sc *stmtctx.StatementContext, ran *ranger.Range) ([]byte, []
}
}

// NOTE: this is a hard-code operation to avoid wrong results when accessing unique index with NULL;
// Please see https://github.com/pingcap/tidb/issues/29650 for more details
if hasNull {
// Append 0 to make unique-key range [null, null] to be a scan rather than point-get.
high = kv.Key(high).Next()
Expand Down
2 changes: 1 addition & 1 deletion planner/core/common_plans.go
Original file line number Diff line number Diff line change
Expand Up @@ -1463,7 +1463,7 @@ func IsPointGetWithPKOrUniqueKeyByAutoCommit(ctx sessionctx.Context, p Plan) (bo
return indexScan.IsPointGetByUniqueKey(ctx), nil
case *PhysicalTableReader:
tableScan := v.TablePlans[0].(*PhysicalTableScan)
isPointRange := len(tableScan.Ranges) == 1 && tableScan.Ranges[0].IsPoint(ctx)
isPointRange := len(tableScan.Ranges) == 1 && tableScan.Ranges[0].IsPointNonNullable(ctx)
if !isPointRange {
return false, nil
}
Expand Down
3 changes: 2 additions & 1 deletion planner/core/find_best_task.go
Original file line number Diff line number Diff line change
Expand Up @@ -860,7 +860,8 @@ func (ds *DataSource) findBestTask(prop *property.PhysicalProperty, planCounter
if canConvertPointGet {
allRangeIsPoint := true
for _, ran := range path.Ranges {
if !ran.IsPoint(ds.ctx) {
if !ran.IsPointNonNullable(ds.ctx) {
// unique indexes can have duplicated NULL rows so we cannot use PointGet if there is NULL
allRangeIsPoint = false
break
}
Expand Down
50 changes: 50 additions & 0 deletions planner/core/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4751,6 +4751,56 @@ func (s *testIntegrationSerialSuite) TestRejectSortForMPP(c *C) {
}
}

func (s *testIntegrationSerialSuite) TestRegardNULLAsPoint(c *C) {
tk := testkit.NewTestKit(c, s.store)
tk.MustExec("use test")

tk.MustExec("drop table if exists tpk")
tk.MustExec(`create table tuk (a int, b int, c int, unique key (a, b, c))`)
tk.MustExec(`create table tik (a int, b int, c int, key (a, b, c))`)
for _, va := range []string{"NULL", "1"} {
for _, vb := range []string{"NULL", "1"} {
for _, vc := range []string{"NULL", "1"} {
tk.MustExec(fmt.Sprintf(`insert into tuk values (%v, %v, %v)`, va, vb, vc))
tk.MustExec(fmt.Sprintf(`insert into tik values (%v, %v, %v)`, va, vb, vc))
if va == "1" && vb == "1" && vc == "1" {
continue
}
// duplicated NULL rows
tk.MustExec(fmt.Sprintf(`insert into tuk values (%v, %v, %v)`, va, vb, vc))
tk.MustExec(fmt.Sprintf(`insert into tik values (%v, %v, %v)`, va, vb, vc))
}
}
}

var input []string
var output []struct {
SQL string
PlanEnabled []string
PlanDisabled []string
Result []string
}
s.testData.GetTestCases(c, &input, &output)
for i, tt := range input {
s.testData.OnRecord(func() {
output[i].SQL = tt
tk.MustExec(`set @@session.tidb_regard_null_as_point=true`)
output[i].PlanEnabled = s.testData.ConvertRowsToStrings(tk.MustQuery("explain " + tt).Rows())
output[i].Result = s.testData.ConvertRowsToStrings(tk.MustQuery(tt).Rows())

tk.MustExec(`set @@session.tidb_regard_null_as_point=false`)
output[i].PlanDisabled = s.testData.ConvertRowsToStrings(tk.MustQuery("explain " + tt).Rows())
})
tk.MustExec(`set @@session.tidb_regard_null_as_point=true`)
tk.MustQuery("explain " + tt).Check(testkit.Rows(output[i].PlanEnabled...))
tk.MustQuery(tt).Check(testkit.Rows(output[i].Result...))

tk.MustExec(`set @@session.tidb_regard_null_as_point=false`)
tk.MustQuery("explain " + tt).Check(testkit.Rows(output[i].PlanDisabled...))
tk.MustQuery(tt).Check(testkit.Rows(output[i].Result...))
}
}

func (s *testIntegrationSuite) TestIssues29711(c *C) {
tk := testkit.NewTestKit(c, s.store)
tk.MustExec("use test")
Expand Down
3 changes: 3 additions & 0 deletions planner/core/partition_pruner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ func (s *testPartitionPruneSuit) TestListPartitionPruner(c *C) {
tk.MustExec("use test_partition")
tk.Se.GetSessionVars().EnableClusteredIndex = variable.ClusteredIndexDefModeIntOnly
tk.MustExec("set @@session.tidb_enable_list_partition = ON")
tk.MustExec(`set @@session.tidb_regard_null_as_point=false`)
tk.MustExec("create table t1 (id int, a int, b int ) partition by list ( a ) (partition p0 values in (1,2,3,4,5), partition p1 values in (6,7,8,9,10,null));")
tk.MustExec("create table t2 (a int, id int, b int) partition by list (a*3 + b - 2*a - b) (partition p0 values in (1,2,3,4,5), partition p1 values in (6,7,8,9,10,null));")
tk.MustExec("create table t3 (b int, id int, a int) partition by list columns (a) (partition p0 values in (1,2,3,4,5), partition p1 values in (6,7,8,9,10,null));")
Expand Down Expand Up @@ -169,6 +170,7 @@ func (s *testPartitionPruneSuit) TestListColumnsPartitionPruner(c *C) {
// tk1 use to test partition table with index.
tk1 := testkit.NewTestKit(c, s.store)
tk1.MustExec("drop database if exists test_partition_1;")
tk1.MustExec(`set @@session.tidb_regard_null_as_point=false`)
tk1.MustExec("create database test_partition_1")
tk1.MustExec("use test_partition_1")
tk1.MustExec("set @@session.tidb_enable_list_partition = ON")
Expand All @@ -180,6 +182,7 @@ func (s *testPartitionPruneSuit) TestListColumnsPartitionPruner(c *C) {
// tk2 use to compare the result with normal table.
tk2 := testkit.NewTestKit(c, s.store)
tk2.MustExec("drop database if exists test_partition_2;")
tk2.MustExec(`set @@session.tidb_regard_null_as_point=false`)
tk2.MustExec("create database test_partition_2")
tk2.MustExec("use test_partition_2")
tk2.MustExec("create table t1 (id int, a int, b int)")
Expand Down
2 changes: 1 addition & 1 deletion planner/core/physical_plans.go
Original file line number Diff line number Diff line change
Expand Up @@ -1199,7 +1199,7 @@ func (p *PhysicalIndexScan) IsPointGetByUniqueKey(sctx sessionctx.Context) bool
return len(p.Ranges) == 1 &&
p.Index.Unique &&
len(p.Ranges[0].LowVal) == len(p.Index.Columns) &&
p.Ranges[0].IsPoint(sctx)
p.Ranges[0].IsPointNonNullable(sctx)
}

// PhysicalSelection represents a filter.
Expand Down
12 changes: 12 additions & 0 deletions planner/core/plan_to_pb.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,18 @@ func checkCoverIndex(idx *model.IndexInfo, ranges []*ranger.Range) bool {
if len(rg.LowVal) != len(idx.Columns) {
return false
}
for _, v := range rg.LowVal {
if v.IsNull() {
// a unique index may have duplicated rows with NULLs, so we cannot set the unique attribute to true when the range has NULL
// please see https://github.com/pingcap/tidb/issues/29650 for more details
return false
}
}
for _, v := range rg.HighVal {
if v.IsNull() {
return false
}
}
}
return true
}
Expand Down
19 changes: 19 additions & 0 deletions planner/core/testdata/integration_serial_suite_in.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,25 @@
]

},
{
"name": "TestRegardNULLAsPoint",
"cases": [
"select * from tuk where a<=>null and b=1",
"select * from tik where a<=>null and b=1",
"select * from tuk where a<=>null and b>0 and b<2",
"select * from tik where a<=>null and b>0 and b<2",
"select * from tuk where a<=>null and b>=1 and b<2",
"select * from tik where a<=>null and b>=1 and b<2",
"select * from tuk where a<=>null and b=1 and c=1",
"select * from tik where a<=>null and b=1 and c=1",
"select * from tuk where a=1 and b<=>null and c=1",
"select * from tik where a=1 and b<=>null and c=1",
"select * from tuk where a<=>null and b<=>null and c=1",
"select * from tik where a<=>null and b<=>null and c=1",
"select * from tuk where a<=>null and b<=>null and c<=>null",
"select * from tik where a<=>null and b<=>null and c<=>null"
]
},
{
"name": "TestPushDownToTiFlashWithKeepOrder",
"cases": [
Expand Down
Loading

0 comments on commit a4a63ee

Please sign in to comment.