From ad1efe451080ec60c1c592b141e780ed3cc190fb Mon Sep 17 00:00:00 2001 From: Yuanjia Zhang Date: Tue, 19 Dec 2023 20:07:52 +0800 Subject: [PATCH] planner: match hints from universal bindings correctly (#49585) ref pingcap/tidb#48875 --- pkg/bindinfo/BUILD.bazel | 2 +- pkg/bindinfo/bind_record.go | 7 +- pkg/bindinfo/universal_binding_test.go | 175 ++++++++++++++++++++++- pkg/planner/core/logical_plan_builder.go | 2 +- pkg/planner/core/planbuilder.go | 12 +- pkg/planner/core/rule_join_reorder.go | 2 +- 6 files changed, 192 insertions(+), 8 deletions(-) diff --git a/pkg/bindinfo/BUILD.bazel b/pkg/bindinfo/BUILD.bazel index 3874650f82b48..a2cff9a8afbcd 100644 --- a/pkg/bindinfo/BUILD.bazel +++ b/pkg/bindinfo/BUILD.bazel @@ -59,7 +59,7 @@ go_test( embed = [":bindinfo"], flaky = True, race = "on", - shard_count = 46, + shard_count = 47, deps = [ "//pkg/bindinfo/internal", "//pkg/config", diff --git a/pkg/bindinfo/bind_record.go b/pkg/bindinfo/bind_record.go index 241ea24cbae95..16f11399ec233 100644 --- a/pkg/bindinfo/bind_record.go +++ b/pkg/bindinfo/bind_record.go @@ -187,7 +187,12 @@ func (br *BindRecord) prepareHints(sctx sessionctx.Context) error { if (bind.Hint != nil && bind.ID != "") || bind.Status == deleted { continue } - hintsSet, stmt, warns, err := hint.ParseHintsSet(p, bind.BindSQL, bind.Charset, bind.Collation, br.Db) + dbName := br.Db + if bind.Type == TypeUniversal { + dbName = "*" // ues '*' for universal bindings + } + + hintsSet, stmt, warns, err := hint.ParseHintsSet(p, bind.BindSQL, bind.Charset, bind.Collation, dbName) if err != nil { return err } diff --git a/pkg/bindinfo/universal_binding_test.go b/pkg/bindinfo/universal_binding_test.go index 21312804a8654..d87611803db4a 100644 --- a/pkg/bindinfo/universal_binding_test.go +++ b/pkg/bindinfo/universal_binding_test.go @@ -37,6 +37,21 @@ func showBinding(tk *testkit.TestKit, showStmt string) [][]interface{} { return result } +func removeAllBindings(tk *testkit.TestKit, global bool) { + scope := "session" + if global { + scope = "global" + } + res := showBinding(tk, fmt.Sprintf("show %v bindings", scope)) + for _, r := range res { + if r[4] == "builtin" { + continue + } + tk.MustExec(fmt.Sprintf("drop %v binding for sql digest '%v'", scope, r[6])) + } + tk.MustQuery(fmt.Sprintf("show %v bindings", scope)).Check(testkit.Rows()) // empty +} + func TestUniversalBindingBasic(t *testing.T) { store := testkit.CreateMockStore(t) tk1 := testkit.NewTestKit(t, store) @@ -64,10 +79,12 @@ func TestUniversalBindingBasic(t *testing.T) { tk.MustExec("use " + useDB) for _, testDB := range []string{"", "test.", "test1.", "test2."} { tk.MustExec(`set @@tidb_opt_enable_universal_binding=1`) // enabled - tk.MustUseIndex(fmt.Sprintf("select * from %vt", testDB), idx) + require.True(t, tk.MustUseIndex(fmt.Sprintf("select * from %vt", testDB), idx)) tk.MustQuery(`select @@last_plan_from_binding`).Check(testkit.Rows("1")) + require.True(t, tk.MustUseIndex(fmt.Sprintf("select * from %vt", testDB), idx)) + tk.MustQuery(`show warnings`).Check(testkit.Rows()) // no warning tk.MustExec(`set @@tidb_opt_enable_universal_binding=0`) // disabled - tk.MustUseIndex(fmt.Sprintf("select * from %vt", testDB), idx) + tk.MustQuery(fmt.Sprintf("select * from %vt", testDB)) tk.MustQuery(`select @@last_plan_from_binding`).Check(testkit.Rows("0")) } } @@ -238,3 +255,157 @@ func TestUniversalBindingGC(t *testing.T) { require.NoError(t, bindHandle.GCGlobalBinding()) tk.MustQuery(`select bind_sql, status, type from mysql.bind_info where source != 'builtin'`).Check(testkit.Rows()) // empty after GC } + +func TestUniversalBindingHints(t *testing.T) { + store := testkit.CreateMockStore(t) + tk := testkit.NewTestKit(t, store) + tk.MustExec(`use test`) + + for _, db := range []string{"db1", "db2", "db3"} { + tk.MustExec(`create database ` + db) + tk.MustExec(`use ` + db) + tk.MustExec(`create table t1 (a int, b int, c int, d int, key(a), key(b), key(c), key(d))`) + tk.MustExec(`create table t2 (a int, b int, c int, d int, key(a), key(b), key(c), key(d))`) + tk.MustExec(`create table t3 (a int, b int, c int, d int, key(a), key(b), key(c), key(d))`) + } + tk.MustExec(`set @@tidb_opt_enable_universal_binding=1`) + + for _, c := range []struct { + binding string + qTemplate string + }{ + // use index + {`create universal binding using select /*+ use_index(t1, c) */ * from t1 where a=1`, + `select * from %st1 where a=1000`}, + {`create universal binding using select /*+ use_index(t1, c) */ * from t1 where d<1`, + `select * from %st1 where d<10000`}, + {`create universal binding using select /*+ use_index(t1, c) */ * from t1, t2 where t1.d<1`, + `select * from %st1, t2 where t1.d<100`}, + {`create universal binding using select /*+ use_index(t1, c) */ * from t1, t2 where t1.d<1`, + `select * from t1, %st2 where t1.d<100`}, + {`create universal binding using select /*+ use_index(t1, c), use_index(t2, a) */ * from t1, t2 where t1.d<1`, + `select * from %st1, t2 where t1.d<100`}, + {`create universal binding using select /*+ use_index(t1, c), use_index(t2, a) */ * from t1, t2 where t1.d<1`, + `select * from t1, %st2 where t1.d<100`}, + {`create universal binding using select /*+ use_index(t1, c), use_index(t2, a) */ * from t1, t2, t3 where t1.d<1`, + `select * from %st1, t2, t3 where t1.d<100`}, + {`create universal binding using select /*+ use_index(t1, c), use_index(t2, a) */ * from t1, t2, t3 where t1.d<1`, + `select * from t1, t2, %st3 where t1.d<100`}, + + // ignore index + {`create universal binding using select /*+ ignore_index(t1, b) */ * from t1 where b=1`, + `select * from %st1 where b=1000`}, + {`create universal binding using select /*+ ignore_index(t1, b) */ * from t1 where b>1`, + `select * from %st1 where b>1000`}, + {`create universal binding using select /*+ ignore_index(t1, b) */ * from t1 where b in (1,2)`, + `select * from %st1 where b in (1)`}, + {`create universal binding using select /*+ ignore_index(t1, b) */ * from t1 where b in (1,2)`, + `select * from %st1 where b in (1,2,3,4,5)`}, + + // order index hint + {`create universal binding using select /*+ order_index(t1, a) */ a from t1 where a<10 order by a limit 10`, + `select a from %st1 where a<10000 order by a limit 10`}, + {`create universal binding using select /*+ order_index(t1, b) */ b from t1 where b>10 order by b limit 1111`, + `select b from %st1 where b>2 order by b limit 10`}, + + // no order index hint + {`create universal binding using select /*+ no_order_index(t1, c) */ c from t1 where c<10 order by c limit 10`, + `select c from %st1 where c<10000 order by c limit 10`}, + {`create universal binding using select /*+ no_order_index(t1, d) */ d from t1 where d>10 order by d limit 1111`, + `select d from %st1 where d>2 order by d limit 10`}, + + // agg hint + {`create universal binding using select /*+ hash_agg() */ count(*) from t1 group by a`, + `select count(*) from %st1 group by a`}, + {`create universal binding using select /*+ stream_agg() */ count(*) from t1 group by b`, + `select count(*) from %st1 group by b`}, + + // to_cop hint + {`create universal binding using select /*+ agg_to_cop() */ sum(a) from t1`, + `select sum(a) from %st1`}, + {`create universal binding using select /*+ limit_to_cop() */ a from t1 limit 10`, + `select a from %st1 limit 101`}, + + // index merge hint + {`create universal binding using select /*+ use_index_merge(t1, c, d) */ * from t1 where c=1 or d=1`, + `select * from %st1 where c=1000 or d=1000`}, + {`create universal binding using select /*+ no_index_merge() */ * from t1 where a=1 or b=1`, + `select * from %st1 where a=1000 or b=1000`}, + + // join type hint + {`create universal binding using select /*+ hash_join(t1) */ * from t1, t2 where t1.a=t2.a`, + `select * from %st1, t2 where t1.a=t2.a`}, + {`create universal binding using select /*+ hash_join(t2) */ * from t1, t2 where t1.a=t2.a`, + `select * from t1, %st2 where t1.a=t2.a`}, + {`create universal binding using select /*+ hash_join(t2) */ * from t1, t2, t3 where t1.a=t2.a and t3.b=t2.b`, + `select * from t1, %st2, t3 where t1.a=t2.a and t3.b=t2.b`}, + {`create universal binding using select /*+ hash_join_build(t1) */ * from t1, t2 where t1.a=t2.a`, + `select * from t1, %st2 where t1.a=t2.a`}, + {`create universal binding using select /*+ hash_join_probe(t1) */ * from t1, t2 where t1.a=t2.a`, + `select * from t1, %st2 where t1.a=t2.a`}, + {`create universal binding using select /*+ merge_join(t1) */ * from t1, t2 where t1.a=t2.a`, + `select * from %st1, t2 where t1.a=t2.a`}, + {`create universal binding using select /*+ merge_join(t2) */ * from t1, t2 where t1.a=t2.a`, + `select * from t1, %st2 where t1.a=t2.a`}, + {`create universal binding using select /*+ merge_join(t2) */ * from t1, t2, t3 where t1.a=t2.a and t3.b=t2.b`, + `select * from t1, %st2, t3 where t1.a=t2.a and t3.b=t2.b`}, + {`create universal binding using select /*+ inl_join(t1) */ * from t1, t2 where t1.a=t2.a`, + `select * from %st1, t2 where t1.a=t2.a`}, + {`create universal binding using select /*+ inl_join(t2) */ * from t1, t2 where t1.a=t2.a`, + `select * from t1, %st2 where t1.a=t2.a`}, + {`create universal binding using select /*+ inl_join(t2) */ * from t1, t2, t3 where t1.a=t2.a and t3.b=t2.b`, + `select * from t1, %st2, t3 where t1.a=t2.a and t3.b=t2.b`}, + + // TODO: no_xxx_join hints are not compatible with bindings, fix later. + // no join type hint + //{`create universal binding using select /*+ no_hash_join(t1) */ * from t1, t2 where t1.b=t2.b`, + // `select * from %st1, t2 where t1.b=t2.b`}, + //{`create universal binding using select /*+ no_hash_join(t2) */ * from t1, t2 where t1.c=t2.c`, + // `select * from t1, %st2 where t1.c=t2.c`}, + //{`create universal binding using select /*+ no_hash_join(t2) */ * from t1, t2, t3 where t1.a=t2.a and t3.b=t2.b`, + // `select * from t1, %st2, t3 where t1.a=t2.a and t3.b=t2.b`}, + //{`create universal binding using select /*+ no_merge_join(t1) */ * from t1, t2 where t1.b=t2.b`, + // `select * from %st1, t2 where t1.b=t2.b`}, + //{`create universal binding using select /*+ no_merge_join(t2) */ * from t1, t2 where t1.c=t2.c`, + // `select * from t1, %st2 where t1.c=t2.c`}, + //{`create universal binding using select /*+ no_merge_join(t2) */ * from t1, t2, t3 where t1.a=t2.a and t3.b=t2.b`, + // `select * from t1, %st2, t3 where t1.a=t2.a and t3.b=t2.b`}, + //{`create universal binding using select /*+ no_index_join(t1) */ * from t1, t2 where t1.b=t2.b`, + // `select * from %st1, t2 where t1.b=t2.b`}, + //{`create universal binding using select /*+ no_index_join(t2) */ * from t1, t2 where t1.c=t2.c`, + // `select * from t1, %st2 where t1.c=t2.c`}, + //{`create universal binding using select /*+ no_index_join(t2) */ * from t1, t2, t3 where t1.a=t2.a and t3.b=t2.b`, + // `select * from t1, %st2, t3 where t1.a=t2.a and t3.b=t2.b`}, + + // join order hint + {`create universal binding using select /*+ leading(t2) */ * from t1, t2 where t1.b=t2.b`, + `select * from %st1, t2 where t1.b=t2.b`}, + {`create universal binding using select /*+ leading(t2) */ * from t1, t2 where t1.c=t2.c`, + `select * from t1, %st2 where t1.c=t2.c`}, + {`create universal binding using select /*+ leading(t2, t1) */ * from t1, t2 where t1.c=t2.c`, + `select * from t1, %st2 where t1.c=t2.c`}, + {`create universal binding using select /*+ leading(t1, t2) */ * from t1, t2 where t1.c=t2.c`, + `select * from t1, %st2 where t1.c=t2.c`}, + {`create universal binding using select /*+ leading(t1) */ * from t1, t2, t3 where t1.a=t2.a and t3.b=t2.b`, + `select * from t1, %st2, t3 where t1.a=t2.a and t3.b=t2.b`}, + {`create universal binding using select /*+ leading(t2) */ * from t1, t2, t3 where t1.a=t2.a and t3.b=t2.b`, + `select * from t1, %st2, t3 where t1.a=t2.a and t3.b=t2.b`}, + {`create universal binding using select /*+ leading(t2,t3) */ * from t1, t2, t3 where t1.a=t2.a and t3.b=t2.b`, + `select * from t1, %st2, t3 where t1.a=t2.a and t3.b=t2.b`}, + {`create universal binding using select /*+ leading(t2,t3,t1) */ * from t1, t2, t3 where t1.a=t2.a and t3.b=t2.b`, + `select * from t1, %st2, t3 where t1.a=t2.a and t3.b=t2.b`}, + } { + removeAllBindings(tk, false) + tk.MustExec(c.binding) + for _, currentDB := range []string{"db1", "db2", "db3"} { + tk.MustExec(`use ` + currentDB) + for _, db := range []string{"db1.", "db2.", "db3.", ""} { + query := fmt.Sprintf(c.qTemplate, db) + tk.MustExec(query) + tk.MustQuery(`show warnings`).Check(testkit.Rows()) // no warning + tk.MustExec(query) + tk.MustQuery(`select @@last_plan_from_binding`).Check(testkit.Rows("1")) + } + } + } +} diff --git a/pkg/planner/core/logical_plan_builder.go b/pkg/planner/core/logical_plan_builder.go index 50e6745014e05..2d241056f2415 100644 --- a/pkg/planner/core/logical_plan_builder.go +++ b/pkg/planner/core/logical_plan_builder.go @@ -5195,7 +5195,7 @@ func (b *PlanBuilder) buildDataSource(ctx context.Context, tn *ast.TableName, as var indexMergeHints []indexHintInfo if hints := b.TableHints(); hints != nil { for i, hint := range hints.indexMergeHintList { - if hint.tblName.L == tblName.L && hint.dbName.L == dbName.L { + if hint.match(dbName, tblName) { hints.indexMergeHintList[i].matched = true // check whether the index names in IndexMergeHint are valid. invalidIdxNames := make([]string, 0, len(hint.indexHint.IndexNames)) diff --git a/pkg/planner/core/planbuilder.go b/pkg/planner/core/planbuilder.go index 89c5bdcfea2c2..f0f644a4b1d5c 100644 --- a/pkg/planner/core/planbuilder.go +++ b/pkg/planner/core/planbuilder.go @@ -139,6 +139,12 @@ type indexHintInfo struct { matched bool } +func (hint *indexHintInfo) match(dbName, tblName model.CIStr) bool { + return hint.tblName.L == tblName.L && + (hint.dbName.L == dbName.L || + hint.dbName.L == "*") // for universal bindings, e.g. *.t +} + func (hint *indexHintInfo) hintTypeString() string { switch hint.indexHint.HintType { case ast.HintUse: @@ -322,7 +328,9 @@ func (*tableHintInfo) matchTableName(tables []*hintTableInfo, hintTables []hintT if table == nil { continue } - if curEntry.dbName.L == table.dbName.L && curEntry.tblName.L == table.tblName.L && table.selectOffset == curEntry.selectOffset { + if (curEntry.dbName.L == table.dbName.L || curEntry.dbName.L == "*") && + curEntry.tblName.L == table.tblName.L && + table.selectOffset == curEntry.selectOffset { hintTables[i].matched = true hintMatched = true break @@ -1454,7 +1462,7 @@ func getPossibleAccessPaths(ctx sessionctx.Context, tableHints *tableHintInfo, i indexHintsLen := len(indexHints) if tableHints != nil { for i, hint := range tableHints.indexHintList { - if hint.dbName.L == dbName.L && hint.tblName.L == tblName.L { + if hint.match(dbName, tblName) { indexHints = append(indexHints, hint.indexHint) tableHints.indexHintList[i].matched = true } diff --git a/pkg/planner/core/rule_join_reorder.go b/pkg/planner/core/rule_join_reorder.go index 98f066ed877d8..0a67b68c0e795 100644 --- a/pkg/planner/core/rule_join_reorder.go +++ b/pkg/planner/core/rule_join_reorder.go @@ -411,7 +411,7 @@ func (s *baseSingleGroupJoinOrderSolver) generateLeadingJoinGroup(curJoinGroup [ if tableAlias == nil { continue } - if hintTbl.dbName.L == tableAlias.dbName.L && hintTbl.tblName.L == tableAlias.tblName.L && hintTbl.selectOffset == tableAlias.selectOffset { + if (hintTbl.dbName.L == tableAlias.dbName.L || hintTbl.dbName.L == "*") && hintTbl.tblName.L == tableAlias.tblName.L && hintTbl.selectOffset == tableAlias.selectOffset { match = true leadingJoinGroup = append(leadingJoinGroup, joinGroup) leftJoinGroup = append(leftJoinGroup[:i], leftJoinGroup[i+1:]...)