Skip to content

Commit

Permalink
address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
Reminiscent committed Mar 16, 2022
1 parent f7934c2 commit c8f0977
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 18 deletions.
23 changes: 11 additions & 12 deletions bindinfo/handle.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ package bindinfo

import (
"context"
"errors"
"fmt"
"runtime"
"strconv"
Expand Down Expand Up @@ -410,8 +409,7 @@ func (h *BindHandle) DropBindRecord(originalSQL, db string, binding *Binding) (e
}

// SetBindRecordStatus set a BindRecord's status to the storage and bind cache.
func (h *BindHandle) SetBindRecordStatus(originalSQL, db string, binding *Binding, newStatus string) (err error) {
db = strings.ToLower(db)
func (h *BindHandle) SetBindRecordStatus(originalSQL string, binding *Binding, newStatus string) (ok bool, err error) {
h.bindInfo.Lock()
h.sctx.Lock()
defer func() {
Expand All @@ -421,7 +419,7 @@ func (h *BindHandle) SetBindRecordStatus(originalSQL, db string, binding *Bindin
exec, _ := h.sctx.Context.(sqlexec.SQLExecutor)
_, err = exec.ExecuteInternal(context.TODO(), "BEGIN PESSIMISTIC")
if err != nil {
return err
return
}
var (
updateTs types.Time
Expand Down Expand Up @@ -450,35 +448,36 @@ func (h *BindHandle) SetBindRecordStatus(originalSQL, db string, binding *Bindin
return
}
if affectRows == 0 {
err = errors.New("There are no bindings can be set the status. Please check the SQL text.")
return
}

record := &BindRecord{OriginalSQL: originalSQL, Db: db}
// The set binding status operation is success.
ok = true
record := &BindRecord{OriginalSQL: originalSQL}
sqlDigest := parser.DigestNormalized(record.OriginalSQL)
oldRecord := h.GetBindRecord(sqlDigest.String(), originalSQL, db)
setBindingStatusSucc := false
oldRecord := h.GetBindRecord(sqlDigest.String(), originalSQL, "")
setBindingStatusInCacheSucc := false
if oldRecord != nil && len(oldRecord.Bindings) > 0 {
record.Bindings = make([]Binding, len(oldRecord.Bindings))
copy(record.Bindings, oldRecord.Bindings)
for ind, oldBinding := range record.Bindings {
if oldBinding.Status == oldStatus0 || oldBinding.Status == oldStatus1 {
if binding == nil || (binding != nil && oldBinding.isSame(binding)) {
setBindingStatusSucc = true
setBindingStatusInCacheSucc = true
record.Bindings[ind].Status = newStatus
record.Bindings[ind].UpdateTime = updateTs
}
}
}
}
if setBindingStatusSucc {
if setBindingStatusInCacheSucc {
h.setBindRecord(sqlDigest.String(), record)
}
}()

// Lock mysql.bind_info to synchronize with SetBindingStatus on other tidb instances.
if err = h.lockBindInfoTable(); err != nil {
return err
return
}

updateTs = types.NewTime(types.FromGoTime(time.Now()), mysql.TypeTimestamp, 3)
Expand All @@ -492,7 +491,7 @@ func (h *BindHandle) SetBindRecordStatus(originalSQL, db string, binding *Bindin
newStatus, updateTsStr, originalSQL, updateTsStr, binding.BindSQL, oldStatus0, oldStatus1)
}
affectRows = int(h.sctx.Context.GetSessionVars().StmtCtx.AffectedRows())
return err
return
}

// GCBindRecord physically removes the deleted bind records in mysql.bind_info.
Expand Down
2 changes: 1 addition & 1 deletion bindinfo/handle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ func TestSetBindingStatus(t *testing.T) {
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."))
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])
Expand Down
9 changes: 4 additions & 5 deletions executor/bind.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,10 @@ func (e *SQLBindExec) setBindingStatus() error {
Collation: e.collation,
}
}
err := domain.GetDomain(e.ctx).BindHandle().SetBindRecordStatus(e.normdOrigSQL, e.db, bindInfo, e.newStatus)
if err != nil && err.Error() == "There are no bindings can be set the status. Please check the SQL text." {
// TODO: Need to uniform the error code
e.ctx.GetSessionVars().StmtCtx.AppendWarning(err)
return nil
ok, err := domain.GetDomain(e.ctx).BindHandle().SetBindRecordStatus(e.normdOrigSQL, bindInfo, e.newStatus)
if err == nil && !ok {
warningMess := errors.New("There are no bindings can be set the status. Please check the SQL text")
e.ctx.GetSessionVars().StmtCtx.AppendWarning(warningMess)
}
return err
}
Expand Down

0 comments on commit c8f0977

Please sign in to comment.