From f2aa851ba721e9ddd78c90ecd917627c4dd58801 Mon Sep 17 00:00:00 2001 From: qw4990 Date: Thu, 14 Mar 2024 16:01:03 +0800 Subject: [PATCH 1/5] fixup --- pkg/planner/core/planbuilder.go | 4 ++-- pkg/util/hint/hint_processor.go | 27 +++++++++++++-------------- 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/pkg/planner/core/planbuilder.go b/pkg/planner/core/planbuilder.go index 1b5b2b245373a..56914695c9d70 100644 --- a/pkg/planner/core/planbuilder.go +++ b/pkg/planner/core/planbuilder.go @@ -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.New(reason)) } bindSQL := bindinfo.GenerateBindingSQL(originNode, bindableStmt.PlanHint, true, bindableStmt.Schema) var hintNode ast.StmtNode diff --git a/pkg/util/hint/hint_processor.go b/pkg/util/hint/hint_processor.go index 67f143aea18e6..728d0b06f0f3a 100644 --- a/pkg/util/hint/hint_processor.go +++ b/pkg/util/hint/hint_processor.go @@ -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" @@ -355,30 +354,30 @@ 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{} } @@ -386,16 +385,16 @@ type bindableChecker struct { 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 } } @@ -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 } From 295dfbd5f6d10a8dfb80a13b6a5fe0a2bed7b7b9 Mon Sep 17 00:00:00 2001 From: qw4990 Date: Thu, 14 Mar 2024 16:07:35 +0800 Subject: [PATCH 2/5] fixup --- pkg/infoschema/test/clustertablestest/cluster_tables_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/infoschema/test/clustertablestest/cluster_tables_test.go b/pkg/infoschema/test/clustertablestest/cluster_tables_test.go index 5f8b2e85466df..5ea069465d6bd 100644 --- a/pkg/infoschema/test/clustertablestest/cluster_tables_test.go +++ b/pkg/infoschema/test/clustertablestest/cluster_tables_test.go @@ -1315,7 +1315,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) { From b1b205570cd79337b13fee8dfaca8cf074880d4c Mon Sep 17 00:00:00 2001 From: qw4990 Date: Thu, 14 Mar 2024 16:27:46 +0800 Subject: [PATCH 3/5] fixup --- .../test/clustertablestest/cluster_tables_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pkg/infoschema/test/clustertablestest/cluster_tables_test.go b/pkg/infoschema/test/clustertablestest/cluster_tables_test.go index 5ea069465d6bd..8315c968fa2ae 100644 --- a/pkg/infoschema/test/clustertablestest/cluster_tables_test.go +++ b/pkg/infoschema/test/clustertablestest/cluster_tables_test.go @@ -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, ...). From 80fba9420ace57f08e8428811f81ab7b1bb0d180 Mon Sep 17 00:00:00 2001 From: Yuanjia Zhang Date: Thu, 14 Mar 2024 16:47:06 +0800 Subject: [PATCH 4/5] Update pkg/planner/core/planbuilder.go Co-authored-by: Arenatlx --- pkg/planner/core/planbuilder.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/planner/core/planbuilder.go b/pkg/planner/core/planbuilder.go index 56914695c9d70..12e0a57f2b0d8 100644 --- a/pkg/planner/core/planbuilder.go +++ b/pkg/planner/core/planbuilder.go @@ -848,7 +848,7 @@ func (b *PlanBuilder) buildCreateBindPlanFromPlanDigest(v *ast.CreateBindingStmt return nil, errors.Errorf("binding failed: %v", err) } if complete, reason := hint.CheckBindingFromHistoryComplete(originNode, bindableStmt.PlanHint); !complete { - b.ctx.GetSessionVars().StmtCtx.AppendWarning(errors.New(reason)) + b.ctx.GetSessionVars().StmtCtx.AppendWarning(errors.NewNoStackError(reason))... } bindSQL := bindinfo.GenerateBindingSQL(originNode, bindableStmt.PlanHint, true, bindableStmt.Schema) var hintNode ast.StmtNode From 6add6da4c4775316d24ff776f072be7ee8e0e376 Mon Sep 17 00:00:00 2001 From: qw4990 Date: Thu, 14 Mar 2024 16:53:06 +0800 Subject: [PATCH 5/5] fixup --- pkg/planner/core/planbuilder.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/planner/core/planbuilder.go b/pkg/planner/core/planbuilder.go index 12e0a57f2b0d8..d66770a6fe74d 100644 --- a/pkg/planner/core/planbuilder.go +++ b/pkg/planner/core/planbuilder.go @@ -848,7 +848,7 @@ func (b *PlanBuilder) buildCreateBindPlanFromPlanDigest(v *ast.CreateBindingStmt return nil, errors.Errorf("binding failed: %v", err) } if complete, reason := hint.CheckBindingFromHistoryComplete(originNode, bindableStmt.PlanHint); !complete { - b.ctx.GetSessionVars().StmtCtx.AppendWarning(errors.NewNoStackError(reason))... + b.ctx.GetSessionVars().StmtCtx.AppendWarning(errors.NewNoStackError(reason)) } bindSQL := bindinfo.GenerateBindingSQL(originNode, bindableStmt.PlanHint, true, bindableStmt.Schema) var hintNode ast.StmtNode