Skip to content

Commit

Permalink
planner: fix index heuristic rule will prune out hint preferred tifla…
Browse files Browse the repository at this point in the history
…sh path (#46102) (#46144)

close #40146
  • Loading branch information
ti-chi-bot authored Aug 17, 2023
1 parent b5479c9 commit 07c1326
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 2 deletions.
7 changes: 7 additions & 0 deletions planner/core/enforce_mpp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ func TestEnforceMPP(t *testing.T) {
tk.MustExec("drop table if exists t")
tk.MustExec("create table t(a int, b int)")
tk.MustExec("create index idx on t(a)")
tk.MustExec("CREATE TABLE `s` (\n `a` int(11) DEFAULT NULL,\n `b` int(11) DEFAULT NULL,\n `c` int(11) DEFAULT NULL,\n `d` int(11) DEFAULT NULL,\n UNIQUE KEY `a` (`a`),\n KEY `ii` (`a`,`b`)\n)")

// Default RPC encoding may cause statistics explain result differ and then the test unstable.
tk.MustExec("set @@tidb_enable_chunk_rpc = on")
Expand All @@ -111,6 +112,12 @@ func TestEnforceMPP(t *testing.T) {
Available: true,
}
}
if tblInfo.Name.L == "s" {
tblInfo.TiFlashReplica = &model.TiFlashReplicaInfo{
Count: 1,
Available: true,
}
}
}

var input []string
Expand Down
8 changes: 7 additions & 1 deletion planner/core/stats.go
Original file line number Diff line number Diff line change
Expand Up @@ -363,8 +363,14 @@ func (ds *DataSource) derivePathStatsAndTryHeuristics() error {
selected = uniqueBest
}
}
// If some path matches a heuristic rule, just remove other possible paths
// heuristic rule pruning other path should consider hint prefer.
// If no hints and some path matches a heuristic rule, just remove other possible paths.
if selected != nil {
// if user wanna tiFlash read, while current heuristic choose a TiKV path. so we shouldn't prune other paths.
keep := ds.preferStoreType&preferTiFlash != 0 && selected.StoreType != kv.TiFlash
if keep {
return nil
}
ds.possibleAccessPaths[0] = selected
ds.possibleAccessPaths = ds.possibleAccessPaths[:1]
if ds.ctx.GetSessionVars().StmtCtx.InVerboseExplain {
Expand Down
3 changes: 2 additions & 1 deletion planner/core/testdata/enforce_mpp_suite_in.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@
"set @@tidb_enforce_mpp=1;",
"explain format='verbose' select count(*) from t where a=1",
"explain format='verbose' select /*+ read_from_storage(tikv[t]) */ count(*) from t where a=1",
"explain format='verbose' select /*+ read_from_storage(tiflash[t]) */ count(*) from t where a=1"
"explain format='verbose' select /*+ read_from_storage(tiflash[t]) */ count(*) from t where a=1",
"explain select /*+ READ_FROM_STORAGE(TIFLASH[s]) */ a from s where a = 10 and b is null; -- index path huristic rule will prune tiflash path"
]
},
{
Expand Down
11 changes: 11 additions & 0 deletions planner/core/testdata/enforce_mpp_suite_out.json
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,17 @@
" └─TableFullScan_25 10000.00 928000.00 batchCop[tiflash] table:t keep order:false, stats:pseudo"
],
"Warn": null
},
{
"SQL": "explain select /*+ READ_FROM_STORAGE(TIFLASH[s]) */ a from s where a = 10 and b is null; -- index path huristic rule will prune tiflash path",
"Plan": [
"TableReader_12 0.10 root data:ExchangeSender_11",
"└─ExchangeSender_11 0.10 mpp[tiflash] ExchangeType: PassThrough",
" └─Projection_5 0.10 mpp[tiflash] test.s.a",
" └─Selection_10 0.10 mpp[tiflash] eq(test.s.a, 10), isnull(test.s.b)",
" └─TableFullScan_9 10000.00 mpp[tiflash] table:s keep order:false, stats:pseudo"
],
"Warn": null
}
]
},
Expand Down

0 comments on commit 07c1326

Please sign in to comment.