From 2cf7a2c79f6aa3757e7f6fbc934463dbee441e14 Mon Sep 17 00:00:00 2001 From: Mingcong Han Date: Fri, 27 Mar 2020 11:32:14 +0800 Subject: [PATCH] planner: add warning when the table name of indexHint cannot be found (#15517) --- planner/core/integration_test.go | 33 +++++++++++++ planner/core/logical_plan_builder.go | 30 +++++++++++- planner/core/planbuilder.go | 33 ++++++++++++- .../core/testdata/integration_suite_in.json | 12 +++++ .../core/testdata/integration_suite_out.json | 49 +++++++++++++++++++ planner/core/testdata/plan_suite_in.json | 2 + planner/core/testdata/plan_suite_out.json | 22 +++++++-- 7 files changed, 174 insertions(+), 7 deletions(-) diff --git a/planner/core/integration_test.go b/planner/core/integration_test.go index 9561afe216226..a7d9fa14256a8 100644 --- a/planner/core/integration_test.go +++ b/planner/core/integration_test.go @@ -22,6 +22,7 @@ import ( "github.com/pingcap/tidb/domain" "github.com/pingcap/tidb/kv" "github.com/pingcap/tidb/planner/core" + "github.com/pingcap/tidb/sessionctx/stmtctx" "github.com/pingcap/tidb/util/testkit" "github.com/pingcap/tidb/util/testutil" ) @@ -625,6 +626,38 @@ func (s *testIntegrationSuite) TestSubqueryWithTopN(c *C) { } } +func (s *testIntegrationSuite) TestIndexHintWarning(c *C) { + tk := testkit.NewTestKit(c, s.store) + tk.MustExec("use test") + tk.MustExec("drop table if exists t1, t2") + tk.MustExec("create table t1(a int, b int, c int, key a(a))") + tk.MustExec("create table t2(a int, b int, c int, key a(a))") + var input []string + var output []struct { + SQL string + Warnings []string + } + s.testData.GetTestCases(c, &input, &output) + for i, tt := range input { + s.testData.OnRecord(func() { + output[i].SQL = tt + tk.MustQuery(tt) + warns := tk.Se.GetSessionVars().StmtCtx.GetWarnings() + output[i].Warnings = make([]string, len(warns)) + for j := range warns { + output[i].Warnings[j] = warns[j].Err.Error() + } + }) + tk.MustQuery(tt) + warns := tk.Se.GetSessionVars().StmtCtx.GetWarnings() + c.Assert(len(warns), Equals, len(output[i].Warnings)) + for j := range warns { + c.Assert(warns[j].Level, Equals, stmtctx.WarnLevelWarning) + c.Assert(warns[j].Err.Error(), Equals, output[i].Warnings[j]) + } + } +} + func (s *testIntegrationSuite) TestIssue15546(c *C) { tk := testkit.NewTestKit(c, s.store) diff --git a/planner/core/logical_plan_builder.go b/planner/core/logical_plan_builder.go index e14884d8fbd92..a8ea0ee5e5c54 100644 --- a/planner/core/logical_plan_builder.go +++ b/planner/core/logical_plan_builder.go @@ -2251,7 +2251,12 @@ func (b *PlanBuilder) pushTableHints(hints []*ast.TableOptimizerHint, nodeType n } case HintIndexMerge: if len(hint.Tables) != 0 { + dbName := hint.Tables[0].DBName + if dbName.L == "" { + dbName = model.NewCIStr(b.ctx.GetSessionVars().CurrentDB) + } indexMergeHintList = append(indexMergeHintList, indexHintInfo{ + dbName: dbName, tblName: hint.Tables[0].TableName, indexHint: &ast.IndexHint{ IndexNames: hint.Indexes, @@ -2281,6 +2286,8 @@ func (b *PlanBuilder) pushTableHints(hints []*ast.TableOptimizerHint, nodeType n func (b *PlanBuilder) popTableHints() { hintInfo := b.tableHintInfo[len(b.tableHintInfo)-1] + b.appendUnmatchedIndexHintWarning(hintInfo.indexHintList, false) + b.appendUnmatchedIndexHintWarning(hintInfo.indexMergeHintList, true) b.appendUnmatchedJoinHintWarning(HintINLJ, TiDBIndexNestedLoopJoin, hintInfo.indexNestedLoopJoinTables.inljTables) b.appendUnmatchedJoinHintWarning(HintINLHJ, "", hintInfo.indexNestedLoopJoinTables.inlhjTables) b.appendUnmatchedJoinHintWarning(HintINLMJ, "", hintInfo.indexNestedLoopJoinTables.inlmjTables) @@ -2289,6 +2296,26 @@ func (b *PlanBuilder) popTableHints() { b.tableHintInfo = b.tableHintInfo[:len(b.tableHintInfo)-1] } +func (b *PlanBuilder) appendUnmatchedIndexHintWarning(indexHints []indexHintInfo, usedForIndexMerge bool) { + for _, hint := range indexHints { + if !hint.matched { + var hintTypeString string + if usedForIndexMerge { + hintTypeString = "use_index_merge" + } else { + hintTypeString = hint.hintTypeString() + } + errMsg := fmt.Sprintf("%s(%s) is inapplicable, check whether the table(%s.%s) exists", + hintTypeString, + hint.indexString(), + hint.dbName, + hint.tblName, + ) + b.ctx.GetSessionVars().StmtCtx.AppendWarning(ErrInternal.GenWithStack(errMsg)) + } + } +} + func (b *PlanBuilder) appendUnmatchedJoinHintWarning(joinType string, joinTypeAlias string, hintTables []hintTableInfo) { unMatchedTables := extractUnmatchedTables(hintTables) if len(unMatchedTables) == 0 { @@ -2629,9 +2656,10 @@ func (b *PlanBuilder) buildDataSource(ctx context.Context, tn *ast.TableName, as // extract the IndexMergeHint var indexMergeHints []*ast.IndexHint if hints := b.TableHints(); hints != nil { - for _, hint := range hints.indexMergeHintList { + for i, hint := range hints.indexMergeHintList { if hint.tblName.L == tblName.L { indexMergeHints = append(indexMergeHints, hint.indexHint) + hints.indexMergeHintList[i].matched = true } } } diff --git a/planner/core/planbuilder.go b/planner/core/planbuilder.go index 67e98ef31107f..f0afd74046f01 100644 --- a/planner/core/planbuilder.go +++ b/planner/core/planbuilder.go @@ -91,6 +91,36 @@ type indexHintInfo struct { dbName model.CIStr tblName model.CIStr indexHint *ast.IndexHint + // Matched indicates whether this index hint + // has been successfully applied to a DataSource. + // If an indexHintInfo is not matched after building + // a Select statement, we will generate a warning for it. + matched bool +} + +func (hint *indexHintInfo) hintTypeString() string { + switch hint.indexHint.HintType { + case ast.HintUse: + return "use_index" + case ast.HintIgnore: + return "ignore_index" + case ast.HintForce: + return "force_index" + } + return "" +} + +// indexString formats the indexHint as dbName.tableName[, indexNames]. +func (hint *indexHintInfo) indexString() string { + var indexListString string + indexList := make([]string, len(hint.indexHint.IndexNames)) + for i := range hint.indexHint.IndexNames { + indexList[i] = hint.indexHint.IndexNames[i].L + } + if len(indexList) > 0 { + indexListString = fmt.Sprintf(", %s", strings.Join(indexList, ", ")) + } + return fmt.Sprintf("%s.%s%s", hint.dbName, hint.tblName, indexListString) } type aggHintInfo struct { @@ -670,9 +700,10 @@ func (b *PlanBuilder) getPossibleAccessPaths(indexHints []*ast.IndexHint, tbl ta // Extract comment-style index hint like /*+ INDEX(t, idx1, idx2) */. indexHintsLen := len(indexHints) if hints := b.TableHints(); hints != nil { - for _, hint := range hints.indexHintList { + for i, hint := range hints.indexHintList { if hint.dbName.L == dbName.L && hint.tblName.L == tblName.L { indexHints = append(indexHints, hint.indexHint) + hints.indexHintList[i].matched = true } } } diff --git a/planner/core/testdata/integration_suite_in.json b/planner/core/testdata/integration_suite_in.json index 9825d1262b598..2b821d5b7cfb2 100644 --- a/planner/core/testdata/integration_suite_in.json +++ b/planner/core/testdata/integration_suite_in.json @@ -93,5 +93,17 @@ "desc select /*+ TIDB_INLJ(t2)*/ * from t1, t2 where t1.a = t2.a and t1.b = t2.b", "desc select /*+ TIDB_INLJ(t2)*/ * from t1, t2 where t1.a = t2.a and t1.b = t2.a and t1.b = t2.b" ] + }, + { + "name": "TestIndexHintWarning", + "cases": [ + "select /*+ USE_INDEX(t1, j) */ * from t1", + "select /*+ IGNORE_INDEX(t1, j) */ * from t1", + "select /*+ USE_INDEX(t2, a, b, c) */ * from t1", + "select /*+ USE_INDEX(t2) */ * from t1", + "select /*+ USE_INDEX(t1, a), USE_INDEX(t2, a), USE_INDEX(t3, a) */ * from t1, t2 where t1.a=t2.a", + "select /*+ USE_INDEX(t3, a), USE_INDEX(t4, b), IGNORE_INDEX(t3, a) */ * from t1, t2 where t1.a=t2.a", + "select /*+ USE_INDEX_MERGE(t3, a, b, d) */ * from t1" + ] } ] diff --git a/planner/core/testdata/integration_suite_out.json b/planner/core/testdata/integration_suite_out.json index 3da8515fd669e..09249b594a1ac 100644 --- a/planner/core/testdata/integration_suite_out.json +++ b/planner/core/testdata/integration_suite_out.json @@ -389,5 +389,54 @@ ] } ] + }, + { + "Name": "TestIndexHintWarning", + "Cases": [ + { + "SQL": "select /*+ USE_INDEX(t1, j) */ * from t1", + "Warnings": [ + "[planner:1176]Key 'j' doesn't exist in table 't1'" + ] + }, + { + "SQL": "select /*+ IGNORE_INDEX(t1, j) */ * from t1", + "Warnings": [ + "[planner:1176]Key 'j' doesn't exist in table 't1'" + ] + }, + { + "SQL": "select /*+ USE_INDEX(t2, a, b, c) */ * from t1", + "Warnings": [ + "[planner:1815]use_index(test.t2, a, b, c) is inapplicable, check whether the table(test.t2) exists" + ] + }, + { + "SQL": "select /*+ USE_INDEX(t2) */ * from t1", + "Warnings": [ + "[planner:1815]use_index(test.t2) is inapplicable, check whether the table(test.t2) exists" + ] + }, + { + "SQL": "select /*+ USE_INDEX(t1, a), USE_INDEX(t2, a), USE_INDEX(t3, a) */ * from t1, t2 where t1.a=t2.a", + "Warnings": [ + "[planner:1815]use_index(test.t3, a) is inapplicable, check whether the table(test.t3) exists" + ] + }, + { + "SQL": "select /*+ USE_INDEX(t3, a), USE_INDEX(t4, b), IGNORE_INDEX(t3, a) */ * from t1, t2 where t1.a=t2.a", + "Warnings": [ + "[planner:1815]use_index(test.t3, a) is inapplicable, check whether the table(test.t3) exists", + "[planner:1815]use_index(test.t4, b) is inapplicable, check whether the table(test.t4) exists", + "[planner:1815]ignore_index(test.t3, a) is inapplicable, check whether the table(test.t3) exists" + ] + }, + { + "SQL": "select /*+ USE_INDEX_MERGE(t3, a, b, d) */ * from t1", + "Warnings": [ + "[planner:1815]use_index_merge(test.t3, a, b, d) is inapplicable, check whether the table(test.t3) exists" + ] + } + ] } ] diff --git a/planner/core/testdata/plan_suite_in.json b/planner/core/testdata/plan_suite_in.json index 3d284ab5ca4e8..22de8310bbcaa 100644 --- a/planner/core/testdata/plan_suite_in.json +++ b/planner/core/testdata/plan_suite_in.json @@ -58,6 +58,8 @@ "select /*+ USE_INDEX_MERGE(t, primary, f_g) */ * from t where a < 1 or f > 2", "select /*+ USE_INDEX_MERGE(t, primary, f_g, c_d_e) */ * from t where a < 1 or f > 2", "select /*+ NO_INDEX_MERGE(), USE_INDEX_MERGE(t, primary, f_g, c_d_e) */ * from t where a < 1 or f > 2", + "select /*+ USE_INDEX_MERGE(t1, c_d_e, f_g) */ * from t where c < 1 or f > 2", + "select /*+ NO_INDEX_MERGE(), USE_INDEX_MERGE(t, primary, f_g, c_d_e) */ * from t where a < 1 or f > 2", "select /*+ USE_INDEX_MERGE(t) USE_INDEX_MERGE(t) */ * from t where c < 1 or f > 2" ] }, diff --git a/planner/core/testdata/plan_suite_out.json b/planner/core/testdata/plan_suite_out.json index 11a63967cf817..fc48395c00e6a 100644 --- a/planner/core/testdata/plan_suite_out.json +++ b/planner/core/testdata/plan_suite_out.json @@ -102,13 +102,13 @@ { "SQL": "select /*+ USE_INDEX(t, c_d_e) */ * from t t1", "Best": "TableReader(Table(t))", - "HasWarn": false, + "HasWarn": true, "Hints": "USE_INDEX(@`sel_1` `test`.`t1` )" }, { "SQL": "select /*+ IGNORE_INDEX(t, c_d_e) */ t1.c from t t1 order by t1.c", "Best": "IndexReader(Index(t.c_d_e)[[NULL,+inf]])", - "HasWarn": false, + "HasWarn": true, "Hints": "USE_INDEX(@`sel_1` `test`.`t1` `c_d_e`)" }, { @@ -212,6 +212,18 @@ "HasWarn": true, "Hints": "USE_INDEX(@`sel_1` `test`.`t` )" }, + { + "SQL": "select /*+ USE_INDEX_MERGE(t1, c_d_e, f_g) */ * from t where c < 1 or f > 2", + "Best": "TableReader(Table(t)->Sel([or(lt(test.t.c, 1), gt(test.t.f, 2))]))", + "HasWarn": true, + "Hints": "USE_INDEX(@`sel_1` `test`.`t` )" + }, + { + "SQL": "select /*+ NO_INDEX_MERGE(), USE_INDEX_MERGE(t, primary, f_g, c_d_e) */ * from t where a < 1 or f > 2", + "Best": "TableReader(Table(t)->Sel([or(lt(test.t.a, 1), gt(test.t.f, 2))]))", + "HasWarn": true, + "Hints": "USE_INDEX(@`sel_1` `test`.`t` )" + }, { "SQL": "select /*+ USE_INDEX_MERGE(t) USE_INDEX_MERGE(t) */ * from t where c < 1 or f > 2", "Best": "IndexMergeReader(PartialPlans->[Index(t.c_d_e)[[-inf,1)], Index(t.f_g)[(2,+inf]]], TablePlan->Table(t))", @@ -1401,17 +1413,17 @@ { "SQL": "select /*+ AGG_TO_COP(), HASH_AGG(), USE_INDEX(t) */ sum(a) from ta group by a", "Best": "IndexReader(Index(ta.a)[[NULL,+inf]]->HashAgg)->HashAgg", - "Warning": "" + "Warning": "[planner:1815]use_index(test.t) is inapplicable, check whether the table(test.t) exists" }, { "SQL": "select /*+ AGG_TO_COP(), USE_INDEX(t) */ sum(b) from ta group by b", "Best": "TableReader(Table(ta)->HashAgg)->HashAgg", - "Warning": "" + "Warning": "[planner:1815]use_index(test.t) is inapplicable, check whether the table(test.t) exists" }, { "SQL": "select /*+ AGG_TO_COP(), HASH_AGG(), USE_INDEX(t) */ distinct a from ta group by a", "Best": "IndexReader(Index(ta.a)[[NULL,+inf]]->HashAgg)->HashAgg", - "Warning": "" + "Warning": "[planner:1815]use_index(test.t) is inapplicable, check whether the table(test.t) exists" }, { "SQL": "select /*+ AGG_TO_COP(), HASH_AGG(), HASH_JOIN(t1), USE_INDEX(t1), USE_INDEX(t2) */ sum(t1.a) from ta t1, ta t2 where t1.a = t2.b group by t1.a",