planner: fix index heuristic rule will prune out hint preferred tifla…

…sh path (#46102) (#46146) close #40146
pingcap · Oct 30, 2023 · 78fc8b4 · 78fc8b4
1 parent 3080406
commit 78fc8b4
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 2 deletions.
diff --git a/planner/core/enforce_mpp_test.go b/planner/core/enforce_mpp_test.go
@@ -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)")
 
 	// Create virtual tiflash replica info.
 	dom := domain.GetDomain(tk.Session())
@@ -108,6 +109,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/stats.go b/planner/core/stats.go
@@ -339,8 +339,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 {

diff --git a/planner/core/testdata/enforce_mpp_suite_in.json b/planner/core/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/testdata/enforce_mpp_suite_out.json b/planner/core/testdata/enforce_mpp_suite_out.json
@@ -180,6 +180,17 @@
           "        └─TableFullScan_20 10000.00 255000.00 mpp[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
       }
     ]
   },