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

…sh path (#46102) close #40146
pingcap · Aug 16, 2023 · 0e1b642 · 0e1b642
1 parent 8822578
commit 0e1b642
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 2 deletions.
diff --git 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
@@ -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
@@ -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
@@ -25,6 +25,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