From 5dc5145cf8c7e04c79aba033bf6c8821b2fa4448 Mon Sep 17 00:00:00 2001 From: Yuanjia Zhang Date: Fri, 5 Jan 2024 10:47:03 +0800 Subject: [PATCH] planner: remove useless binding code and add more test cases (#50098) ref pingcap/tidb#48875 --- pkg/bindinfo/bind_record.go | 11 +------ pkg/bindinfo/fuzzy_binding_test.go | 32 ++++++-------------- pkg/bindinfo/global_handle.go | 5 --- pkg/executor/bind.go | 7 ----- pkg/executor/builder.go | 1 - pkg/planner/core/common_plans.go | 1 - pkg/planner/core/planbuilder.go | 2 -- pkg/server/testdata/optimizer_suite_out.json | 1 - 8 files changed, 10 insertions(+), 50 deletions(-) diff --git a/pkg/bindinfo/bind_record.go b/pkg/bindinfo/bind_record.go index 4bbeea99d38fe..96a3fa736c592 100644 --- a/pkg/bindinfo/bind_record.go +++ b/pkg/bindinfo/bind_record.go @@ -59,13 +59,6 @@ const ( History = "history" ) -const ( - // TypeNormal indicates the binding is a normal binding. - TypeNormal string = "" - // TypeUniversal indicates the binding is a universal binding. - TypeUniversal string = "u" -) - // Binding stores the basic bind hint info. type Binding struct { BindSQL string @@ -84,8 +77,6 @@ type Binding struct { ID string `json:"-"` SQLDigest string PlanDigest string - // Type indicates the type of this binding, currently only 2 types: "" for normal and "u" for universal bindings. - Type string // TableNames records all schema and table names in this binding statement, which are used for fuzzy matching. TableNames []*ast.TableName `json:"-"` @@ -205,7 +196,7 @@ func (br *BindRecord) prepareHints(sctx sessionctx.Context) error { if err != nil { return err } - if sctx != nil && bind.Type == TypeNormal && !isFuzzy { + if sctx != nil && !isFuzzy { paramChecker := ¶mMarkerChecker{} stmt.Accept(paramChecker) if !paramChecker.hasParamMarker { diff --git a/pkg/bindinfo/fuzzy_binding_test.go b/pkg/bindinfo/fuzzy_binding_test.go index beefde59aae04..bf14d78497ef0 100644 --- a/pkg/bindinfo/fuzzy_binding_test.go +++ b/pkg/bindinfo/fuzzy_binding_test.go @@ -87,38 +87,25 @@ func TestFuzzyBindingBasic(t *testing.T) { } } -func TestUniversalDuplicatedBinding(t *testing.T) { - t.Skip("skip it temporarily") +func TestFuzzyDuplicatedBinding(t *testing.T) { store := testkit.CreateMockStore(t) tk := testkit.NewTestKit(t, store) tk.MustExec(`use test`) tk.MustExec(`create table t (a int, b int, c int, d int, e int, key(a), key(b), key(c), key(d), key(e))`) - tk.MustExec(`create global universal binding using select * from t`) + tk.MustExec(`create global binding using select * from *.t`) require.Equal(t, showBinding(tk, "show global bindings"), - [][]interface{}{{"select * from `t`", "SELECT * FROM `t`", "", "enabled", "manual", "e5796985ccafe2f71126ed6c0ac939ffa015a8c0744a24b7aee6d587103fd2f7"}}) + [][]interface{}{{"select * from `*` . `t`", "SELECT * FROM `*`.`t`", "", "enabled", "manual", "a17da0a38af0f1d75229c5cd064d5222a610c5e5ef59436be5da1564c16f1013"}}) // if duplicated, the old one will be replaced - tk.MustExec(`create global universal binding using select /*+ use_index(t, a) */ * from t`) + tk.MustExec(`create global binding using select /*+ use_index(t, a) */ * from *.t`) require.Equal(t, showBinding(tk, "show global bindings"), - [][]interface{}{{"select * from `t`", "SELECT /*+ use_index(`t` `a`)*/ * FROM `t`", "", "enabled", "manual", "e5796985ccafe2f71126ed6c0ac939ffa015a8c0744a24b7aee6d587103fd2f7"}}) + [][]interface{}{{"select * from `*` . `t`", "SELECT /*+ use_index(`t` `a`)*/ * FROM `*`.`t`", "", "enabled", "manual", "a17da0a38af0f1d75229c5cd064d5222a610c5e5ef59436be5da1564c16f1013"}}) // if duplicated, the old one will be replaced - tk.MustExec(`create global universal binding using select /*+ use_index(t, b) */ * from t`) - require.Equal(t, showBinding(tk, "show global bindings"), - [][]interface{}{{"select * from `t`", "SELECT /*+ use_index(`t` `b`)*/ * FROM `t`", "", "enabled", "manual", "e5796985ccafe2f71126ed6c0ac939ffa015a8c0744a24b7aee6d587103fd2f7"}}) - - // normal bindings don't conflict with universal bindings - tk.MustExec(`create global binding using select /*+ use_index(t, b) */ * from t`) - require.Equal(t, showBinding(tk, "show global bindings"), - [][]interface{}{{"select * from `t`", "SELECT /*+ use_index(`t` `b`)*/ * FROM `t`", "", "enabled", "manual", "e5796985ccafe2f71126ed6c0ac939ffa015a8c0744a24b7aee6d587103fd2f7"}, - {"select * from `test` . `t`", "SELECT /*+ use_index(`t` `b`)*/ * FROM `test`.`t`", "test", "enabled", "manual", "8b193b00413fdb910d39073e0d494c96ebf24d1e30b131ecdd553883d0e29b42"}}) - - // session bindings don't conflict with global bindings - tk.MustExec(`create session universal binding using select /*+ use_index(t, c) */ * from t`) + tk.MustExec(`create global binding using select /*+ use_index(t, b) */ * from *.t`) require.Equal(t, showBinding(tk, "show global bindings"), - [][]interface{}{{"select * from `t`", "SELECT /*+ use_index(`t` `b`)*/ * FROM `t`", "", "enabled", "manual", "e5796985ccafe2f71126ed6c0ac939ffa015a8c0744a24b7aee6d587103fd2f7"}, - {"select * from `test` . `t`", "SELECT /*+ use_index(`t` `b`)*/ * FROM `test`.`t`", "test", "enabled", "manual", "8b193b00413fdb910d39073e0d494c96ebf24d1e30b131ecdd553883d0e29b42"}}) + [][]interface{}{{"select * from `*` . `t`", "SELECT /*+ use_index(`t` `b`)*/ * FROM `*`.`t`", "", "enabled", "manual", "a17da0a38af0f1d75229c5cd064d5222a610c5e5ef59436be5da1564c16f1013"}}) } func TestUniversalBindingPriority(t *testing.T) { @@ -230,13 +217,12 @@ func TestFuzzyBindingSwitch(t *testing.T) { tk3.MustQuery(`show global variables like 'tidb_opt_enable_fuzzy_binding'`).Check(testkit.Rows("tidb_opt_enable_fuzzy_binding OFF")) } -func TestUniversalBindingSetVar(t *testing.T) { - t.Skip("skip it temporarily") +func TestFuzzyBindingSetVar(t *testing.T) { store := testkit.CreateMockStore(t) tk := testkit.NewTestKit(t, store) tk.MustExec(`use test`) tk.MustExec(`create table t (a int, b int, key(a), key(b))`) - tk.MustExec(`create universal binding using select /*+ use_index(t, a) */ * from t`) + tk.MustExec(`create global binding using select /*+ use_index(t, a) */ * from *.t`) tk.MustExec(`set @@tidb_opt_enable_fuzzy_binding=0`) tk.MustExec(`select * from t`) diff --git a/pkg/bindinfo/global_handle.go b/pkg/bindinfo/global_handle.go index 69b7b6e784aa0..b44231c21c2fd 100644 --- a/pkg/bindinfo/global_handle.go +++ b/pkg/bindinfo/global_handle.go @@ -588,10 +588,6 @@ func newBindRecord(sctx sessionctx.Context, row chunk.Row) (string, *BindRecord, status = Enabled } defaultDB := row.GetString(2) - bindingType := TypeNormal - if defaultDB == "" { - bindingType = TypeUniversal - } bindSQL := row.GetString(1) charset, collation := row.GetString(6), row.GetString(7) @@ -611,7 +607,6 @@ func newBindRecord(sctx sessionctx.Context, row chunk.Row) (string, *BindRecord, Source: row.GetString(8), SQLDigest: row.GetString(9), PlanDigest: row.GetString(10), - Type: bindingType, TableNames: tableNames, } bindRecord := &BindRecord{ diff --git a/pkg/executor/bind.go b/pkg/executor/bind.go index ecaadab5d22f3..6d43d303a9fa8 100644 --- a/pkg/executor/bind.go +++ b/pkg/executor/bind.go @@ -38,7 +38,6 @@ type SQLBindExec struct { collation string db string isGlobal bool - isUniversal bool // for universal binding bindAst ast.StmtNode newStatus string source string // by manual or from history, only in create stmt @@ -130,11 +129,6 @@ func (e *SQLBindExec) createSQLBind() error { e.Ctx().GetSessionVars().StmtCtx = saveStmtCtx }() - bindingType := bindinfo.TypeNormal - if e.isUniversal { - bindingType = bindinfo.TypeUniversal - } - bindInfo := bindinfo.Binding{ BindSQL: e.bindSQL, Charset: e.charset, @@ -143,7 +137,6 @@ func (e *SQLBindExec) createSQLBind() error { Source: e.source, SQLDigest: e.sqlDigest, PlanDigest: e.planDigest, - Type: bindingType, } record := &bindinfo.BindRecord{ OriginalSQL: e.normdOrigSQL, diff --git a/pkg/executor/builder.go b/pkg/executor/builder.go index 9ff3568fcd75a..2f31c44f17b64 100644 --- a/pkg/executor/builder.go +++ b/pkg/executor/builder.go @@ -4913,7 +4913,6 @@ func (b *executorBuilder) buildSQLBindExec(v *plannercore.SQLBindPlan) exec.Exec collation: v.Collation, db: v.Db, isGlobal: v.IsGlobal, - isUniversal: v.IsUniversal, bindAst: v.BindStmt, newStatus: v.NewStatus, source: v.Source, diff --git a/pkg/planner/core/common_plans.go b/pkg/planner/core/common_plans.go index 2a1d387ba2839..a085ea2a685fc 100644 --- a/pkg/planner/core/common_plans.go +++ b/pkg/planner/core/common_plans.go @@ -274,7 +274,6 @@ type SQLBindPlan struct { NormdOrigSQL string BindSQL string IsGlobal bool - IsUniversal bool // for universal binding BindStmt ast.StmtNode Db string Charset string diff --git a/pkg/planner/core/planbuilder.go b/pkg/planner/core/planbuilder.go index d9904a19f8fb1..b8b6632a23ba9 100644 --- a/pkg/planner/core/planbuilder.go +++ b/pkg/planner/core/planbuilder.go @@ -871,7 +871,6 @@ func (b *PlanBuilder) buildCreateBindPlanFromPlanDigest(v *ast.CreateBindingStmt NormdOrigSQL: normdOrigSQL, BindSQL: bindSQL, IsGlobal: v.GlobalScope, - IsUniversal: v.IsUniversal, BindStmt: hintNode, Db: db, Charset: bindableStmt.Charset, @@ -913,7 +912,6 @@ func (b *PlanBuilder) buildCreateBindPlan(v *ast.CreateBindingStmt) (Plan, error NormdOrigSQL: normdOrigSQL, BindSQL: bindSQL, IsGlobal: v.GlobalScope, - IsUniversal: v.IsUniversal, BindStmt: v.HintedNode, Db: db, Charset: charSet, diff --git a/pkg/server/testdata/optimizer_suite_out.json b/pkg/server/testdata/optimizer_suite_out.json index 68aa360ffe2d7..db4e392d330ee 100644 --- a/pkg/server/testdata/optimizer_suite_out.json +++ b/pkg/server/testdata/optimizer_suite_out.json @@ -262,7 +262,6 @@ "SQLDigest": "36ceb6159adb3ac83539ec90c861ac4be4bc5cdb5fa02f70542744a4af640eac", "Source": "manual", "Status": "enabled", - "Type": "", "UpdateTime": 0 } ],