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

planner: regard NULL as point when accessing composite index (#30244) #30614

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
4 changes: 2 additions & 2 deletions executor/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -4041,7 +4041,7 @@ func buildRangesForIndexJoin(ctx sessionctx.Context, lookUpContents []*indexJoin
return retRanges, nil
}

return ranger.UnionRanges(ctx.GetSessionVars().StmtCtx, tmpDatumRanges, true)
return ranger.UnionRanges(ctx, tmpDatumRanges, true)
}

// buildKvRangesForIndexJoin builds kv ranges for index join when the inner plan is index scan plan.
Expand Down Expand Up @@ -4095,7 +4095,7 @@ func buildKvRangesForIndexJoin(ctx sessionctx.Context, tableID, indexID int64, l
return kvRanges, nil
}

tmpDatumRanges, err = ranger.UnionRanges(ctx.GetSessionVars().StmtCtx, tmpDatumRanges, true)
tmpDatumRanges, err = ranger.UnionRanges(ctx, tmpDatumRanges, true)
if err != nil {
return nil, err
}
Expand Down
20 changes: 15 additions & 5 deletions expression/builtin_time.go
Original file line number Diff line number Diff line change
Expand Up @@ -7188,11 +7188,21 @@ func CalAppropriateTime(minTime, maxTime, minSafeTime time.Time) time.Time {
// 2. If t2 < t, we will use t2 as the result,
// and with it, a read request won't fail because it's bigger than the latest SafeTS.
func calAppropriateTime(minTime, maxTime, minSafeTime time.Time) time.Time {
if minSafeTime.Before(minTime) {
return minTime
} else if minSafeTime.After(maxTime) {
return maxTime
}
if minSafeTime.Before(minTime) || minSafeTime.After(maxTime) {
logutil.BgLogger().Warn("calAppropriateTime",
zap.Time("minTime", minTime),
zap.Time("maxTime", maxTime),
zap.Time("minSafeTime", minSafeTime))
if minSafeTime.Before(minTime) {
return minTime
} else if minSafeTime.After(maxTime) {
return maxTime
}
}
logutil.BgLogger().Debug("calAppropriateTime",
zap.Time("minTime", minTime),
zap.Time("maxTime", maxTime),
zap.Time("minSafeTime", minSafeTime))
return minSafeTime
}

Expand Down
10 changes: 5 additions & 5 deletions planner/core/common_plans.go
Original file line number Diff line number Diff line change
Expand Up @@ -606,7 +606,7 @@ func (e *Execute) rebuildRange(p Plan) error {
}
}
if pkCol != nil {
ranges, err := ranger.BuildTableRange(x.AccessConditions, x.ctx.GetSessionVars().StmtCtx, pkCol.RetType)
ranges, err := ranger.BuildTableRange(x.AccessConditions, x.ctx, pkCol.RetType)
if err != nil {
return err
}
Expand Down Expand Up @@ -656,7 +656,7 @@ func (e *Execute) rebuildRange(p Plan) error {
}
}
if pkCol != nil {
ranges, err := ranger.BuildTableRange(x.AccessConditions, x.ctx.GetSessionVars().StmtCtx, pkCol.RetType)
ranges, err := ranger.BuildTableRange(x.AccessConditions, x.ctx, pkCol.RetType)
if err != nil {
return err
}
Expand Down Expand Up @@ -750,7 +750,7 @@ func (e *Execute) buildRangeForTableScan(sctx sessionctx.Context, ts *PhysicalTa
}
}
if pkCol != nil {
ts.Ranges, err = ranger.BuildTableRange(ts.AccessCondition, sctx.GetSessionVars().StmtCtx, pkCol.RetType)
ts.Ranges, err = ranger.BuildTableRange(ts.AccessCondition, sctx, pkCol.RetType)
if err != nil {
return err
}
Expand Down Expand Up @@ -1460,10 +1460,10 @@ func IsPointGetWithPKOrUniqueKeyByAutoCommit(ctx sessionctx.Context, p Plan) (bo
switch v := p.(type) {
case *PhysicalIndexReader:
indexScan := v.IndexPlans[0].(*PhysicalIndexScan)
return indexScan.IsPointGetByUniqueKey(ctx.GetSessionVars().StmtCtx), nil
return indexScan.IsPointGetByUniqueKey(ctx), nil
case *PhysicalTableReader:
tableScan := v.TablePlans[0].(*PhysicalTableScan)
isPointRange := len(tableScan.Ranges) == 1 && tableScan.Ranges[0].IsPoint(ctx.GetSessionVars().StmtCtx)
isPointRange := len(tableScan.Ranges) == 1 && tableScan.Ranges[0].IsPointNonNullable(ctx)
if !isPointRange {
return false, nil
}
Expand Down
7 changes: 3 additions & 4 deletions planner/core/exhaust_physical_plans.go
Original file line number Diff line number Diff line change
Expand Up @@ -1205,7 +1205,7 @@ func (cwc *ColWithCmpFuncManager) BuildRangesByRow(ctx sessionctx.Context, row c
}
exprs = append(exprs, newExpr)
}
ranges, err := ranger.BuildColumnRange(exprs, ctx.GetSessionVars().StmtCtx, cwc.TargetCol.RetType, cwc.colLength)
ranges, err := ranger.BuildColumnRange(exprs, ctx, cwc.TargetCol.RetType, cwc.colLength)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -1449,7 +1449,7 @@ func (ijHelper *indexJoinBuildHelper) analyzeLookUpFilters(path *util.AccessPath
var ranges, nextColRange []*ranger.Range
var err error
if len(colAccesses) > 0 {
nextColRange, err = ranger.BuildColumnRange(colAccesses, ijHelper.join.ctx.GetSessionVars().StmtCtx, lastPossibleCol.RetType, path.IdxColLens[lastColPos])
nextColRange, err = ranger.BuildColumnRange(colAccesses, ijHelper.join.ctx, lastPossibleCol.RetType, path.IdxColLens[lastColPos])
if err != nil {
return false, err
}
Expand Down Expand Up @@ -1551,14 +1551,13 @@ func (ijHelper *indexJoinBuildHelper) buildTemplateRange(matchedKeyCnt int, eqAn
HighVal: make([]types.Datum, pointLength),
})
}
sc := ijHelper.join.ctx.GetSessionVars().StmtCtx
for i, j := 0, 0; j < len(eqAndInFuncs); i++ {
// This position is occupied by join key.
if ijHelper.curIdxOff2KeyOff[i] != -1 {
continue
}
exprs := []expression.Expression{eqAndInFuncs[j]}
oneColumnRan, err := ranger.BuildColumnRange(exprs, sc, ijHelper.curNotUsedIndexCols[j].RetType, ijHelper.curNotUsedColLens[j])
oneColumnRan, err := ranger.BuildColumnRange(exprs, ijHelper.join.ctx, ijHelper.curNotUsedIndexCols[j].RetType, ijHelper.curNotUsedColLens[j])
if err != nil {
return nil, false, err
}
Expand Down
5 changes: 3 additions & 2 deletions 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.GetSessionVars().StmtCtx) {
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 Expand Up @@ -1581,7 +1582,7 @@ func (ds *DataSource) crossEstimateRowCount(path *util.AccessPath, conds []expre
return 0, false, corr
}
sc := ds.ctx.GetSessionVars().StmtCtx
ranges, err := ranger.BuildColumnRange(accessConds, sc, col.RetType, types.UnspecifiedLength)
ranges, err := ranger.BuildColumnRange(accessConds, ds.ctx, col.RetType, types.UnspecifiedLength)
if len(ranges) == 0 || err != nil {
return 0, err == nil, corr
}
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
2 changes: 1 addition & 1 deletion planner/core/logical_plans.go
Original file line number Diff line number Diff line change
Expand Up @@ -828,7 +828,7 @@ func (ds *DataSource) deriveTablePathStats(path *util.AccessPath, conds []expres
path.CountAfterAccess = 1
return nil
}
path.Ranges, err = ranger.BuildTableRange(path.AccessConds, sc, pkCol.RetType)
path.Ranges, err = ranger.BuildTableRange(path.AccessConds, ds.ctx, pkCol.RetType)
if err != nil {
return err
}
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
7 changes: 3 additions & 4 deletions planner/core/physical_plans.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import (
"github.com/pingcap/tidb/planner/property"
"github.com/pingcap/tidb/planner/util"
"github.com/pingcap/tidb/sessionctx"
"github.com/pingcap/tidb/sessionctx/stmtctx"
"github.com/pingcap/tidb/statistics"
"github.com/pingcap/tidb/table"
"github.com/pingcap/tidb/table/tables"
Expand Down Expand Up @@ -548,7 +547,7 @@ func (ts *PhysicalTableScan) ResolveCorrelatedColumns() ([]*ranger.Range, error)
} else {
var err error
pkTP := ts.Table.GetPkColInfo().FieldType
ts.Ranges, err = ranger.BuildTableRange(access, ts.SCtx().GetSessionVars().StmtCtx, &pkTP)
ts.Ranges, err = ranger.BuildTableRange(access, ts.SCtx(), &pkTP)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -1196,11 +1195,11 @@ func (p *PhysicalIndexScan) IsPartition() (bool, int64) {
}

// IsPointGetByUniqueKey checks whether is a point get by unique key.
func (p *PhysicalIndexScan) IsPointGetByUniqueKey(sc *stmtctx.StatementContext) bool {
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(sc)
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
6 changes: 3 additions & 3 deletions planner/core/rule_partition_processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ func (s *partitionProcessor) findUsedPartitions(ctx sessionctx.Context, tbl tabl
ranges := detachedResult.Ranges
used := make([]int, 0, len(ranges))
for _, r := range ranges {
if r.IsPointNullable(ctx.GetSessionVars().StmtCtx) {
if r.IsPointNullable(ctx) {
if !r.HighVal[0].IsNull() {
if len(r.HighVal) != len(partIdx) {
used = []int{-1}
Expand Down Expand Up @@ -472,7 +472,7 @@ func (l *listPartitionPruner) locateColumnPartitionsByCondition(cond expression.
return nil, true, nil
}
var locations []tables.ListPartitionLocation
if r.IsPointNullable(sc) {
if r.IsPointNullable(l.ctx) {
location, err := colPrune.LocatePartition(sc, r.HighVal[0])
if types.ErrOverflow.Equal(err) {
return nil, true, nil // return full-scan if over-flow
Expand Down Expand Up @@ -554,7 +554,7 @@ func (l *listPartitionPruner) findUsedListPartitions(conds []expression.Expressi
}
used := make(map[int]struct{}, len(ranges))
for _, r := range ranges {
if r.IsPointNullable(l.ctx.GetSessionVars().StmtCtx) {
if r.IsPointNullable(l.ctx) {
if len(r.HighVal) != len(exprCols) {
return l.fullRange, nil
}
Expand Down
9 changes: 4 additions & 5 deletions planner/core/stats.go
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ func (ds *DataSource) derivePathStatsAndTryHeuristics() error {
selected = path
break
}
if path.OnlyPointRange(ds.SCtx().GetSessionVars().StmtCtx) {
if path.OnlyPointRange(ds.SCtx()) {
if path.IsTablePath() || path.Index.Unique {
if path.IsSingleScan {
selected = path
Expand Down Expand Up @@ -476,11 +476,10 @@ func (ts *LogicalTableScan) DeriveStats(childStats []*property.StatsInfo, selfSc
ts.AccessConds[i] = expression.PushDownNot(ts.ctx, expr)
}
ts.stats = ts.Source.deriveStatsByFilter(ts.AccessConds, nil)
sc := ts.SCtx().GetSessionVars().StmtCtx
// ts.Handle could be nil if PK is Handle, and PK column has been pruned.
// TODO: support clustered index.
if ts.HandleCols != nil {
ts.Ranges, err = ranger.BuildTableRange(ts.AccessConds, sc, ts.HandleCols.GetCol(0).RetType)
ts.Ranges, err = ranger.BuildTableRange(ts.AccessConds, ts.ctx, ts.HandleCols.GetCol(0).RetType)
} else {
isUnsigned := false
if ts.Source.tableInfo.PKIsHandle {
Expand Down Expand Up @@ -632,7 +631,7 @@ func (ds *DataSource) accessPathsForConds(conditions []expression.Expression, us
continue
}
// If we have point or empty range, just remove other possible paths.
if len(path.Ranges) == 0 || path.OnlyPointRange(ds.SCtx().GetSessionVars().StmtCtx) {
if len(path.Ranges) == 0 || path.OnlyPointRange(ds.SCtx()) {
if len(results) == 0 {
results = append(results, path)
} else {
Expand All @@ -657,7 +656,7 @@ func (ds *DataSource) accessPathsForConds(conditions []expression.Expression, us
continue
}
// If we have empty range, or point range on unique index, just remove other possible paths.
if len(path.Ranges) == 0 || (path.OnlyPointRange(ds.SCtx().GetSessionVars().StmtCtx) && path.Index.Unique) {
if len(path.Ranges) == 0 || (path.OnlyPointRange(ds.SCtx()) && path.Index.Unique) {
if len(results) == 0 {
results = append(results, path)
} else {
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