From 0f67a3568d7609a4981e5f34534bee472fbb998a Mon Sep 17 00:00:00 2001 From: AilinKid <314806019@qq.com> Date: Tue, 15 Aug 2023 15:58:07 +0800 Subject: [PATCH 1/3] fix heuristic rule will prune out hint preferred path Signed-off-by: AilinKid <314806019@qq.com> --- planner/core/casetest/enforcempp/enforce_mpp_test.go | 7 +++++++ .../enforcempp/testdata/enforce_mpp_suite_in.json | 3 ++- .../enforcempp/testdata/enforce_mpp_suite_out.json | 11 +++++++++++ planner/core/stats.go | 12 +++++++++++- 4 files changed, 31 insertions(+), 2 deletions(-) diff --git a/planner/core/casetest/enforcempp/enforce_mpp_test.go b/planner/core/casetest/enforcempp/enforce_mpp_test.go index 7378dd71ba38a..eae18eb8f12e6 100644 --- a/planner/core/casetest/enforcempp/enforce_mpp_test.go +++ b/planner/core/casetest/enforcempp/enforce_mpp_test.go @@ -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") @@ -57,6 +58,12 @@ func TestEnforceMPP(t *testing.T) { Available: true, } } + if tblInfo.Name.L == "s" { + tblInfo.TiFlashReplica = &model.TiFlashReplicaInfo{ + Count: 1, + Available: true, + } + } } var input []string diff --git a/planner/core/casetest/enforcempp/testdata/enforce_mpp_suite_in.json b/planner/core/casetest/enforcempp/testdata/enforce_mpp_suite_in.json index f70c8a397ca8b..2d7742ced5fb6 100644 --- a/planner/core/casetest/enforcempp/testdata/enforce_mpp_suite_in.json +++ b/planner/core/casetest/enforcempp/testdata/enforce_mpp_suite_in.json @@ -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" ] }, { diff --git a/planner/core/casetest/enforcempp/testdata/enforce_mpp_suite_out.json b/planner/core/casetest/enforcempp/testdata/enforce_mpp_suite_out.json index 8c48d106eaaa4..86ce6827de66a 100644 --- a/planner/core/casetest/enforcempp/testdata/enforce_mpp_suite_out.json +++ b/planner/core/casetest/enforcempp/testdata/enforce_mpp_suite_out.json @@ -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: 2, 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 } ] }, diff --git a/planner/core/stats.go b/planner/core/stats.go index 14ab62502bd51..4c31c2f1e9b4d 100644 --- a/planner/core/stats.go +++ b/planner/core/stats.go @@ -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" @@ -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 @@ -459,6 +466,9 @@ func (ds *DataSource) DeriveStats(_ []*property.StatsInfo, _ *expression.Schema, if ds.StatsInfo() != nil && len(colGroups) == 0 { return ds.StatsInfo(), nil } + if strings.HasPrefix(ds.SCtx().GetSessionVars().StmtCtx.OriginalSQL, "explain format='verbose' select count(*) from t where a=1") { + fmt.Println(1) + } ds.initStats(colGroups) if ds.StatsInfo() != nil { // Just reload the GroupNDVs. From 435c76b59448ee476f1a69a500f5a2dfa2ba84fd Mon Sep 17 00:00:00 2001 From: AilinKid <314806019@qq.com> Date: Tue, 15 Aug 2023 16:02:18 +0800 Subject: [PATCH 2/3] remove useless code Signed-off-by: AilinKid <314806019@qq.com> --- planner/core/stats.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/planner/core/stats.go b/planner/core/stats.go index 4c31c2f1e9b4d..6f5a073846d44 100644 --- a/planner/core/stats.go +++ b/planner/core/stats.go @@ -466,9 +466,6 @@ func (ds *DataSource) DeriveStats(_ []*property.StatsInfo, _ *expression.Schema, if ds.StatsInfo() != nil && len(colGroups) == 0 { return ds.StatsInfo(), nil } - if strings.HasPrefix(ds.SCtx().GetSessionVars().StmtCtx.OriginalSQL, "explain format='verbose' select count(*) from t where a=1") { - fmt.Println(1) - } ds.initStats(colGroups) if ds.StatsInfo() != nil { // Just reload the GroupNDVs. From 9b5f67d047770db9859f43488a0381a56280bc66 Mon Sep 17 00:00:00 2001 From: AilinKid <314806019@qq.com> Date: Wed, 16 Aug 2023 14:09:21 +0800 Subject: [PATCH 3/3] make fmt Signed-off-by: AilinKid <314806019@qq.com> --- planner/core/casetest/enforcempp/enforce_mpp_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/planner/core/casetest/enforcempp/enforce_mpp_test.go b/planner/core/casetest/enforcempp/enforce_mpp_test.go index eae18eb8f12e6..414604003c08d 100644 --- a/planner/core/casetest/enforcempp/enforce_mpp_test.go +++ b/planner/core/casetest/enforcempp/enforce_mpp_test.go @@ -60,7 +60,7 @@ func TestEnforceMPP(t *testing.T) { } if tblInfo.Name.L == "s" { tblInfo.TiFlashReplica = &model.TiFlashReplicaInfo{ - Count: 1, + Count: 1, Available: true, } }