Skip to content

Commit

Permalink
planner: add warning when join hint has no arguments (pingcap#15583)
Browse files Browse the repository at this point in the history
  • Loading branch information
@francis0407 @sre-bot
francis0407 authored and sre-bot committed Mar 30, 2020
1 parent 98fea8d commit 3f4a3d5
Show file tree
Hide file tree
Showing 4 changed files with 168 additions and 42 deletions.
32 changes: 32 additions & 0 deletions planner/core/integration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -710,3 +710,35 @@ func (s *testIntegrationSuite) TestIssue15546(c *C) {
tk.MustExec("create definer='root'@'localhost' view vt(a, b) as select a, b from t")
tk.MustQuery("select * from pt, vt where pt.a = vt.a").Check(testkit.Rows("1 1 1 1"))
}

func (s *testIntegrationSuite) TestHintWithoutTableWarning(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])
}
}
}
98 changes: 56 additions & 42 deletions planner/core/logical_plan_builder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2185,6 +2185,16 @@ func (b *PlanBuilder) unfoldWildStar(p LogicalPlan, selectFields []*ast.SelectFi
return resultList, nil
}

func (b *PlanBuilder) pushHintWithoutTableWarning(hint *ast.TableOptimizerHint) {
var sb strings.Builder
ctx := format.NewRestoreCtx(0, &sb)
if err := hint.Restore(ctx); err != nil {
return
}
errMsg := fmt.Sprintf("Hint %s is inapplicable. Please specify the table names in the arguments.", sb.String())
b.ctx.GetSessionVars().StmtCtx.AppendWarning(ErrInternal.GenWithStack(errMsg))
}

func (b *PlanBuilder) pushTableHints(hints []*ast.TableOptimizerHint, nodeType nodeType, currentLevel int) {
hints = b.hintProcessor.getCurrentStmtHints(hints, nodeType, currentLevel)
var (
Expand All @@ -2195,6 +2205,16 @@ func (b *PlanBuilder) pushTableHints(hints []*ast.TableOptimizerHint, nodeType n
timeRangeHint ast.HintTimeRange
)
for _, hint := range hints {
// Set warning for the hint that requires the table name.
switch hint.HintName.L {
case TiDBMergeJoin, HintSMJ, TiDBIndexNestedLoopJoin, HintINLJ, HintINLHJ, HintINLMJ,
TiDBHashJoin, HintHJ, HintUseIndex, HintIgnoreIndex, HintIndexMerge:
if len(hint.Tables) == 0 {
b.pushHintWithoutTableWarning(hint)
continue
}
}

switch hint.HintName.L {
case TiDBMergeJoin, HintSMJ:
sortMergeTables = append(sortMergeTables, tableNames2HintTableInfo(b.ctx, hint.Tables, b.hintProcessor, nodeType, currentLevel)...)
Expand All @@ -2213,37 +2233,33 @@ func (b *PlanBuilder) pushTableHints(hints []*ast.TableOptimizerHint, nodeType n
case HintAggToCop:
aggHints.preferAggToCop = true
case HintUseIndex:
if len(hint.Tables) != 0 {
dbName := hint.Tables[0].DBName
if dbName.L == "" {
dbName = model.NewCIStr(b.ctx.GetSessionVars().CurrentDB)
}
indexHintList = append(indexHintList, indexHintInfo{
dbName: dbName,
tblName: hint.Tables[0].TableName,
indexHint: &ast.IndexHint{
IndexNames: hint.Indexes,
HintType: ast.HintUse,
HintScope: ast.HintForScan,
},
})
dbName := hint.Tables[0].DBName
if dbName.L == "" {
dbName = model.NewCIStr(b.ctx.GetSessionVars().CurrentDB)
}
indexHintList = append(indexHintList, indexHintInfo{
dbName: dbName,
tblName: hint.Tables[0].TableName,
indexHint: &ast.IndexHint{
IndexNames: hint.Indexes,
HintType: ast.HintUse,
HintScope: ast.HintForScan,
},
})
case HintIgnoreIndex:
if len(hint.Tables) != 0 {
dbName := hint.Tables[0].DBName
if dbName.L == "" {
dbName = model.NewCIStr(b.ctx.GetSessionVars().CurrentDB)
}
indexHintList = append(indexHintList, indexHintInfo{
dbName: dbName,
tblName: hint.Tables[0].TableName,
indexHint: &ast.IndexHint{
IndexNames: hint.Indexes,
HintType: ast.HintIgnore,
HintScope: ast.HintForScan,
},
})
dbName := hint.Tables[0].DBName
if dbName.L == "" {
dbName = model.NewCIStr(b.ctx.GetSessionVars().CurrentDB)
}
indexHintList = append(indexHintList, indexHintInfo{
dbName: dbName,
tblName: hint.Tables[0].TableName,
indexHint: &ast.IndexHint{
IndexNames: hint.Indexes,
HintType: ast.HintIgnore,
HintScope: ast.HintForScan,
},
})
case HintReadFromStorage:
switch hint.HintData.(model.CIStr).L {
case HintTiFlash:
Expand All @@ -2252,21 +2268,19 @@ func (b *PlanBuilder) pushTableHints(hints []*ast.TableOptimizerHint, nodeType n
tikvTables = append(tikvTables, tableNames2HintTableInfo(b.ctx, hint.Tables, b.hintProcessor, nodeType, currentLevel)...)
}
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,
HintType: ast.HintUse,
HintScope: ast.HintForScan,
},
})
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,
HintType: ast.HintUse,
HintScope: ast.HintForScan,
},
})
case HintTimeRange:
timeRangeHint = hint.HintData.(ast.HintTimeRange)
default:
Expand Down
15 changes: 15 additions & 0 deletions planner/core/testdata/integration_suite_in.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,5 +114,20 @@
"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"
]
},
{
"name": "TestHintWithoutTableWarning",
"cases": [
"select /*+ TIDB_SMJ() */ * from t1, t2 where t1.a=t2.a",
"select /*+ MERGE_JOIN() */ * from t1, t2 where t1.a=t2.a",
"select /*+ INL_JOIN() */ * from t1, t2 where t1.a=t2.a",
"select /*+ TIDB_INLJ() */ * from t1, t2 where t1.a=t2.a",
"select /*+ INL_HASH_JOIN() */ * from t1, t2 where t1.a=t2.a",
"select /*+ INL_MERGE_JOIN() */ * from t1, t2 where t1.a=t2.a",
"select /*+ HASH_JOIN() */ * from t1, t2 where t1.a=t2.a",
"select /*+ USE_INDEX() */ * from t1, t2 where t1.a=t2.a",
"select /*+ IGNORE_INDEX() */ * from t1, t2 where t1.a=t2.a",
"select /*+ USE_INDEX_MERGE() */ * from t1, t2 where t1.a=t2.a"
]
}
]
65 changes: 65 additions & 0 deletions planner/core/testdata/integration_suite_out.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -477,5 +477,70 @@
]
}
]
},
{
"Name": "TestHintWithoutTableWarning",
"Cases": [
{
"SQL": "select /*+ TIDB_SMJ() */ * from t1, t2 where t1.a=t2.a",
"Warnings": [
"[planner:1815]Hint TIDB_SMJ() is inapplicable. Please specify the table names in the arguments."
]
},
{
"SQL": "select /*+ MERGE_JOIN() */ * from t1, t2 where t1.a=t2.a",
"Warnings": [
"[planner:1815]Hint MERGE_JOIN() is inapplicable. Please specify the table names in the arguments."
]
},
{
"SQL": "select /*+ INL_JOIN() */ * from t1, t2 where t1.a=t2.a",
"Warnings": [
"[planner:1815]Hint INL_JOIN() is inapplicable. Please specify the table names in the arguments."
]
},
{
"SQL": "select /*+ TIDB_INLJ() */ * from t1, t2 where t1.a=t2.a",
"Warnings": [
"[planner:1815]Hint TIDB_INLJ() is inapplicable. Please specify the table names in the arguments."
]
},
{
"SQL": "select /*+ INL_HASH_JOIN() */ * from t1, t2 where t1.a=t2.a",
"Warnings": [
"[planner:1815]Hint INL_HASH_JOIN() is inapplicable. Please specify the table names in the arguments."
]
},
{
"SQL": "select /*+ INL_MERGE_JOIN() */ * from t1, t2 where t1.a=t2.a",
"Warnings": [
"[planner:1815]Hint INL_MERGE_JOIN() is inapplicable. Please specify the table names in the arguments."
]
},
{
"SQL": "select /*+ HASH_JOIN() */ * from t1, t2 where t1.a=t2.a",
"Warnings": [
"[planner:1815]Hint HASH_JOIN() is inapplicable. Please specify the table names in the arguments."
]
},
{
"SQL": "select /*+ USE_INDEX() */ * from t1, t2 where t1.a=t2.a",
"Warnings": [
"[parser:1064]You have an error in your SQL syntax; check the manual that corresponds to your TiDB version for the right syntax to use [parser:8064]Optimizer hint syntax error at line 1 column 22 near \") */\" "
]
},
{
"SQL": "select /*+ IGNORE_INDEX() */ * from t1, t2 where t1.a=t2.a",
"Warnings": [
"[parser:1064]You have an error in your SQL syntax; check the manual that corresponds to your TiDB version for the right syntax to use [parser:8064]Optimizer hint syntax error at line 1 column 25 near \") */\" "
]
},
{
"SQL": "select /*+ USE_INDEX_MERGE() */ * from t1, t2 where t1.a=t2.a",
"Warnings": [
"[parser:1064]You have an error in your SQL syntax; check the manual that corresponds to your TiDB version for the right syntax to use [parser:8064]Optimizer hint syntax error at line 1 column 28 near \") */\" "
]
}
]
}
]

0 comments on commit 3f4a3d5

Please sign in to comment.