diff --git a/pkg/bindinfo/BUILD.bazel b/pkg/bindinfo/BUILD.bazel index 504cc88a10250..eeeecc8ce5b9f 100644 --- a/pkg/bindinfo/BUILD.bazel +++ b/pkg/bindinfo/BUILD.bazel @@ -23,7 +23,6 @@ go_library( "//pkg/sessionctx/sessionstates", "//pkg/sessionctx/variable", "//pkg/types", - "//pkg/types/parser_driver", "//pkg/util", "//pkg/util/chunk", "//pkg/util/hack", diff --git a/pkg/bindinfo/binding.go b/pkg/bindinfo/binding.go index 7bd9df2abe496..a2ced548d3c56 100644 --- a/pkg/bindinfo/binding.go +++ b/pkg/bindinfo/binding.go @@ -264,7 +264,7 @@ func (*tableNameCollector) Leave(in ast.Node) (out ast.Node, ok bool) { // prepareHints builds ID and Hint for Bindings. If sctx is not nil, we check if // the BindSQL is still valid. -func prepareHints(sctx sessionctx.Context, binding *Binding) (rerr error) { +func prepareHints(_ sessionctx.Context, binding *Binding) (rerr error) { defer func() { if r := recover(); r != nil { rerr = errors.Errorf("panic when preparing hints for binding %v, panic: %v", binding.BindSQL, r) @@ -281,25 +281,15 @@ func prepareHints(sctx sessionctx.Context, binding *Binding) (rerr error) { return err } tableNames := CollectTableNames(bindingStmt) - isFuzzy := isCrossDBBinding(bindingStmt) - if isFuzzy { + isCrossDB := isCrossDBBinding(bindingStmt) + if isCrossDB { dbName = "*" // ues '*' for universal bindings } - hintsSet, stmt, warns, err := hint.ParseHintsSet(p, binding.BindSQL, binding.Charset, binding.Collation, dbName) + hintsSet, _, warns, err := hint.ParseHintsSet(p, binding.BindSQL, binding.Charset, binding.Collation, dbName) if err != nil { return err } - if sctx != nil && !isFuzzy { - paramChecker := ¶mMarkerChecker{} - stmt.Accept(paramChecker) - if !paramChecker.hasParamMarker { - _, err = getHintsForSQL(sctx, binding.BindSQL) - if err != nil { - return err - } - } - } hintsStr, err := hintsSet.Restore() if err != nil { return err diff --git a/pkg/bindinfo/global_handle.go b/pkg/bindinfo/global_handle.go index 72055ed6f62bf..3f6e3145808ee 100644 --- a/pkg/bindinfo/global_handle.go +++ b/pkg/bindinfo/global_handle.go @@ -34,7 +34,6 @@ import ( "github.com/pingcap/tidb/pkg/sessionctx" "github.com/pingcap/tidb/pkg/sessionctx/variable" "github.com/pingcap/tidb/pkg/types" - driver "github.com/pingcap/tidb/pkg/types/parser_driver" "github.com/pingcap/tidb/pkg/util" "github.com/pingcap/tidb/pkg/util/chunk" "github.com/pingcap/tidb/pkg/util/hint" @@ -418,50 +417,12 @@ func newBindingFromStorage(sctx sessionctx.Context, row chunk.Row) (string, *Bin return sqlDigest.String(), binding, err } -func getHintsForSQL(sctx sessionctx.Context, sql string) (string, error) { - origVals := sctx.GetSessionVars().UsePlanBaselines - sctx.GetSessionVars().UsePlanBaselines = false - - // Usually passing a sprintf to ExecuteInternal is not recommended, but in this case - // it is safe because ExecuteInternal does not permit MultiStatement execution. Thus, - // the statement won't be able to "break out" from EXPLAIN. - rs, err := exec(sctx, fmt.Sprintf("EXPLAIN FORMAT='hint' %s", sql)) - sctx.GetSessionVars().UsePlanBaselines = origVals - if rs != nil { - defer func() { - // Audit log is collected in Close(), set InRestrictedSQL to avoid 'create sql binding' been recorded as 'explain'. - origin := sctx.GetSessionVars().InRestrictedSQL - sctx.GetSessionVars().InRestrictedSQL = true - terror.Call(rs.Close) - sctx.GetSessionVars().InRestrictedSQL = origin - }() - } - if err != nil { - return "", err - } - chk := rs.NewChunk(nil) - err = rs.Next(context.TODO(), chk) - if err != nil { - return "", err - } - return chk.GetRow(0).GetString(0), nil -} - // GenerateBindingSQL generates binding sqls from stmt node and plan hints. -func GenerateBindingSQL(stmtNode ast.StmtNode, planHint string, skipCheckIfHasParam bool, defaultDB string) string { +func GenerateBindingSQL(stmtNode ast.StmtNode, planHint string, defaultDB string) string { // If would be nil for very simple cases such as point get, we do not need to evolve for them. if planHint == "" { return "" } - if !skipCheckIfHasParam { - paramChecker := ¶mMarkerChecker{} - stmtNode.Accept(paramChecker) - // We need to evolve on current sql, but we cannot restore values for paramMarkers yet, - // so just ignore them now. - if paramChecker.hasParamMarker { - return "" - } - } // We need to evolve plan based on the current sql, not the original sql which may have different parameters. // So here we would remove the hint and inject the current best plan hint. hint.BindHint(stmtNode, &hint.HintsSet{}) @@ -514,22 +475,6 @@ func GenerateBindingSQL(stmtNode ast.StmtNode, planHint string, skipCheckIfHasPa return "" } -type paramMarkerChecker struct { - hasParamMarker bool -} - -func (e *paramMarkerChecker) Enter(in ast.Node) (ast.Node, bool) { - if _, ok := in.(*driver.ParamMarkerExpr); ok { - e.hasParamMarker = true - return in, true - } - return in, false -} - -func (*paramMarkerChecker) Leave(in ast.Node) (ast.Node, bool) { - return in, true -} - func (h *globalBindingHandle) callWithSCtx(wrapTxn bool, f func(sctx sessionctx.Context) error) (err error) { resource, err := h.sPool.Get() if err != nil { diff --git a/pkg/bindinfo/global_handle_test.go b/pkg/bindinfo/global_handle_test.go index d01ab43477646..20378dedf03a2 100644 --- a/pkg/bindinfo/global_handle_test.go +++ b/pkg/bindinfo/global_handle_test.go @@ -97,11 +97,10 @@ func TestBindingLastUpdateTimeWithInvalidBind(t *testing.T) { updateTime0 := rows0[0][1] require.Equal(t, updateTime0, "0000-00-00 00:00:00") - tk.MustExec("insert into mysql.bind_info values('select * from `test` . `t`', 'select * from `test` . `t` use index(`idx`)', 'test', 'enabled', '2000-01-01 09:00:00', '2000-01-01 09:00:00', '', '','" + + tk.MustExec("insert into mysql.bind_info values('select * from `test` . `t`', 'invalid_binding', 'test', 'enabled', '2000-01-01 09:00:00', '2000-01-01 09:00:00', '', '','" + bindinfo.Manual + "', '', '')") tk.MustExec("use test") tk.MustExec("drop table if exists t") - tk.MustExec("create table t(a int)") tk.MustExec("admin reload bindings;") rows1 := tk.MustQuery("show status like 'last_plan_binding_update_time';").Rows() @@ -748,7 +747,7 @@ func TestErrorBind(t *testing.T) { tk := testkit.NewTestKit(t, store) tk.MustExec("use test") - tk.MustGetErrMsg("create global binding for select * from t using select * from t", "[schema:1146]Table 'test.t' doesn't exist") + tk.MustContainErrMsg("create global binding for select * xxx", "You have an error in your SQL syntax") tk.MustExec("drop table if exists t") tk.MustExec("drop table if exists t1") tk.MustExec("create table t(i int, s varchar(20))") diff --git a/pkg/bindinfo/tests/bind_test.go b/pkg/bindinfo/tests/bind_test.go index be0dc0e41afd9..83cb0749c9acb 100644 --- a/pkg/bindinfo/tests/bind_test.go +++ b/pkg/bindinfo/tests/bind_test.go @@ -348,12 +348,10 @@ func TestInvisibleIndex(t *testing.T) { tk.MustExec("use test") tk.MustExec("drop table if exists t") tk.MustExec("create table t(a int, b int, unique idx_a(a), index idx_b(b) invisible)") - tk.MustGetErrMsg( - "create global binding for select * from t using select * from t use index(idx_b) ", - "[planner:1176]Key 'idx_b' doesn't exist in table 't'") // Create bind using index - tk.MustExec("create global binding for select * from t using select * from t use index(idx_a) ") + tk.MustExec("create global binding for select * from t using select * from t use index(idx_b)") + tk.MustExec("create global binding for select * from t using select * from t use index(idx_a)") tk.MustQuery("select * from t") require.Equal(t, "t:idx_a", tk.Session().GetSessionVars().StmtCtx.IndexNames[0]) diff --git a/pkg/planner/core/planbuilder.go b/pkg/planner/core/planbuilder.go index d6405611d52f4..184f3ea803e7d 100644 --- a/pkg/planner/core/planbuilder.go +++ b/pkg/planner/core/planbuilder.go @@ -918,7 +918,7 @@ func constructSQLBindOPFromPlanDigest( return nil, errors.NewNoStackErrorf("binding failed: %v. Plan Digest: %v", err, planDigest) } complete, reason := hint.CheckBindingFromHistoryComplete(originNode, bindableStmt.PlanHint) - bindSQL := bindinfo.GenerateBindingSQL(originNode, bindableStmt.PlanHint, true, bindableStmt.Schema) + bindSQL := bindinfo.GenerateBindingSQL(originNode, bindableStmt.PlanHint, bindableStmt.Schema) var hintNode ast.StmtNode hintNode, err = parser4binding.ParseOneStmt(bindSQL, bindableStmt.Charset, bindableStmt.Collation) if err != nil { diff --git a/pkg/sessionctx/sessionstates/session_states_test.go b/pkg/sessionctx/sessionstates/session_states_test.go index 2caea78e5575b..474293a48ccf0 100644 --- a/pkg/sessionctx/sessionstates/session_states_test.go +++ b/pkg/sessionctx/sessionstates/session_states_test.go @@ -1293,7 +1293,9 @@ func TestSQLBinding(t *testing.T) { tk.MustExec("drop table test.t1") return nil }, - restoreErr: errno.ErrNoSuchTable, + checkFunc: func(tk *testkit.TestKit, param any) { + require.Equal(t, 1, len(tk.MustQuery("show session bindings").Rows())) + }, cleanFunc: func(tk *testkit.TestKit) { tk.MustExec("create table test.t1(id int primary key, name varchar(10), key(name))") }, @@ -1308,7 +1310,9 @@ func TestSQLBinding(t *testing.T) { tk.MustExec("drop database test1") return nil }, - restoreErr: errno.ErrNoSuchTable, + checkFunc: func(tk *testkit.TestKit, param any) { + require.Equal(t, 1, len(tk.MustQuery("show session bindings").Rows())) + }, }, { // alter the table @@ -1317,7 +1321,9 @@ func TestSQLBinding(t *testing.T) { tk.MustExec("alter table test.t1 drop index name") return nil }, - restoreErr: errno.ErrKeyDoesNotExist, + checkFunc: func(tk *testkit.TestKit, param any) { + require.Equal(t, 1, len(tk.MustQuery("show session bindings").Rows())) + }, cleanFunc: func(tk *testkit.TestKit) { tk.MustExec("alter table test.t1 add index name(name)") },