From 2db6c0a2695deb03d420ef8097939e9c1fbbefef Mon Sep 17 00:00:00 2001 From: Yuanjia Zhang Date: Tue, 12 Sep 2023 17:59:03 +0800 Subject: [PATCH] planner: fix the issue that binding for `in (?)` cannot work for `in (?,?,?)` (#46896) ref pingcap/tidb#44298 --- bindinfo/tests/BUILD.bazel | 2 +- bindinfo/tests/bind_test.go | 76 +++++++++++++++++++++++++++++++++++++ planner/core/planbuilder.go | 8 ++-- planner/core/preprocess.go | 4 +- 4 files changed, 83 insertions(+), 7 deletions(-) diff --git a/bindinfo/tests/BUILD.bazel b/bindinfo/tests/BUILD.bazel index ebcd7c0f66222..547923e0c7317 100644 --- a/bindinfo/tests/BUILD.bazel +++ b/bindinfo/tests/BUILD.bazel @@ -9,7 +9,7 @@ go_test( ], flaky = True, race = "on", - shard_count = 35, + shard_count = 37, deps = [ "//bindinfo", "//bindinfo/internal", diff --git a/bindinfo/tests/bind_test.go b/bindinfo/tests/bind_test.go index a494320d1973d..399051dd10a2c 100644 --- a/bindinfo/tests/bind_test.go +++ b/bindinfo/tests/bind_test.go @@ -35,6 +35,82 @@ import ( "github.com/stretchr/testify/require" ) +func TestBindingInListEffect(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)`) + + // binding created with `in (?)` can work for `in (?,?,?)` + tk.MustQuery(`select a from t where a in (1, 2, 3)`) + tk.MustQuery(`select @@last_plan_from_binding`).Check(testkit.Rows("0")) + tk.MustExec(`create binding for select a from t where a in (1) using select a from t where a in (1)`) + tk.MustQuery(`select a from t where a in (1, 2, 3)`) + tk.MustQuery(`select @@last_plan_from_binding`).Check(testkit.Rows("1")) + tk.MustQuery(`select a from t where a in (1, 2)`) + tk.MustQuery(`select @@last_plan_from_binding`).Check(testkit.Rows("1")) + tk.MustQuery(`select a from t where a in (1)`) + tk.MustQuery(`select @@last_plan_from_binding`).Check(testkit.Rows("1")) + + // binding created with `in (?,?,?)` can work for `in (?)` + tk.MustQuery(`select b from t where b in (1)`) + tk.MustQuery(`select @@last_plan_from_binding`).Check(testkit.Rows("0")) + tk.MustExec(`create binding for select b from t where b in (1,2,3) using select b from t where b in (1,2,3)`) + tk.MustQuery(`select b from t where b in (1)`) + tk.MustQuery(`select @@last_plan_from_binding`).Check(testkit.Rows("1")) + + // bindings with multiple in-lists can take effect + tk.MustQuery(`select * from t where a in (1) and b in (1) and c in (1)`) + tk.MustQuery(`select @@last_plan_from_binding`).Check(testkit.Rows("0")) + tk.MustExec(`create binding for select * from t where a in (1) and b in (1,2) and c in (1,2,3) using +select * from t where a in (1,2,3) and b in (1,2) and c in (1)`) + tk.MustQuery(`select * from t where a in (1) and b in (1) and c in (1)`) + tk.MustQuery(`select @@last_plan_from_binding`).Check(testkit.Rows("1")) + tk.MustQuery(`select * from t where a in (1) and b in (1,2) and c in (1,2,3)`) + tk.MustQuery(`select @@last_plan_from_binding`).Check(testkit.Rows("1")) + tk.MustQuery(`select * from t where a in (1,2,3) and b in (1,2) and c in (1)`) + tk.MustQuery(`select @@last_plan_from_binding`).Check(testkit.Rows("1")) +} + +func TestBindingInListOperation(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)`) + + // only 1 binding will be left + tk.MustExec(`create binding for select * from t where a in(1) using select * from t where a in(1)`) + tk.MustExec(`create binding for select * from t where a in(1,2) using select * from t where a in(1)`) + tk.MustExec(`create binding for select * from t where a in(1) using select * from t where a in(1,2)`) + tk.MustExec(`create binding for select * from t where a in(1,2) using select * from t where a in(1,2)`) + tk.MustExec(`create binding for select * from t where a in(1,2,3) using select * from t where a in(1,2,3)`) + require.Equal(t, 1, len(tk.MustQuery(`show bindings`).Rows())) + tk.MustExec(`drop binding for select * from t where a in(1)`) + require.Equal(t, 0, len(tk.MustQuery(`show bindings`).Rows())) + + // create and drop + tk.MustExec(`create binding for select * from t where a in(1,2,3) using select * from t where a in(1)`) + require.Equal(t, 1, len(tk.MustQuery(`show bindings`).Rows())) + tk.MustExec(`drop binding for select * from t where a in(1)`) + require.Equal(t, 0, len(tk.MustQuery(`show bindings`).Rows())) + tk.MustExec(`create binding for select * from t where a in(1) using select * from t where a in(1)`) + require.Equal(t, 1, len(tk.MustQuery(`show bindings`).Rows())) + tk.MustExec(`drop binding for select * from t where a in(1,2,3)`) + require.Equal(t, 0, len(tk.MustQuery(`show bindings`).Rows())) + tk.MustExec(`create binding for select * from t where a in(1) using select * from t where a in(1)`) + require.Equal(t, 1, len(tk.MustQuery(`show bindings`).Rows())) + tk.MustExec(`drop binding for select * from t where a in(1,2,3,4,5,6,7,8,9,0,11,12)`) + require.Equal(t, 0, len(tk.MustQuery(`show bindings`).Rows())) + + // create and set status + tk.MustExec(`create global binding for select * from t where a in(1,2,3) using select * from t where a in(1)`) + require.Equal(t, "enabled", tk.MustQuery(`show global bindings`).Rows()[0][3].(string)) + tk.MustExec(`set binding disabled for select * from t where a in(1)`) + require.Equal(t, "disabled", tk.MustQuery(`show global bindings`).Rows()[0][3].(string)) + tk.MustExec(`set binding enabled for select * from t where a in(1,2,3,4,5)`) + require.Equal(t, "enabled", tk.MustQuery(`show global bindings`).Rows()[0][3].(string)) +} + func TestPrepareCacheWithBinding(t *testing.T) { store := testkit.CreateMockStore(t) tk := testkit.NewTestKit(t, store) diff --git a/planner/core/planbuilder.go b/planner/core/planbuilder.go index b2bd17137b04c..149a3b0b7bef9 100644 --- a/planner/core/planbuilder.go +++ b/planner/core/planbuilder.go @@ -1053,7 +1053,7 @@ func (b *PlanBuilder) buildDropBindPlan(v *ast.DropBindingStmt) (Plan, error) { if v.OriginNode != nil { p = &SQLBindPlan{ SQLBindOp: OpSQLBindDrop, - NormdOrigSQL: parser.Normalize(utilparser.RestoreWithDefaultDB(v.OriginNode, b.ctx.GetSessionVars().CurrentDB, v.OriginNode.Text())), + NormdOrigSQL: parser.NormalizeForBinding(utilparser.RestoreWithDefaultDB(v.OriginNode, b.ctx.GetSessionVars().CurrentDB, v.OriginNode.Text())), IsGlobal: v.GlobalScope, Db: utilparser.GetDefaultDB(v.OriginNode, b.ctx.GetSessionVars().CurrentDB), } @@ -1076,7 +1076,7 @@ func (b *PlanBuilder) buildSetBindingStatusPlan(v *ast.SetBindingStmt) (Plan, er if v.OriginNode != nil { p = &SQLBindPlan{ SQLBindOp: OpSetBindingStatus, - NormdOrigSQL: parser.Normalize(utilparser.RestoreWithDefaultDB(v.OriginNode, b.ctx.GetSessionVars().CurrentDB, v.OriginNode.Text())), + NormdOrigSQL: parser.NormalizeForBinding(utilparser.RestoreWithDefaultDB(v.OriginNode, b.ctx.GetSessionVars().CurrentDB, v.OriginNode.Text())), Db: utilparser.GetDefaultDB(v.OriginNode, b.ctx.GetSessionVars().CurrentDB), } } else if v.SQLDigest != "" { @@ -1169,7 +1169,7 @@ func (b *PlanBuilder) buildCreateBindPlanFromPlanDigest(v *ast.CreateBindingStmt if err != nil { return nil, errors.Errorf("binding failed: %v", err) } - normdOrigSQL, sqlDigestWithDB := parser.NormalizeDigest(utilparser.RestoreWithDefaultDB(originNode, bindableStmt.Schema, bindableStmt.Query)) + normdOrigSQL, sqlDigestWithDB := parser.NormalizeDigestForBinding(utilparser.RestoreWithDefaultDB(originNode, bindableStmt.Schema, bindableStmt.Query)) p := &SQLBindPlan{ SQLBindOp: OpSQLBindCreate, NormdOrigSQL: normdOrigSQL, @@ -1202,7 +1202,7 @@ func (b *PlanBuilder) buildCreateBindPlan(v *ast.CreateBindingStmt) (Plan, error return nil, err } - normdOrigSQL, sqlDigestWithDB := parser.NormalizeDigest(utilparser.RestoreWithDefaultDB(v.OriginNode, b.ctx.GetSessionVars().CurrentDB, v.OriginNode.Text())) + normdOrigSQL, sqlDigestWithDB := parser.NormalizeDigestForBinding(utilparser.RestoreWithDefaultDB(v.OriginNode, b.ctx.GetSessionVars().CurrentDB, v.OriginNode.Text())) p := &SQLBindPlan{ SQLBindOp: OpSQLBindCreate, NormdOrigSQL: normdOrigSQL, diff --git a/planner/core/preprocess.go b/planner/core/preprocess.go index d34714036fc3c..ff26794fa6059 100644 --- a/planner/core/preprocess.go +++ b/planner/core/preprocess.go @@ -547,8 +547,8 @@ func (p *preprocessor) checkBindGrammar(originNode, hintedNode ast.StmtNode, def tn.DBInfo = dbInfo } - originSQL := parser.Normalize(utilparser.RestoreWithDefaultDB(originNode, defaultDB, originNode.Text())) - hintedSQL := parser.Normalize(utilparser.RestoreWithDefaultDB(hintedNode, defaultDB, hintedNode.Text())) + originSQL := parser.NormalizeForBinding(utilparser.RestoreWithDefaultDB(originNode, defaultDB, originNode.Text())) + hintedSQL := parser.NormalizeForBinding(utilparser.RestoreWithDefaultDB(hintedNode, defaultDB, hintedNode.Text())) if originSQL != hintedSQL { p.err = errors.Errorf("hinted sql and origin sql don't match when hinted sql erase the hint info, after erase hint info, originSQL:%s, hintedSQL:%s", originSQL, hintedSQL) }