Skip to content

Commit

Permalink
planner/core: raise warning for unmatched join hint (pingcap#9914) (p…
Browse files Browse the repository at this point in the history
  • Loading branch information
alivxxx authored and zz-jason committed Apr 1, 2019
1 parent 8e04634 commit fe7aefa
Show file tree
Hide file tree
Showing 4 changed files with 118 additions and 18 deletions.
2 changes: 1 addition & 1 deletion planner/core/exhaust_physical_plans.go
Original file line number Diff line number Diff line change
Expand Up @@ -639,7 +639,7 @@ func (p *LogicalJoin) tryToGetIndexJoin(prop *property.PhysicalProperty) (indexJ
// Construct warning message prefix.
errMsg := "Optimizer Hint TIDB_INLJ is inapplicable"
if p.hintInfo != nil {
errMsg = fmt.Sprintf("Optimizer Hint %s is inapplicable", p.hintInfo.restore2IndexJoinHint())
errMsg = fmt.Sprintf("Optimizer Hint %s is inapplicable", restore2JoinHint(TiDBIndexNestedLoopJoin, p.hintInfo.indexNestedLoopJoinTables))
}

// Append inapplicable reason.
Expand Down
22 changes: 18 additions & 4 deletions planner/core/logical_plan_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -1613,15 +1613,15 @@ func (b *planBuilder) unfoldWildStar(p LogicalPlan, selectFields []*ast.SelectFi
}

func (b *planBuilder) pushTableHints(hints []*ast.TableOptimizerHint) bool {
var sortMergeTables, INLJTables, hashJoinTables []model.CIStr
var sortMergeTables, INLJTables, hashJoinTables []hintTableInfo
for _, hint := range hints {
switch hint.HintName.L {
case TiDBMergeJoin:
sortMergeTables = append(sortMergeTables, hint.Tables...)
sortMergeTables = tableNames2HintTableInfo(hint.Tables)
case TiDBIndexNestedLoopJoin:
INLJTables = append(INLJTables, hint.Tables...)
INLJTables = tableNames2HintTableInfo(hint.Tables)
case TiDBHashJoin:
hashJoinTables = append(hashJoinTables, hint.Tables...)
hashJoinTables = tableNames2HintTableInfo(hint.Tables)
default:
// ignore hints that not implemented
}
Expand All @@ -1638,9 +1638,23 @@ func (b *planBuilder) pushTableHints(hints []*ast.TableOptimizerHint) bool {
}

func (b *planBuilder) popTableHints() {
hintInfo := b.tableHintInfo[len(b.tableHintInfo)-1]
b.appendUnmatchedJoinHintWarning(TiDBIndexNestedLoopJoin, hintInfo.indexNestedLoopJoinTables)
b.appendUnmatchedJoinHintWarning(TiDBMergeJoin, hintInfo.sortMergeJoinTables)
b.appendUnmatchedJoinHintWarning(TiDBHashJoin, hintInfo.hashJoinTables)
b.tableHintInfo = b.tableHintInfo[:len(b.tableHintInfo)-1]
}

func (b *planBuilder) appendUnmatchedJoinHintWarning(joinType string, hintTables []hintTableInfo) {
unMatchedTables := extractUnmatchedTables(hintTables)
if len(unMatchedTables) == 0 {
return
}
errMsg := fmt.Sprintf("There are no matching table names for (%s) in optimizer hint %s. Maybe you can use the table alias name",
strings.Join(unMatchedTables, ", "), restore2JoinHint(joinType, hintTables))
b.ctx.GetSessionVars().StmtCtx.AppendWarning(ErrInternal.GenWithStack(errMsg))
}

// TableHints returns the *tableHintInfo of PlanBuilder.
func (b *planBuilder) TableHints() *tableHintInfo {
if len(b.tableHintInfo) == 0 {
Expand Down
55 changes: 55 additions & 0 deletions planner/core/physical_plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/pingcap/tidb/planner/core"
"github.com/pingcap/tidb/session"
"github.com/pingcap/tidb/sessionctx"
"github.com/pingcap/tidb/sessionctx/stmtctx"
"github.com/pingcap/tidb/util/testleak"
"golang.org/x/net/context"
)
Expand Down Expand Up @@ -1334,3 +1335,57 @@ func (s *testPlanSuite) TestDoSubquery(c *C) {
c.Assert(core.ToString(p), Equals, tt.best, comment)
}
}

func (s *testPlanSuite) TestUnmatchedTableInHint(c *C) {
defer testleak.AfterTest(c)()
store, dom, err := newStoreWithBootstrap()
c.Assert(err, IsNil)
defer func() {
dom.Close()
store.Close()
}()
se, err := session.CreateSession4Test(store)
c.Assert(err, IsNil)
_, err = se.Execute(context.Background(), "use test")
c.Assert(err, IsNil)
tests := []struct {
sql string
warning string
}{
{
sql: "SELECT /*+ TIDB_SMJ(t3, t4) */ * from t t1, t t2 where t1.a = t2.a",
warning: "[planner:1815]There are no matching table names for (t3, t4) in optimizer hint /*+ TIDB_SMJ(t3, t4) */. Maybe you can use the table alias name",
},
{
sql: "SELECT /*+ TIDB_HJ(t3, t4) */ * from t t1, t t2 where t1.a = t2.a",
warning: "[planner:1815]There are no matching table names for (t3, t4) in optimizer hint /*+ TIDB_HJ(t3, t4) */. Maybe you can use the table alias name",
},
{
sql: "SELECT /*+ TIDB_INLJ(t3, t4) */ * from t t1, t t2 where t1.a = t2.a",
warning: "[planner:1815]There are no matching table names for (t3, t4) in optimizer hint /*+ TIDB_INLJ(t3, t4) */. Maybe you can use the table alias name",
},
{
sql: "SELECT /*+ TIDB_SMJ(t1, t2) */ * from t t1, t t2 where t1.a = t2.a",
warning: "",
},
{
sql: "SELECT /*+ TIDB_SMJ(t3, t4) */ * from t t1, t t2, t t3 where t1.a = t2.a and t2.a = t3.a",
warning: "[planner:1815]There are no matching table names for (t4) in optimizer hint /*+ TIDB_SMJ(t3, t4) */. Maybe you can use the table alias name",
},
}
for _, test := range tests {
se.GetSessionVars().StmtCtx.SetWarnings(nil)
stmt, err := s.ParseOneStmt(test.sql, "", "")
c.Assert(err, IsNil)
_, err = core.Optimize(se, stmt, s.is)
c.Assert(err, IsNil)
warnings := se.GetSessionVars().StmtCtx.GetWarnings()
if test.warning == "" {
c.Assert(len(warnings), Equals, 0)
} else {
c.Assert(len(warnings), Equals, 1)
c.Assert(warnings[0].Level, Equals, stmtctx.WarnLevelWarning)
c.Assert(warnings[0].Err.Error(), Equals, test.warning)
}
}
}
57 changes: 44 additions & 13 deletions planner/core/planbuilder.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,25 @@ type visitInfo struct {
}

type tableHintInfo struct {
indexNestedLoopJoinTables []model.CIStr
sortMergeJoinTables []model.CIStr
hashJoinTables []model.CIStr
indexNestedLoopJoinTables []hintTableInfo
sortMergeJoinTables []hintTableInfo
hashJoinTables []hintTableInfo
}

type hintTableInfo struct {
name model.CIStr
matched bool
}

func tableNames2HintTableInfo(tableNames []model.CIStr) []hintTableInfo {
if len(tableNames) == 0 {
return nil
}
hintTables := make([]hintTableInfo, 0, len(tableNames))
for _, tableName := range tableNames {
hintTables = append(hintTables, hintTableInfo{name: tableName})
}
return hintTables
}

func (info *tableHintInfo) ifPreferMergeJoin(tableNames ...*model.CIStr) bool {
Expand All @@ -70,32 +86,47 @@ func (info *tableHintInfo) ifPreferINLJ(tableNames ...*model.CIStr) bool {
// Which it joins on with depend on sequence of traverse
// and without reorder, user might adjust themselves.
// This is similar to MySQL hints.
func (info *tableHintInfo) matchTableName(tables []*model.CIStr, tablesInHints []model.CIStr) bool {
func (info *tableHintInfo) matchTableName(tables []*model.CIStr, hintTables []hintTableInfo) bool {
hintMatched := false
for _, tableName := range tables {
if tableName == nil {
continue
}
for _, curEntry := range tablesInHints {
if curEntry.L == tableName.L {
return true
for i, curEntry := range hintTables {
if curEntry.name.L == tableName.L {
hintTables[i].matched = true
hintMatched = true
break
}
}
}
return false
return hintMatched
}

func (info *tableHintInfo) restore2IndexJoinHint() string {
buffer := bytes.NewBufferString("/*+ TIDB_INLJ(")
for i, tableName := range info.indexNestedLoopJoinTables {
buffer.WriteString(tableName.O)
if i < len(info.indexNestedLoopJoinTables)-1 {
func restore2JoinHint(hintType string, hintTables []hintTableInfo) string {
buffer := bytes.NewBufferString("/*+ ")
buffer.WriteString(strings.ToUpper(hintType))
buffer.WriteString("(")
for i, table := range hintTables {
buffer.WriteString(table.name.L)
if i < len(hintTables)-1 {
buffer.WriteString(", ")
}
}
buffer.WriteString(") */")
return buffer.String()
}

func extractUnmatchedTables(hintTables []hintTableInfo) []string {
var tableNames []string
for _, table := range hintTables {
if !table.matched {
tableNames = append(tableNames, table.name.O)
}
}
return tableNames
}

// clauseCode indicates in which clause the column is currently.
type clauseCode int

Expand Down

0 comments on commit fe7aefa

Please sign in to comment.