Skip to content

Commit

Permalink
planner: forbid point get with select lock choose tiflash as datasour…
Browse files Browse the repository at this point in the history
…ce path (#39388)

close #39344
  • Loading branch information
Yisaer authored Nov 29, 2022
1 parent 4476173 commit cca78e3
Show file tree
Hide file tree
Showing 2 changed files with 102 additions and 0 deletions.
68 changes: 68 additions & 0 deletions planner/core/find_best_task.go
Original file line number Diff line number Diff line change
Expand Up @@ -912,6 +912,14 @@ func (ds *DataSource) findBestTask(prop *property.PhysicalProperty, planCounter
p: dual,
}, cntPlan, nil
}

// if the path is the point get range path with for update lock, we should forbid tiflash as it's store path.
if path.StoreType == kv.TiFlash && ds.isForUpdateRead && ds.ctx.GetSessionVars().TxnCtx.IsExplicit {
if ds.isPointGetConditions() {
continue
}
}

canConvertPointGet := len(path.Ranges) > 0 && path.StoreType == kv.TiKV && ds.isPointGetConvertableSchema()

if canConvertPointGet && expression.MaybeOverOptimized4PlanCache(ds.ctx, path.AccessConds) {
Expand Down Expand Up @@ -1903,6 +1911,66 @@ func (s *LogicalIndexScan) GetPhysicalIndexScan(_ *expression.Schema, stats *pro
return is
}

// isPointGetConditions indicates whether the conditions are point-get-able.
// eg: create table t(a int, b int,c int unique, primary (a,b))
// select * from t where a = 1 and b = 1 and c =1;
// the datasource can access by primary key(a,b) or unique key (c) which are both point-get-able
func (ds *DataSource) isPointGetConditions() bool {
t, _ := ds.is.TableByID(ds.physicalTableID)
columns := map[string]struct{}{}
for _, cond := range ds.allConds {
s, ok := cond.(*expression.ScalarFunction)
if !ok {
return false
}
if s.FuncName.L != ast.EQ || (s.FuncName.L == ast.In && len(s.GetArgs()) != 2) {
return false
}
arg0 := s.GetArgs()[0]
arg1 := s.GetArgs()[1]
_, ok1 := arg0.(*expression.Constant)
col, ok2 := arg1.(*expression.Column)
if ok1 && ok2 {
columns[t.Meta().FindColumnNameByID(col.ID)] = struct{}{}
continue
}
col, ok1 = arg0.(*expression.Column)
_, ok2 = arg1.(*expression.Constant)
if ok1 && ok2 {
columns[t.Meta().FindColumnNameByID(col.ID)] = struct{}{}
continue
}
}
return ds.findPKOrUniqueIndexMatchColumns(columns)
}

func (ds *DataSource) findPKOrUniqueIndexMatchColumns(columns map[string]struct{}) bool {
for _, idx := range ds.tableInfo.Indices {
if !idx.Unique && !idx.Primary {
continue
}
if idx.HasPrefixIndex() {
continue
}
if len(idx.Columns) > len(columns) {
continue
}
flag := true
for _, idxCol := range idx.Columns {
_, ok := columns[idxCol.Name.String()]
if !ok {
flag = false
break
}
}
if !flag {
continue
}
return true
}
return false
}

// convertToTableScan converts the DataSource to table scan.
func (ds *DataSource) convertToTableScan(prop *property.PhysicalProperty, candidate *candidatePath, _ *physicalOptimizeOp) (task task, err error) {
// It will be handled in convertToIndexScan.
Expand Down
34 changes: 34 additions & 0 deletions planner/core/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7556,6 +7556,40 @@ func TestEnableTiFlashReadForWriteStmt(t *testing.T) {
checkMpp(rs)
}

func TestPointGetWithSelectLock(t *testing.T) {
store, dom := testkit.CreateMockStoreAndDomain(t)
tk := testkit.NewTestKit(t, store)
tk.MustExec("use test")
tk.MustExec("create table t(a int, b int, primary key(a, b));")
tk.MustExec("create table t1(c int unique, d int);")
tbl, err := dom.InfoSchema().TableByName(model.CIStr{O: "test", L: "test"}, model.CIStr{O: "t", L: "t"})
require.NoError(t, err)
// Set the hacked TiFlash replica for explain tests.
tbl.Meta().TiFlashReplica = &model.TiFlashReplicaInfo{Count: 1, Available: true}
tbl1, err := dom.InfoSchema().TableByName(model.CIStr{O: "test", L: "test"}, model.CIStr{O: "t1", L: "t1"})
require.NoError(t, err)
// Set the hacked TiFlash replica for explain tests.
tbl1.Meta().TiFlashReplica = &model.TiFlashReplicaInfo{Count: 1, Available: true}

tk.MustExec("set @@tidb_enable_tiflash_read_for_write_stmt = on;")
tk.MustExec("set @@tidb_isolation_read_engines='tidb,tiflash';")
tk.MustExec("begin;")
// assert point get condition with txn commit and tiflash store
tk.MustGetErrMsg("explain select a, b from t where a = 1 and b = 2 for update;", "[planner:1815]Internal : Can't find a proper physical plan for this query")
tk.MustGetErrMsg("explain select c, d from t1 where c = 1 for update;", "[planner:1815]Internal : Can't find a proper physical plan for this query")
tk.MustGetErrMsg("explain select c, d from t1 where c = 1 and d = 1 for update;", "[planner:1815]Internal : Can't find a proper physical plan for this query")
tk.MustQuery("explain select a, b from t where a = 1 for update;")
tk.MustQuery("explain select c, d from t1 where c > 1 for update;")
tk.MustExec("set tidb_isolation_read_engines='tidb,tikv,tiflash';")
tk.MustQuery("explain select a, b from t where a = 1 and b = 2 for update;")
tk.MustQuery("explain select c, d from t1 where c = 1 for update;")
tk.MustExec("commit")
tk.MustExec("set tidb_isolation_read_engines='tidb,tiflash';")
// assert point get condition with auto commit and tiflash store
tk.MustQuery("explain select a, b from t where a = 1 and b = 2 for update;")
tk.MustQuery("explain select c, d from t1 where c = 1 for update;")
}

func TestTableRangeFallback(t *testing.T) {
store := testkit.CreateMockStore(t)
tk := testkit.NewTestKit(t, store)
Expand Down

0 comments on commit cca78e3

Please sign in to comment.