From 180e4d5163cc2d9701409d8f6aaaf019a6a4e926 Mon Sep 17 00:00:00 2001
From: Kenan Yao <cauchy1992@gmail.com>
Date: Thu, 3 Dec 2020 15:12:04 +0800
Subject: [PATCH] bindinfo: physically delete previous binding when recreating
 a binding (#21349)

---
 bindinfo/bind_test.go | 29 +++++++++++++++++++++++++++++
 bindinfo/handle.go    | 22 ++++++++++++++--------
 2 files changed, 43 insertions(+), 8 deletions(-)

diff --git a/bindinfo/bind_test.go b/bindinfo/bind_test.go
index 7d9dee559322b..e85ac5fba683d 100644
--- a/bindinfo/bind_test.go
+++ b/bindinfo/bind_test.go
@@ -1710,6 +1710,35 @@ func (s *testSuite) TestIssue19836(c *C) {
 	tk.MustQuery("explain for connection " + strconv.FormatUint(tk.Se.ShowProcess().ID, 10)).Check(explainResult)
 }
 
+func (s *testSuite) TestReCreateBind(c *C) {
+	tk := testkit.NewTestKit(c, s.store)
+	s.cleanBindingEnv(tk)
+	tk.MustExec("use test")
+	tk.MustExec("drop table if exists t")
+	tk.MustExec("create table t(a int, b int, index idx(a))")
+
+	tk.MustQuery("select * from mysql.bind_info").Check(testkit.Rows())
+	tk.MustQuery("show global bindings").Check(testkit.Rows())
+
+	tk.MustExec("create global binding for select * from t using select * from t")
+	tk.MustQuery("select original_sql, status from mysql.bind_info").Check(testkit.Rows(
+		"select * from t using",
+	))
+	rows := tk.MustQuery("show global bindings").Rows()
+	c.Assert(len(rows), Equals, 1)
+	c.Assert(rows[0][0], Equals, "select * from t")
+	c.Assert(rows[0][3], Equals, "using")
+
+	tk.MustExec("create global binding for select * from t using select * from t")
+	tk.MustQuery("select original_sql, status from mysql.bind_info").Check(testkit.Rows(
+		"select * from t using",
+	))
+	rows = tk.MustQuery("show global bindings").Rows()
+	c.Assert(len(rows), Equals, 1)
+	c.Assert(rows[0][0], Equals, "select * from t")
+	c.Assert(rows[0][3], Equals, "using")
+}
+
 func (s *testSuite) TestDMLIndexHintBind(c *C) {
 	tk := testkit.NewTestKit(c, s.store)
 	s.cleanBindingEnv(tk)
diff --git a/bindinfo/handle.go b/bindinfo/handle.go
index cf7e32a29406e..fb5f529e92adc 100644
--- a/bindinfo/handle.go
+++ b/bindinfo/handle.go
@@ -213,17 +213,22 @@ func (h *BindHandle) CreateBindRecord(sctx sessionctx.Context, record *BindRecor
 		h.bindInfo.Unlock()
 	}()
 
-	txn, err1 := h.sctx.Context.Txn(true)
-	if err1 != nil {
-		return err1
+	var txn kv.Transaction
+	txn, err = h.sctx.Context.Txn(true)
+	if err != nil {
+		return err
 	}
 	now := types.NewTime(types.FromGoTime(oracle.GetTimeFromTS(txn.StartTS())), mysql.TypeTimestamp, 3)
 
 	if oldRecord != nil {
 		for _, binding := range oldRecord.Bindings {
-			_, err1 = exec.ExecuteInternal(context.TODO(), h.logicalDeleteBindInfoSQL(record.OriginalSQL, record.Db, now, binding.BindSQL))
+			// Binding recreation should physically delete previous bindings, since marking them as deleted may
+			// cause unexpected binding caches if there are concurrent CREATE BINDING on multiple tidb instances,
+			// because the record with `using` status is not guaranteed to have larger update_time than those records
+			// with `deleted` status.
+			_, err = exec.ExecuteInternal(context.TODO(), h.deleteBindInfoSQL(record.OriginalSQL, record.Db, binding.BindSQL))
 			if err != nil {
-				return err1
+				return err
 			}
 		}
 	}
@@ -290,9 +295,10 @@ func (h *BindHandle) AddBindRecord(sctx sessionctx.Context, record *BindRecord)
 		h.bindInfo.Unlock()
 	}()
 
-	txn, err1 := h.sctx.Context.Txn(true)
-	if err1 != nil {
-		return err1
+	var txn kv.Transaction
+	txn, err = h.sctx.Context.Txn(true)
+	if err != nil {
+		return err
 	}
 
 	if duplicateBinding != nil {