From 5c7469daeb623680b3264213464e6c38fec60ee0 Mon Sep 17 00:00:00 2001 From: rebelice Date: Tue, 12 Jan 2021 15:06:31 +0800 Subject: [PATCH 1/2] session: fix case for duplicate bindings when updating bind info --- session/bootstrap.go | 73 +++++++++++++++++++++++++-------------- session/bootstrap_test.go | 5 +-- 2 files changed, 51 insertions(+), 27 deletions(-) diff --git a/session/bootstrap.go b/session/bootstrap.go index e30a83c12e5f6..0747f2bb14dae 100644 --- a/session/bootstrap.go +++ b/session/bootstrap.go @@ -22,6 +22,7 @@ import ( "encoding/hex" "flag" "fmt" + "github.com/pingcap/tidb/expression" "runtime/debug" "strconv" "strings" @@ -36,7 +37,6 @@ import ( "github.com/pingcap/tidb/config" "github.com/pingcap/tidb/ddl" "github.com/pingcap/tidb/domain" - "github.com/pingcap/tidb/expression" "github.com/pingcap/tidb/infoschema" "github.com/pingcap/tidb/planner/core" "github.com/pingcap/tidb/sessionctx/variable" @@ -1305,11 +1305,20 @@ func upgradeToVer60(s Session, ver int64) { doReentrantDDL(s, CreateStatsExtended) } +type bindInfo struct { + bindSQL string + status string + createTime types.Time + charset string + collation string + source string +} + func upgradeToVer61(s Session, ver int64) { if ver >= version61 { return } - bindMap := make(map[string]types.Time) + bindMap := make(map[string]bindInfo) h := &bindinfo.BindHandle{} var err error mustExecute(s, "BEGIN PESSIMISTIC") @@ -1324,7 +1333,11 @@ func upgradeToVer61(s Session, ver int64) { }() mustExecute(s, h.LockBindInfoSQL()) var recordSets []sqlexec.RecordSet - recordSets, err = s.ExecuteInternal(context.Background(), "SELECT original_sql, bind_sql, default_db, charset, collation, update_time from mysql.bind_info where source != 'builtin'") + recordSets, err = s.ExecuteInternal(context.Background(), + `SELECT bind_sql, default_db, status, create_time, charset, collation, source + FROM mysql.bind_info + WHERE source != 'builtin' + ORDER BY update_time DESC`) if err != nil { logutil.BgLogger().Fatal("upgradeToVer61 error", zap.Error(err)) } @@ -1345,40 +1358,50 @@ func upgradeToVer61(s Session, ver int64) { } updateBindInfo(s, iter, p, now.String(), bindMap) } + + mustExecute(s, "DELETE FROM mysql.bind_info where source != 'builtin'") + for original, bind := range bindMap { + mustExecute(s, fmt.Sprintf("INSERT INTO mysql.bind_info VALUES(%s, %s, '', %s, %s, %s, %s, %s, %s)", + expression.Quote(original), + expression.Quote(bind.bindSQL), + expression.Quote(bind.status), + expression.Quote(bind.createTime.String()), + expression.Quote(now.String()), + expression.Quote(bind.charset), + expression.Quote(bind.collation), + expression.Quote(bind.source), + )) + } } -func updateBindInfo(s Session, iter *chunk.Iterator4Chunk, p *parser.Parser, now string, bindMap map[string]types.Time) { +func updateBindInfo(s Session, iter *chunk.Iterator4Chunk, p *parser.Parser, now string, bindMap map[string]bindInfo) { for row := iter.Begin(); row != iter.End(); row = iter.Next() { - original := row.GetString(0) - bind := row.GetString(1) - db := row.GetString(2) - charset := row.GetString(3) - collation := row.GetString(4) - updateTime := row.GetTime(5) + bind := row.GetString(0) + db := row.GetString(1) + charset := row.GetString(4) + collation := row.GetString(5) stmt, err := p.ParseOneStmt(bind, charset, collation) if err != nil { logutil.BgLogger().Fatal("updateBindInfo error", zap.Error(err)) } originWithDB := parser.Normalize(utilparser.RestoreWithDefaultDB(stmt, db)) - if latest, ok := bindMap[originWithDB]; ok { - // In the following cases, duplicate originWithDB may occur + if _, ok := bindMap[originWithDB]; ok { + // The results are sorted in descending order of time. + // And in the following cases, duplicate originWithDB may occur // originalText |bindText |DB // `select * from t` |`select /*+ use_index(t, idx) */ * from t` |`test` // `select * from test.t` |`select /*+ use_index(t, idx) */ * from test.t`|`` - // If repeated, we will keep the latest binding. - if updateTime.Compare(latest) <= 0 { - continue - } - mustExecute(s, fmt.Sprintf(`DELETE FROM mysql.bind_info WHERE original_sql=%s`, expression.Quote(original))) - original = originWithDB + // Therefore, if repeated, we can skip to keep the latest binding. + continue + } + bindMap[originWithDB] = bindInfo{ + bindSQL: utilparser.RestoreWithDefaultDB(stmt, db), + status: row.GetString(2), + createTime: row.GetTime(3), + charset: charset, + collation: collation, + source: row.GetString(6), } - bindMap[originWithDB] = updateTime - bindWithDB := utilparser.RestoreWithDefaultDB(stmt, db) - mustExecute(s, fmt.Sprintf(`UPDATE mysql.bind_info SET original_sql=%s, bind_sql=%s, default_db='', update_time=%s where original_sql=%s`, - expression.Quote(originWithDB), - expression.Quote(bindWithDB), - expression.Quote(now), - expression.Quote(original))) } } diff --git a/session/bootstrap_test.go b/session/bootstrap_test.go index f7ebf09c610bf..5470082eff101 100644 --- a/session/bootstrap_test.go +++ b/session/bootstrap_test.go @@ -581,11 +581,11 @@ func (s *testBootstrapSuite) TestUpdateDuplicateBindInfo(c *C) { se := newSession(c, store, s.dbName) mustExecSQL(c, se, `insert into mysql.bind_info values('select * from t', 'select /*+ use_index(t, idx_a)*/ * from t', 'test', 'using', '2021-01-04 14:50:58.257', '2021-01-04 14:50:58.257', 'utf8', 'utf8_general_ci', 'manual')`) // The latest one. - mustExecSQL(c, se, `insert into mysql.bind_info values('select * from test.t', 'select /*+ use_index(t, idx_b)*/ * from test.t', 'test', 'using', '2021-01-04 14:50:58.257', '2021-01-09 14:50:58.257', 'utf8', 'utf8_general_ci', 'manual')`) + mustExecSQL(c, se, `insert into mysql.bind_info values('select * from test . t', 'select /*+ use_index(t, idx_b)*/ * from test.t', 'test', 'using', '2021-01-04 14:50:58.257', '2021-01-09 14:50:58.257', 'utf8', 'utf8_general_ci', 'manual')`) upgradeToVer61(se, version60) - r := mustExecSQL(c, se, `select original_sql, bind_sql, default_db, status from mysql.bind_info where source != 'builtin'`) + r := mustExecSQL(c, se, `select original_sql, bind_sql, default_db, status, create_time from mysql.bind_info where source != 'builtin'`) req := r.NewChunk() c.Assert(r.Next(ctx, req), IsNil) c.Assert(req.NumRows(), Equals, 1) @@ -594,6 +594,7 @@ func (s *testBootstrapSuite) TestUpdateDuplicateBindInfo(c *C) { c.Assert(row.GetString(1), Equals, "SELECT /*+ use_index(t idx_b)*/ * FROM test.t") c.Assert(row.GetString(2), Equals, "") c.Assert(row.GetString(3), Equals, "using") + c.Assert(row.GetTime(4).String(), Equals, "2021-01-04 14:50:58.257") c.Assert(r.Close(), IsNil) mustExecSQL(c, se, "delete from mysql.bind_info where original_sql = 'select * from test . t'") } From e2e9218d438708c9cceb887feeaafade3deee675 Mon Sep 17 00:00:00 2001 From: rebelice Date: Tue, 12 Jan 2021 15:26:59 +0800 Subject: [PATCH 2/2] fmt --- session/bootstrap.go | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/session/bootstrap.go b/session/bootstrap.go index 0747f2bb14dae..2bfb718d88c48 100644 --- a/session/bootstrap.go +++ b/session/bootstrap.go @@ -22,7 +22,6 @@ import ( "encoding/hex" "flag" "fmt" - "github.com/pingcap/tidb/expression" "runtime/debug" "strconv" "strings" @@ -37,6 +36,7 @@ import ( "github.com/pingcap/tidb/config" "github.com/pingcap/tidb/ddl" "github.com/pingcap/tidb/domain" + "github.com/pingcap/tidb/expression" "github.com/pingcap/tidb/infoschema" "github.com/pingcap/tidb/planner/core" "github.com/pingcap/tidb/sessionctx/variable" @@ -1306,12 +1306,12 @@ func upgradeToVer60(s Session, ver int64) { } type bindInfo struct { - bindSQL string - status string + bindSQL string + status string createTime types.Time - charset string - collation string - source string + charset string + collation string + source string } func upgradeToVer61(s Session, ver int64) { @@ -1356,7 +1356,7 @@ func upgradeToVer61(s Session, ver int64) { if req.NumRows() == 0 { break } - updateBindInfo(s, iter, p, now.String(), bindMap) + updateBindInfo(iter, p, bindMap) } mustExecute(s, "DELETE FROM mysql.bind_info where source != 'builtin'") @@ -1374,7 +1374,7 @@ func upgradeToVer61(s Session, ver int64) { } } -func updateBindInfo(s Session, iter *chunk.Iterator4Chunk, p *parser.Parser, now string, bindMap map[string]bindInfo) { +func updateBindInfo(iter *chunk.Iterator4Chunk, p *parser.Parser, bindMap map[string]bindInfo) { for row := iter.Begin(); row != iter.End(); row = iter.Next() { bind := row.GetString(0) db := row.GetString(1) @@ -1395,12 +1395,12 @@ func updateBindInfo(s Session, iter *chunk.Iterator4Chunk, p *parser.Parser, now continue } bindMap[originWithDB] = bindInfo{ - bindSQL: utilparser.RestoreWithDefaultDB(stmt, db), - status: row.GetString(2), + bindSQL: utilparser.RestoreWithDefaultDB(stmt, db), + status: row.GetString(2), createTime: row.GetTime(3), - charset: charset, - collation: collation, - source: row.GetString(6), + charset: charset, + collation: collation, + source: row.GetString(6), } } }