Skip to content

Commit

Permalink
planner: remove useless binding code and add more test cases (pingcap…
Browse files Browse the repository at this point in the history
  • Loading branch information
qw4990 authored and AilinKid committed Jan 17, 2024
1 parent ed412df commit 5dc5145
Show file tree
Hide file tree
Showing 8 changed files with 10 additions and 50 deletions.
11 changes: 1 addition & 10 deletions pkg/bindinfo/bind_record.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:"-"`
Expand Down Expand Up @@ -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 := &paramMarkerChecker{}
stmt.Accept(paramChecker)
if !paramChecker.hasParamMarker {
Expand Down
32 changes: 9 additions & 23 deletions pkg/bindinfo/fuzzy_binding_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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`)
Expand Down
5 changes: 0 additions & 5 deletions pkg/bindinfo/global_handle.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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{
Expand Down
7 changes: 0 additions & 7 deletions pkg/executor/bind.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down
1 change: 0 additions & 1 deletion pkg/executor/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 0 additions & 1 deletion pkg/planner/core/common_plans.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 0 additions & 2 deletions pkg/planner/core/planbuilder.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
1 change: 0 additions & 1 deletion pkg/server/testdata/optimizer_suite_out.json
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,6 @@
"SQLDigest": "36ceb6159adb3ac83539ec90c861ac4be4bc5cdb5fa02f70542744a4af640eac",
"Source": "manual",
"Status": "enabled",
"Type": "",
"UpdateTime": 0
}
],
Expand Down

0 comments on commit 5dc5145

Please sign in to comment.