Skip to content

Commit

Permalink
planner: output a warning instead of returning an error when creating…
Browse files Browse the repository at this point in the history
… fast binding on a incomplete hint (#51781)

ref #51347
  • Loading branch information
qw4990 authored Mar 14, 2024
1 parent f46ccb3 commit 6306512
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 19 deletions.
9 changes: 6 additions & 3 deletions pkg/infoschema/test/clustertablestest/cluster_tables_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1264,12 +1264,14 @@ func TestErrorCasesCreateBindingFromHistory(t *testing.T) {
sql := "select * from t1 where t1.id in (select id from t2)"
tk.MustExec(sql)
planDigest := tk.MustQuery(fmt.Sprintf("select plan_digest from information_schema.statements_summary where query_sample_text = '%s'", sql)).Rows()
tk.MustGetErrMsg(fmt.Sprintf("create binding from history using plan digest '%s'", planDigest[0][0]), "can't create binding for query with sub query")
tk.MustExec(fmt.Sprintf("create binding from history using plan digest '%s'", planDigest[0][0]))
tk.MustQuery(`show warnings`).Check(testkit.Rows("Warning 1105 auto-generated hint for queries with sub queries might not be complete, the plan might change even after creating this binding"))

sql = "select * from t1, t2, t3 where t1.id = t2.id and t2.id = t3.id"
tk.MustExec(sql)
planDigest = tk.MustQuery(fmt.Sprintf("select plan_digest from information_schema.statements_summary where query_sample_text = '%s'", sql)).Rows()
tk.MustGetErrMsg(fmt.Sprintf("create binding from history using plan digest '%s'", planDigest[0][0]), "can't create binding for query with more than two table join")
tk.MustExec(fmt.Sprintf("create binding from history using plan digest '%s'", planDigest[0][0]))
tk.MustQuery(`show warnings`).Check(testkit.Rows("Warning 1105 auto-generated hint for queries with more than 3 table join might not be complete, the plan might change even after creating this binding"))
}

// withMockTiFlash sets the mockStore to have N TiFlash stores (naming as tiflash0, tiflash1, ...).
Expand Down Expand Up @@ -1315,7 +1317,8 @@ func TestBindingFromHistoryWithTiFlashBindable(t *testing.T) {
sql := "select * from t"
tk.MustExec(sql)
planDigest := tk.MustQuery(fmt.Sprintf("select plan_digest from information_schema.cluster_statements_summary where query_sample_text = '%s'", sql)).Rows()
tk.MustGetErrMsg(fmt.Sprintf("create binding from history using plan digest '%s'", planDigest[0][0]), "can't create binding for query with tiflash engine")
tk.MustExec(fmt.Sprintf("create binding from history using plan digest '%s'", planDigest[0][0]))
tk.MustQuery(`show warnings`).Check(testkit.Rows("Warning 1105 auto-generated hint for queries accessing TiFlash might not be complete, the plan might change even after creating this binding"))
}

func TestSetBindingStatusBySQLDigest(t *testing.T) {
Expand Down
4 changes: 2 additions & 2 deletions pkg/planner/core/planbuilder.go
Original file line number Diff line number Diff line change
Expand Up @@ -847,8 +847,8 @@ func (b *PlanBuilder) buildCreateBindPlanFromPlanDigest(v *ast.CreateBindingStmt
if err != nil {
return nil, errors.Errorf("binding failed: %v", err)
}
if err = hint.CheckBindingFromHistoryBindable(originNode, bindableStmt.PlanHint); err != nil {
return nil, err
if complete, reason := hint.CheckBindingFromHistoryComplete(originNode, bindableStmt.PlanHint); !complete {
b.ctx.GetSessionVars().StmtCtx.AppendWarning(errors.NewNoStackError(reason))
}
bindSQL := bindinfo.GenerateBindingSQL(originNode, bindableStmt.PlanHint, true, bindableStmt.Schema)
var hintNode ast.StmtNode
Expand Down
27 changes: 13 additions & 14 deletions pkg/util/hint/hint_processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
"fmt"
"strings"

"github.com/pingcap/errors"
"github.com/pingcap/tidb/pkg/parser"
"github.com/pingcap/tidb/pkg/parser/ast"
"github.com/pingcap/tidb/pkg/parser/format"
Expand Down Expand Up @@ -355,47 +354,47 @@ func nodeType4Stmt(node ast.StmtNode) NodeType {
return TypeInvalid
}

// CheckBindingFromHistoryBindable checks whether the ast and hint string from history is bindable.
// Not support:
// CheckBindingFromHistoryComplete checks whether the ast and hint string from history is complete.
// For these complex queries, the auto-generated binding might be not complete:
// 1. query use tiFlash engine
// 2. query with sub query
// 3. query with more than 2 table join
func CheckBindingFromHistoryBindable(node ast.Node, hintStr string) error {
func CheckBindingFromHistoryComplete(node ast.Node, hintStr string) (complete bool, reason string) {
// check tiflash
contain := strings.Contains(hintStr, "tiflash")
if contain {
return errors.New("can't create binding for query with tiflash engine")
return false, "auto-generated hint for queries accessing TiFlash might not be complete, the plan might change even after creating this binding"
}

checker := bindableChecker{
bindable: true,
complete: true,
tables: make(map[model.CIStr]struct{}, 2),
}
node.Accept(&checker)
return checker.reason
return checker.complete, checker.reason
}

// bindableChecker checks whether a binding from history can be created.
type bindableChecker struct {
bindable bool
reason error
complete bool
reason string
tables map[model.CIStr]struct{}
}

// Enter implements Visitor interface.
func (checker *bindableChecker) Enter(in ast.Node) (out ast.Node, skipChildren bool) {
switch node := in.(type) {
case *ast.ExistsSubqueryExpr, *ast.SubqueryExpr:
checker.bindable = false
checker.reason = errors.New("can't create binding for query with sub query")
checker.complete = false
checker.reason = "auto-generated hint for queries with sub queries might not be complete, the plan might change even after creating this binding"
return in, true
case *ast.TableName:
if _, ok := checker.tables[node.Schema]; !ok {
checker.tables[node.Name] = struct{}{}
}
if len(checker.tables) >= 3 {
checker.bindable = false
checker.reason = errors.New("can't create binding for query with more than two table join")
checker.complete = false
checker.reason = "auto-generated hint for queries with more than 3 table join might not be complete, the plan might change even after creating this binding"
return in, true
}
}
Expand All @@ -404,5 +403,5 @@ func (checker *bindableChecker) Enter(in ast.Node) (out ast.Node, skipChildren b

// Leave implements Visitor interface.
func (checker *bindableChecker) Leave(in ast.Node) (out ast.Node, ok bool) {
return in, checker.bindable
return in, checker.complete
}

0 comments on commit 6306512

Please sign in to comment.