Skip to content

Commit

Permalink
planner: add warning when the table name of indexHint cannot b… (ping…
Browse files Browse the repository at this point in the history
  • Loading branch information
sre-bot authored Apr 15, 2020
1 parent cedba7f commit 9f1d049
Show file tree
Hide file tree
Showing 8 changed files with 219 additions and 57 deletions.
33 changes: 33 additions & 0 deletions planner/core/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -583,6 +584,38 @@ func (s *testIntegrationSuite) TestTopNByConstFunc(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)

Expand Down
15 changes: 15 additions & 0 deletions planner/core/logical_plan_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -2094,12 +2094,27 @@ func (b *PlanBuilder) pushTableHints(hints []*ast.TableOptimizerHint, nodeType n

func (b *PlanBuilder) popTableHints() {
hintInfo := b.tableHintInfo[len(b.tableHintInfo)-1]
b.appendUnmatchedIndexHintWarning(hintInfo.indexHintList)
b.appendUnmatchedJoinHintWarning(HintINLJ, TiDBIndexNestedLoopJoin, hintInfo.indexNestedLoopJoinTables)
b.appendUnmatchedJoinHintWarning(HintSMJ, TiDBMergeJoin, hintInfo.sortMergeJoinTables)
b.appendUnmatchedJoinHintWarning(HintHJ, TiDBHashJoin, hintInfo.hashJoinTables)
b.tableHintInfo = b.tableHintInfo[:len(b.tableHintInfo)-1]
}

func (b *PlanBuilder) appendUnmatchedIndexHintWarning(indexHints []indexHintInfo) {
for _, hint := range indexHints {
if !hint.matched {
hintTypeString := hint.hintTypeString()
errMsg := fmt.Sprintf("%s(%s) is inapplicable, check whether the table(%s) exists",
hintTypeString,
hint.indexString(),
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 {
Expand Down
70 changes: 16 additions & 54 deletions planner/core/physical_plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -881,69 +881,31 @@ func (s *testPlanSuite) TestIndexHint(c *C) {
_, err = se.Execute(context.Background(), "use test")
c.Assert(err, IsNil)

sessionVars := se.(sessionctx.Context).GetSessionVars()
sessionVars.HashAggFinalConcurrency = 1
sessionVars.HashAggPartialConcurrency = 1

tests := []struct {
sql string
best string
hasWarn bool
}{
// simple case
{
sql: "select /*+ USE_INDEX(t, c_d_e) */ * from t",
best: "IndexLookUp(Index(t.c_d_e)[[NULL,+inf]], Table(t))",
hasWarn: false,
},
{
sql: "select /*+ USE_INDEX(t, c_d_e) */ * from t t1",
best: "TableReader(Table(t))",
hasWarn: false,
},
{
sql: "select /*+ USE_INDEX(t1, c_d_e) */ * from t t1",
best: "IndexLookUp(Index(t.c_d_e)[[NULL,+inf]], Table(t))",
hasWarn: false,
},
{
sql: "select /*+ USE_INDEX(t1, c_d_e), USE_INDEX(t2, f) */ * from t t1, t t2 where t1.a = t2.b",
best: "LeftHashJoin{IndexLookUp(Index(t.c_d_e)[[NULL,+inf]], Table(t))->IndexLookUp(Index(t.f)[[NULL,+inf]], Table(t))}(test.t1.a,test.t2.b)",
hasWarn: false,
},
// test multiple indexes
{
sql: "select /*+ USE_INDEX(t, c_d_e, f, g) */ * from t order by f",
best: "IndexLookUp(Index(t.f)[[NULL,+inf]], Table(t))",
hasWarn: false,
},
// use TablePath when the hint only contains table.
{
sql: "select /*+ USE_INDEX(t) */ f from t where f > 10",
best: "TableReader(Table(t)->Sel([gt(test.t.f, 10)]))",
hasWarn: false,
},
// there will be a warning instead of error when index not exist
{
sql: "select /*+ USE_INDEX(t, no_such_index) */ * from t",
best: "TableReader(Table(t))",
hasWarn: true,
},
var input []string
var output []struct {
SQL string
Best string
HasWarn bool
}
s.testData.GetTestCases(c, &input, &output)
ctx := context.Background()
for i, test := range tests {
comment := Commentf("case:%v sql:%s", i, test.sql)
for i, test := range input {
comment := Commentf("case:%v sql:%s", i, test)
se.GetSessionVars().StmtCtx.SetWarnings(nil)

stmt, err := s.ParseOneStmt(test.sql, "", "")
stmt, err := s.ParseOneStmt(test, "", "")
c.Assert(err, IsNil, comment)

p, err := planner.Optimize(ctx, se, stmt, s.is)
c.Assert(err, IsNil)
c.Assert(core.ToString(p), Equals, test.best, comment)

s.testData.OnRecord(func() {
output[i].SQL = test
output[i].Best = core.ToString(p)
output[i].HasWarn = len(se.GetSessionVars().StmtCtx.GetWarnings()) > 0
})
c.Assert(core.ToString(p), Equals, output[i].Best, comment)
warnings := se.GetSessionVars().StmtCtx.GetWarnings()
if test.hasWarn {
if output[i].HasWarn {
c.Assert(warnings, HasLen, 1, comment)
} else {
c.Assert(warnings, HasLen, 0, comment)
Expand Down
33 changes: 32 additions & 1 deletion planner/core/planbuilder.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,36 @@ type hintTableInfo struct {
type indexHintInfo struct {
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", hint.tblName, indexListString)
}

type aggHintInfo struct {
Expand Down Expand Up @@ -557,9 +587,10 @@ func (b *PlanBuilder) getPossibleAccessPaths(indexHints []*ast.IndexHint, tblInf
// 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.tblName == tblName {
indexHints = append(indexHints, hint.indexHint)
hints.indexHintList[i].matched = true
}
}
}
Expand Down
11 changes: 11 additions & 0 deletions planner/core/testdata/integration_suite_in.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,16 @@
"explain select * from t order by a limit 3",
"select * from t order by a limit 3"
]
},
{
"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"
]
}
]
43 changes: 43 additions & 0 deletions planner/core/testdata/integration_suite_out.json
Original file line number Diff line number Diff line change
Expand Up @@ -108,5 +108,48 @@
]
}
]
},
{
"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(t2, a, b, c) is inapplicable, check whether the table(t2) exists"
]
},
{
"SQL": "select /*+ USE_INDEX(t2) */ * from t1",
"Warnings": [
"[planner:1815]use_index(t2) is inapplicable, check whether the table(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(t3, a) is inapplicable, check whether the table(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(t3, a) is inapplicable, check whether the table(t3) exists",
"[planner:1815]use_index(t4, b) is inapplicable, check whether the table(t4) exists",
"[planner:1815]ignore_index(t3, a) is inapplicable, check whether the table(t3) exists"
]
}
]
}
]
8 changes: 7 additions & 1 deletion planner/core/testdata/plan_suite_in.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,13 @@
// use TablePath when the hint only contains table.
"select /*+ USE_INDEX(t) */ f from t where f > 10",
// there will be a warning instead of error when index not exist
"select /*+ USE_INDEX(t, no_such_index) */ * from t"
"select /*+ USE_INDEX(t, no_such_index) */ * from t",
"select /*+ IGNORE_INDEX(t, no_such_index) */ * from t",
// use both use_index and ignore_index, same as index hints in sql.
"select /*+ USE_INDEX(t, c_d_e), IGNORE_INDEX(t, f) */ c from t order by c",
"select /*+ USE_INDEX(t, f), IGNORE_INDEX(t, f) */ c from t order by c",
"select /*+ USE_INDEX(t, c_d_e), IGNORE_INDEX(t, c_d_e) */ c from t order by c",
"select /*+ USE_INDEX(t, c_d_e, f), IGNORE_INDEX(t, c_d_e) */ c from t order by c"
]
},
{
Expand Down
63 changes: 62 additions & 1 deletion planner/core/testdata/plan_suite_out.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,68 @@
[
{
"Name": "TestIndexHint",
"Cases": null
"Cases": [
{
"SQL": "select /*+ USE_INDEX(t, c_d_e) */ * from t",
"Best": "IndexLookUp(Index(t.c_d_e)[[NULL,+inf]], Table(t))",
"HasWarn": false
},
{
"SQL": "select /*+ USE_INDEX(t, c_d_e) */ * from t t1",
"Best": "TableReader(Table(t))",
"HasWarn": true
},
{
"SQL": "select /*+ USE_INDEX(t1, c_d_e) */ * from t t1",
"Best": "IndexLookUp(Index(t.c_d_e)[[NULL,+inf]], Table(t))",
"HasWarn": false
},
{
"SQL": "select /*+ USE_INDEX(t1, c_d_e), USE_INDEX(t2, f) */ * from t t1, t t2 where t1.a = t2.b",
"Best": "LeftHashJoin{IndexLookUp(Index(t.c_d_e)[[NULL,+inf]], Table(t))->IndexLookUp(Index(t.f)[[NULL,+inf]], Table(t))}(test.t1.a,test.t2.b)",
"HasWarn": false
},
{
"SQL": "select /*+ USE_INDEX(t, c_d_e, f, g) */ * from t order by f",
"Best": "IndexLookUp(Index(t.f)[[NULL,+inf]], Table(t))",
"HasWarn": false
},
{
"SQL": "select /*+ USE_INDEX(t) */ f from t where f > 10",
"Best": "TableReader(Table(t)->Sel([gt(test.t.f, 10)]))",
"HasWarn": false
},
{
"SQL": "select /*+ USE_INDEX(t, no_such_index) */ * from t",
"Best": "TableReader(Table(t))",
"HasWarn": true
},
{
"SQL": "select /*+ IGNORE_INDEX(t, no_such_index) */ * from t",
"Best": "TableReader(Table(t))",
"HasWarn": true
},
{
"SQL": "select /*+ USE_INDEX(t, c_d_e), IGNORE_INDEX(t, f) */ c from t order by c",
"Best": "IndexReader(Index(t.c_d_e)[[NULL,+inf]])",
"HasWarn": false
},
{
"SQL": "select /*+ USE_INDEX(t, f), IGNORE_INDEX(t, f) */ c from t order by c",
"Best": "TableReader(Table(t))->Sort",
"HasWarn": false
},
{
"SQL": "select /*+ USE_INDEX(t, c_d_e), IGNORE_INDEX(t, c_d_e) */ c from t order by c",
"Best": "TableReader(Table(t))->Sort",
"HasWarn": false
},
{
"SQL": "select /*+ USE_INDEX(t, c_d_e, f), IGNORE_INDEX(t, c_d_e) */ c from t order by c",
"Best": "IndexLookUp(Index(t.f)[[NULL,+inf]], Table(t))->Sort",
"HasWarn": false
}
]
},
{
"Name": "TestDAGPlanBuilderSimpleCase",
Expand Down

0 comments on commit 9f1d049

Please sign in to comment.