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) (#46145)

close #40146
  • Loading branch information
ti-chi-bot authored Oct 16, 2023
1 parent f7b0710 commit 981fc3b
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 2 deletions.
7 changes: 7 additions & 0 deletions planner/core/casetest/enforce_mpp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,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 @@ -55,6 +56,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
3 changes: 2 additions & 1 deletion planner/core/casetest/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/casetest/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 pushed down filter:empty, 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 MppVersion: 1, 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] isnull(test.s.b)",
" └─TableFullScan_9 10.00 mpp[tiflash] table:s pushed down filter:eq(test.s.a, 10), keep order:false, stats:pseudo"
],
"Warn": null
}
]
},
Expand Down
9 changes: 8 additions & 1 deletion planner/core/stats.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/pingcap/errors"
"github.com/pingcap/tidb/domain"
"github.com/pingcap/tidb/expression"
"github.com/pingcap/tidb/kv"
"github.com/pingcap/tidb/parser/model"
"github.com/pingcap/tidb/parser/mysql"
"github.com/pingcap/tidb/planner/property"
Expand Down Expand Up @@ -415,8 +416,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]
var tableName string
Expand Down

0 comments on commit 981fc3b

Please sign in to comment.