planner: simplify global and session binding handle (#50063)

ref #48875

pkg/bindinfo/bind_cache.go

@@ -148,7 +148,7 @@ func (c *bindCache) SetBinding(sqlDigest string, meta *BindRecord) (err error) {
 
 // RemoveBinding removes the BindRecord which has same originSQL with specified BindRecord.
 // The function is thread-safe.
-func (c *bindCache) RemoveBinding(sqlDigest string, _ *BindRecord) {
+func (c *bindCache) RemoveBinding(sqlDigest string) {
 	c.lock.Lock()
 	defer c.lock.Unlock()
 	c.delete(bindCacheKey(sqlDigest))

pkg/bindinfo/bind_record.go

@@ -250,24 +250,6 @@ func merge(lBindRecord, rBindRecord *BindRecord) *BindRecord {
 	return result
 }
 
-func (br *BindRecord) remove(deleted *BindRecord) *BindRecord {
-	// Delete all bindings.
-	if len(deleted.Bindings) == 0 {
-		return &BindRecord{OriginalSQL: br.OriginalSQL, Db: br.Db}
-	}
-	result := br.shallowCopy()
-	for j := range deleted.Bindings {
-		deletedBind := deleted.Bindings[j]
-		for i, bind := range result.Bindings {
-			if bind.isSame(&deletedBind) {
-				result.Bindings = append(result.Bindings[:i], result.Bindings[i+1:]...)
-				break
-			}
-		}
-	}
-	return result
-}
-
 func (br *BindRecord) removeDeletedBindings() *BindRecord {
 	result := BindRecord{OriginalSQL: br.OriginalSQL, Db: br.Db, Bindings: make([]Binding, 0, len(br.Bindings))}
 	for _, binding := range br.Bindings {

pkg/bindinfo/capture.go

@@ -171,7 +171,7 @@ func (h *globalBindingHandle) CaptureBaselines() {
 		}
 		dbName := utilparser.GetDefaultDB(stmt, bindableStmt.Schema)
 		normalizedSQL, digest := parser.NormalizeDigest(utilparser.RestoreWithDefaultDB(stmt, dbName, bindableStmt.Query))
-		if r := h.GetGlobalBinding(digest.String()); r != nil && r.HasAvailableBinding() {
+		if r := h.getCache().GetBinding(digest.String()); r != nil && r.HasAvailableBinding() {
 			continue
 		}
 		bindSQL := GenerateBindSQL(context.TODO(), stmt, bindableStmt.PlanHint, true, dbName)

pkg/bindinfo/global_handle.go

@@ -59,10 +59,7 @@ type GlobalBindingHandle interface {
 	DropGlobalBinding(sqlDigest string) (deletedRows uint64, err error)
 
 	// SetGlobalBindingStatus set a BindRecord's status to the storage and bind cache.
-	SetGlobalBindingStatus(originalSQL string, binding *Binding, newStatus string) (ok bool, err error)
-
-	// SetGlobalBindingStatusByDigest set a BindRecord's status to the storage and bind cache.
-	SetGlobalBindingStatusByDigest(newStatus, sqlDigest string) (ok bool, err error)
+	SetGlobalBindingStatus(newStatus, sqlDigest string) (ok bool, err error)
 
 	// AddInvalidGlobalBinding adds BindRecord which needs to be deleted into invalidBindRecordMap.
 	AddInvalidGlobalBinding(invalidBindRecord *BindRecord)
@@ -173,8 +170,12 @@ func (h *globalBindingHandle) Reset() {
 	h.lastUpdateTime.Store(types.ZeroTimestamp)
 	h.invalidBindRecordMap.Value.Store(make(map[string]*bindRecordUpdate))
 	h.invalidBindRecordMap.flushFunc = func(record *BindRecord) error {
-		_, err := h.dropGlobalBinding(record.OriginalSQL, record.Db)
-		return err
+		for _, binding := range record.Bindings {
+			if _, err := h.dropGlobalBinding(binding.SQLDigest); err != nil {
+				return err
+			}
+		}
+		return nil
 	}
 	h.setCache(newBindCache())
 	variable.RegisterStatistics(h)
@@ -250,7 +251,7 @@ func (h *globalBindingHandle) LoadFromStorageToCache(fullLoad bool) (err error)
 					logutil.BgLogger().Warn("BindHandle.Update", zap.String("category", "sql-bind"), zap.Error(err))
 				}
 			} else {
-				newCache.RemoveBinding(sqlDigest, newRecord)
+				newCache.RemoveBinding(sqlDigest)
 			}
 			updateMetrics(metrics.ScopeGlobal, oldRecord, newCache.GetBinding(sqlDigest), true)
 		}
@@ -314,13 +315,7 @@ func (h *globalBindingHandle) CreateGlobalBinding(sctx sessionctx.Context, recor
 }
 
 // dropGlobalBinding drops a BindRecord to the storage and BindRecord int the cache.
-func (h *globalBindingHandle) dropGlobalBinding(originalSQL, _ string) (deletedRows uint64, err error) {
-	defer func() {
-		if err == nil {
-			err = h.LoadFromStorageToCache(false)
-		}
-	}()
-
+func (h *globalBindingHandle) dropGlobalBinding(sqlDigest string) (deletedRows uint64, err error) {
 	err = h.callWithSCtx(false, func(sctx sessionctx.Context) error {
 		// Lock mysql.bind_info to synchronize with CreateBindRecord / AddBindRecord / DropBindRecord on other tidb instances.
 		if err = lockBindInfoTable(sctx); err != nil {
@@ -329,8 +324,8 @@ func (h *globalBindingHandle) dropGlobalBinding(originalSQL, _ string) (deletedR
 
 		updateTs := types.NewTime(types.FromGoTime(time.Now()), mysql.TypeTimestamp, 3).String()
 
-		_, err = exec(sctx, `UPDATE mysql.bind_info SET status = %?, update_time = %? WHERE original_sql = %? AND update_time < %? AND status != %?`,
-			deleted, updateTs, originalSQL, updateTs, deleted)
+		_, err = exec(sctx, `UPDATE mysql.bind_info SET status = %?, update_time = %? WHERE sql_digest = %? AND update_time < %? AND status != %?`,
+			deleted, updateTs, sqlDigest, updateTs, deleted)
 		if err != nil {
 			return err
 		}
@@ -345,15 +340,16 @@ func (h *globalBindingHandle) DropGlobalBinding(sqlDigest string) (deletedRows u
 	if sqlDigest == "" {
 		return 0, errors.New("sql digest is empty")
 	}
-	oldRecord := h.GetGlobalBinding(sqlDigest)
-	if oldRecord == nil {
-		return 0, errors.Errorf("can't find any binding for '%s'", sqlDigest)
-	}
-	return h.dropGlobalBinding(oldRecord.OriginalSQL, strings.ToLower(oldRecord.Db))
+	defer func() {
+		if err == nil {
+			err = h.LoadFromStorageToCache(false)
+		}
+	}()
+	return h.dropGlobalBinding(sqlDigest)
 }
 
 // SetGlobalBindingStatus set a BindRecord's status to the storage and bind cache.
-func (h *globalBindingHandle) SetGlobalBindingStatus(originalSQL string, binding *Binding, newStatus string) (ok bool, err error) {
+func (h *globalBindingHandle) SetGlobalBindingStatus(newStatus, sqlDigest string) (ok bool, err error) {
 	var (
 		updateTs               types.Time
 		oldStatus0, oldStatus1 string
@@ -384,27 +380,13 @@ func (h *globalBindingHandle) SetGlobalBindingStatus(originalSQL string, binding
 		updateTs = types.NewTime(types.FromGoTime(time.Now()), mysql.TypeTimestamp, 3)
 		updateTsStr := updateTs.String()
 
-		if binding == nil {
-			_, err = exec(sctx, `UPDATE mysql.bind_info SET status = %?, update_time = %? WHERE original_sql = %? AND update_time < %? AND status IN (%?, %?)`,
-				newStatus, updateTsStr, originalSQL, updateTsStr, oldStatus0, oldStatus1)
-		} else {
-			_, err = exec(sctx, `UPDATE mysql.bind_info SET status = %?, update_time = %? WHERE original_sql = %? AND update_time < %? AND bind_sql = %? AND status IN (%?, %?)`,
-				newStatus, updateTsStr, originalSQL, updateTsStr, binding.BindSQL, oldStatus0, oldStatus1)
-		}
-		return nil
+		_, err = exec(sctx, `UPDATE mysql.bind_info SET status = %?, update_time = %? WHERE sql_digest = %? AND update_time < %? AND status IN (%?, %?)`,
+			newStatus, updateTsStr, sqlDigest, updateTsStr, oldStatus0, oldStatus1)
+		return err
 	})
 	return
 }
 
-// SetGlobalBindingStatusByDigest set a BindRecord's status to the storage and bind cache.
-func (h *globalBindingHandle) SetGlobalBindingStatusByDigest(newStatus, sqlDigest string) (ok bool, err error) {
-	oldRecord := h.GetGlobalBinding(sqlDigest)
-	if oldRecord == nil {
-		return false, errors.Errorf("can't find any binding for '%s'", sqlDigest)
-	}
-	return h.SetGlobalBindingStatus(oldRecord.OriginalSQL, nil, newStatus)
-}
-
 // GCGlobalBinding physically removes the deleted bind records in mysql.bind_info.
 func (h *globalBindingHandle) GCGlobalBinding() (err error) {
 	return h.callWithSCtx(true, func(sctx sessionctx.Context) error {
@@ -486,6 +468,11 @@ func (tmpMap *tmpBindRecordMap) Add(bindRecord *BindRecord) {
 
 // DropInvalidGlobalBinding executes the drop BindRecord tasks.
 func (h *globalBindingHandle) DropInvalidGlobalBinding() {
+	defer func() {
+		if err := h.LoadFromStorageToCache(false); err != nil {
+			logutil.BgLogger().Warn("drop invalid global binding error", zap.Error(err))
+		}
+	}()
 	h.invalidBindRecordMap.flushToStore()
 }
 
@@ -514,11 +501,6 @@ func (h *globalBindingHandle) MatchGlobalBinding(currentDB string, stmt ast.Stmt
 	return bindingCache.GetBinding(sqlDigest), nil
 }
 
-// GetGlobalBinding returns the BindRecord of the (normalizedSQL,db) if BindRecord exist.
-func (h *globalBindingHandle) GetGlobalBinding(sqlDigest string) *BindRecord {
-	return h.getCache().GetBinding(sqlDigest)
-}
-
 // GetAllGlobalBindings returns all bind records in cache.
 func (h *globalBindingHandle) GetAllGlobalBindings() (bindRecords []*BindRecord) {
 	return h.getCache().GetAllBindings()

pkg/bindinfo/global_handle_test.go

@@ -211,14 +211,6 @@ func TestSetBindingStatus(t *testing.T) {
 	tk.MustExec("select * from t where a > 10")
 	tk.MustQuery("select @@last_plan_from_binding").Check(testkit.Rows("1"))
 
-	tk.MustExec("set binding disabled for select * from t where a > 10 using select * from t where a > 10")
-	tk.MustQuery("show warnings").Check(testkit.Rows("Warning 1105 There are no bindings can be set the status. Please check the SQL text"))
-	rows = tk.MustQuery("show global bindings").Rows()
-	require.Len(t, rows, 1)
-	require.Equal(t, bindinfo.Enabled, rows[0][3])
-	tk.MustExec("select * from t where a > 10")
-	tk.MustQuery("select @@last_plan_from_binding").Check(testkit.Rows("1"))
-
 	tk.MustExec("set binding disabled for select * from t where a > 10")
 	rows = tk.MustQuery("show global bindings").Rows()
 	require.Len(t, rows, 1)
@@ -269,10 +261,11 @@ func TestSetBindingStatusWithoutBindingInCache(t *testing.T) {
 	tk.MustQuery("show global bindings").Check(testkit.Rows())
 
 	// Simulate creating bindings on other machines
+	_, sqlDigest := parser.NormalizeDigestForBinding("select * from `test` . `t` where `a` > ?")
 	tk.MustExec("insert into mysql.bind_info values('select * from `test` . `t` where `a` > ?', 'SELECT /*+ USE_INDEX(`t` `idx_a`)*/ * FROM `test`.`t` WHERE `a` > 10', 'test', 'deleted', '2000-01-01 09:00:00', '2000-01-01 09:00:00', '', '','" +
-		bindinfo.Manual + "', '', '')")
+		bindinfo.Manual + "', '" + sqlDigest.String() + "', '')")
 	tk.MustExec("insert into mysql.bind_info values('select * from `test` . `t` where `a` > ?', 'SELECT /*+ USE_INDEX(`t` `idx_a`)*/ * FROM `test`.`t` WHERE `a` > 10', 'test', 'enabled', '2000-01-02 09:00:00', '2000-01-02 09:00:00', '', '','" +
-		bindinfo.Manual + "', '', '')")
+		bindinfo.Manual + "', '" + sqlDigest.String() + "', '')")
 	dom.BindHandle().Clear()
 	tk.MustExec("set binding disabled for select * from t where a > 10")
 	tk.MustExec("admin reload bindings")
@@ -285,9 +278,9 @@ func TestSetBindingStatusWithoutBindingInCache(t *testing.T) {
 
 	// Simulate creating bindings on other machines
 	tk.MustExec("insert into mysql.bind_info values('select * from `test` . `t` where `a` > ?', 'SELECT * FROM `test`.`t` WHERE `a` > 10', 'test', 'deleted', '2000-01-01 09:00:00', '2000-01-01 09:00:00', '', '','" +
-		bindinfo.Manual + "', '', '')")
+		bindinfo.Manual + "', '" + sqlDigest.String() + "', '')")
 	tk.MustExec("insert into mysql.bind_info values('select * from `test` . `t` where `a` > ?', 'SELECT * FROM `test`.`t` WHERE `a` > 10', 'test', 'disabled', '2000-01-02 09:00:00', '2000-01-02 09:00:00', '', '','" +
-		bindinfo.Manual + "', '', '')")
+		bindinfo.Manual + "', '" + sqlDigest.String() + "', '')")
 	dom.BindHandle().Clear()
 	tk.MustExec("set binding enabled for select * from t where a > 10")
 	tk.MustExec("admin reload bindings")

pkg/bindinfo/session_handle.go

@@ -95,45 +95,13 @@ func (h *sessionBindingHandle) CreateSessionBinding(sctx sessionctx.Context, rec
 	return nil
 }
 
-// dropSessionBinding drops a BindRecord in the cache.
-func (h *sessionBindingHandle) dropSessionBinding(originalSQL, db string, binding *Binding) error {
-	db = strings.ToLower(db)
-	sqlDigest := parser.DigestNormalized(originalSQL).String()
-	oldRecord := h.GetSessionBinding(sqlDigest)
-	var newRecord *BindRecord
-	record := &BindRecord{OriginalSQL: originalSQL, Db: db}
-	if binding != nil {
-		record.Bindings = append(record.Bindings, *binding)
-	}
-	if oldRecord != nil {
-		newRecord = oldRecord.remove(record)
-	} else {
-		newRecord = record
-	}
-	err := h.ch.SetBinding(sqlDigest, newRecord)
-	if err != nil {
-		// Should never reach here, just return an error for safety
-		return err
-	}
-	updateMetrics(metrics.ScopeSession, oldRecord, newRecord, false)
-	return nil
-}
-
 // DropSessionBinding drop BindRecord in the cache.
 func (h *sessionBindingHandle) DropSessionBinding(sqlDigest string) error {
 	if sqlDigest == "" {
 		return errors.New("sql digest is empty")
 	}
-	oldRecord := h.GetSessionBinding(sqlDigest)
-	if oldRecord == nil {
-		return errors.Errorf("can't find any binding for '%s'", sqlDigest)
-	}
-	return h.dropSessionBinding(oldRecord.OriginalSQL, strings.ToLower(oldRecord.Db), nil)
-}
-
-// GetSessionBinding return all BindMeta corresponding to sqlDigest.
-func (h *sessionBindingHandle) GetSessionBinding(sqlDigest string) *BindRecord {
-	return h.ch.GetBinding(sqlDigest)
+	h.ch.RemoveBinding(sqlDigest)
+	return nil
 }
 
 // MatchSessionBinding returns the matched binding for this statement.

pkg/bindinfo/session_handle_test.go

@@ -152,16 +152,7 @@ func TestSessionBinding(t *testing.T) {
 		require.NoError(t, err)
 		bindData, err = handle.MatchSessionBinding("test", stmt)
 		require.NoError(t, err)
-		require.NotNil(t, bindData)
-		require.Equal(t, testSQL.originSQL, bindData.OriginalSQL)
-		require.Len(t, bindData.Bindings, 0)
-
-		err = metrics.BindTotalGauge.WithLabelValues(metrics.ScopeSession, bindinfo.Enabled).Write(pb)
-		require.NoError(t, err)
-		require.Equal(t, float64(0), pb.GetGauge().GetValue())
-		err = metrics.BindMemoryUsage.WithLabelValues(metrics.ScopeSession, bindinfo.Enabled).Write(pb)
-		require.NoError(t, err)
-		require.Equal(t, float64(0), pb.GetGauge().GetValue())
+		require.Nil(t, bindData) // dropped
 	}
 }
 
@@ -214,8 +205,9 @@ func TestBaselineDBLowerCase(t *testing.T) {
 	internal.UtilCleanBindingEnv(tk, dom)
 
 	// Simulate existing bindings with upper case default_db.
+	_, sqlDigest := parser.NormalizeDigestForBinding("select * from `spm` . `t`")
 	tk.MustExec("insert into mysql.bind_info values('select * from `spm` . `t`', 'select * from `spm` . `t`', 'SPM', 'enabled', '2000-01-01 09:00:00', '2000-01-01 09:00:00', '', '','" +
-		bindinfo.Manual + "', '', '')")
+		bindinfo.Manual + "', '" + sqlDigest.String() + "', '')")
 	tk.MustQuery("select original_sql, default_db from mysql.bind_info where original_sql = 'select * from `spm` . `t`'").Check(testkit.Rows(
 		"select * from `spm` . `t` SPM",
 	))
@@ -233,7 +225,7 @@ func TestBaselineDBLowerCase(t *testing.T) {
 	internal.UtilCleanBindingEnv(tk, dom)
 	// Simulate existing bindings with upper case default_db.
 	tk.MustExec("insert into mysql.bind_info values('select * from `spm` . `t`', 'select * from `spm` . `t`', 'SPM', 'enabled', '2000-01-01 09:00:00', '2000-01-01 09:00:00', '', '','" +
-		bindinfo.Manual + "', '', '')")
+		bindinfo.Manual + "', '" + sqlDigest.String() + "', '')")
 	tk.MustQuery("select original_sql, default_db from mysql.bind_info where original_sql = 'select * from `spm` . `t`'").Check(testkit.Rows(
 		"select * from `spm` . `t` SPM",
 	))

pkg/bindinfo/tests/bind_test.go

@@ -840,7 +840,6 @@ func TestDropBindBySQLDigest(t *testing.T) {
 	}
 
 	// exception cases
-	tk.MustGetErrMsg(fmt.Sprintf("drop binding for sql digest '%s'", "1"), "can't find any binding for '1'")
 	tk.MustGetErrMsg(fmt.Sprintf("drop binding for sql digest '%s'", ""), "sql digest is empty")
 }
 
@@ -861,6 +860,5 @@ func TestJoinOrderHintWithBinding(t *testing.T) {
 	res := tk.MustQuery("show global bindings").Rows()
 	require.Equal(t, res[0][0], "select * from ( `test` . `t1` join `test` . `t2` on `t1` . `a` = `t2` . `a` ) left join `test` . `t3` on `t2` . `b` = `t3` . `b`")
 
-	// TODO(hawkingrei,qw4990)：Here is a situation of inconsistent behavior between 7.5 and master.
-	// tk.MustExec("drop global binding for select * from t1 join t2 on t1.a=t2.a join t3 on t2.b=t3.b")
+	tk.MustExec("drop global binding for select * from t1 join t2 on t1.a=t2.a join t3 on t2.b=t3.b")
 }

pkg/executor/bind.go

@@ -21,6 +21,7 @@ import (
 	"github.com/pingcap/tidb/pkg/bindinfo"
 	"github.com/pingcap/tidb/pkg/domain"
 	"github.com/pingcap/tidb/pkg/executor/internal/exec"
+	"github.com/pingcap/tidb/pkg/parser"
 	"github.com/pingcap/tidb/pkg/parser/ast"
 	plannercore "github.com/pingcap/tidb/pkg/planner/core"
 	"github.com/pingcap/tidb/pkg/util/chunk"
@@ -99,15 +100,8 @@ func (e *SQLBindExec) dropSQLBindByDigest() error {
 }
 
 func (e *SQLBindExec) setBindingStatus() error {
-	var bindInfo *bindinfo.Binding
-	if e.bindSQL != "" {
-		bindInfo = &bindinfo.Binding{
-			BindSQL:   e.bindSQL,
-			Charset:   e.charset,
-			Collation: e.collation,
-		}
-	}
-	ok, err := domain.GetDomain(e.Ctx()).BindHandle().SetGlobalBindingStatus(e.normdOrigSQL, bindInfo, e.newStatus)
+	_, sqlDigest := parser.NormalizeDigestForBinding(e.normdOrigSQL)
+	ok, err := domain.GetDomain(e.Ctx()).BindHandle().SetGlobalBindingStatus(e.newStatus, sqlDigest.String())
 	if err == nil && !ok {
 		warningMess := errors.NewNoStackError("There are no bindings can be set the status. Please check the SQL text")
 		e.Ctx().GetSessionVars().StmtCtx.AppendWarning(warningMess)
@@ -116,7 +110,7 @@ func (e *SQLBindExec) setBindingStatus() error {
 }
 
 func (e *SQLBindExec) setBindingStatusByDigest() error {
-	ok, err := domain.GetDomain(e.Ctx()).BindHandle().SetGlobalBindingStatusByDigest(e.newStatus, e.sqlDigest)
+	ok, err := domain.GetDomain(e.Ctx()).BindHandle().SetGlobalBindingStatus(e.newStatus, e.sqlDigest)
 	if err == nil && !ok {
 		warningMess := errors.NewNoStackError("There are no bindings can be set the status. Please check the SQL text")
 		e.Ctx().GetSessionVars().StmtCtx.AppendWarning(warningMess)

pkg/infoschema/test/clustertablestest/cluster_tables_test.go

@@ -1328,7 +1328,6 @@ func TestSetBindingStatusBySQLDigest(t *testing.T) {
 	tk.MustExec(fmt.Sprintf("set binding enabled for sql digest '%s'", sqlDigest[0][9]))
 	tk.MustExec(sql)
 	tk.MustQuery("select @@last_plan_from_binding").Check(testkit.Rows("1"))
-	tk.MustGetErrMsg("set binding enabled for sql digest '2'", "can't find any binding for '2'")
 	tk.MustGetErrMsg("set binding enabled for sql digest ''", "sql digest is empty")
 	tk.MustGetErrMsg("set binding disabled for sql digest ''", "sql digest is empty")
 }