session: fix bug user privileges change after upgrade from 4.0.11 to …

…5.0 (#23403) (#23527)

session/bootstrap.go

@@ -477,11 +477,12 @@ const (
 	version66 = 66
 	// version67 restore all SQL bindings.
 	version67 = 67
-
-	// please make sure this is the largest version
-	currentBootstrapVersion = version67
 )
 
+// currentBootstrapVersion is defined as a variable, so we can modify its value for testing.
+// please make sure this is the largest version
+var currentBootstrapVersion int64 = version67
+
 var (
 	bootstrapVersion = []func(Session, int64){
 		upgradeToVer2,
@@ -640,7 +641,7 @@ func upgrade(s Session) {
 		}
 		logutil.BgLogger().Fatal("[Upgrade] upgrade failed",
 			zap.Int64("from", ver),
-			zap.Int("to", currentBootstrapVersion),
+			zap.Int64("to", currentBootstrapVersion),
 			zap.Error(err))
 	}
 }
@@ -1443,7 +1444,7 @@ func upgradeToVer64(s Session, ver int64) {
 	}
 	doReentrantDDL(s, "ALTER TABLE mysql.user ADD COLUMN `Repl_slave_priv` ENUM('N','Y') CHARACTER SET utf8 NOT NULL DEFAULT 'N' AFTER `Execute_priv`", infoschema.ErrColumnExists)
 	doReentrantDDL(s, "ALTER TABLE mysql.user ADD COLUMN `Repl_client_priv` ENUM('N','Y') CHARACTER SET utf8 NOT NULL DEFAULT 'N' AFTER `Repl_slave_priv`", infoschema.ErrColumnExists)
-	mustExecute(s, "UPDATE HIGH_PRIORITY mysql.user SET Repl_slave_priv='Y',Repl_client_priv='Y'")
+	mustExecute(s, "UPDATE HIGH_PRIORITY mysql.user SET Repl_slave_priv='Y',Repl_client_priv='Y' where Super_priv='Y'")
 }
 
 func upgradeToVer65(s Session, ver int64) {

session/bootstrap_test.go

@@ -27,6 +27,7 @@ import (
 	"github.com/pingcap/tidb/sessionctx"
 	"github.com/pingcap/tidb/sessionctx/variable"
 	"github.com/pingcap/tidb/statistics"
+	"github.com/pingcap/tidb/store/mockstore"
 	"github.com/pingcap/tidb/util/testleak"
 )
 
@@ -215,7 +216,7 @@ func (s *testBootstrapSuite) TestUpgrade(c *C) {
 	se1 := newSession(c, store, s.dbName)
 	ver, err := getBootstrapVersion(se1)
 	c.Assert(err, IsNil)
-	c.Assert(ver, Equals, int64(currentBootstrapVersion))
+	c.Assert(ver, Equals, currentBootstrapVersion)
 
 	// Do something to downgrade the store.
 	// downgrade meta bootstrap version
@@ -260,7 +261,7 @@ func (s *testBootstrapSuite) TestUpgrade(c *C) {
 
 	ver, err = getBootstrapVersion(se2)
 	c.Assert(err, IsNil)
-	c.Assert(ver, Equals, int64(currentBootstrapVersion))
+	c.Assert(ver, Equals, currentBootstrapVersion)
 
 	// Verify that 'new_collation_enabled' is false.
 	r = mustExecSQL(c, se2, fmt.Sprintf(`SELECT VARIABLE_VALUE from mysql.TiDB where VARIABLE_NAME='%s';`, tidbNewCollationEnabled))
@@ -307,7 +308,7 @@ func (s *testBootstrapSuite) TestIssue17979_1(c *C) {
 	seV4 := newSession(c, store, s.dbName)
 	ver, err = getBootstrapVersion(seV4)
 	c.Assert(err, IsNil)
-	c.Assert(ver, Equals, int64(currentBootstrapVersion))
+	c.Assert(ver, Equals, currentBootstrapVersion)
 	r := mustExecSQL(c, seV4, "select variable_value from mysql.tidb where variable_name='default_oom_action'")
 	req := r.NewChunk()
 	r.Next(ctx, req)
@@ -350,7 +351,7 @@ func (s *testBootstrapSuite) TestIssue17979_2(c *C) {
 	seV4 := newSession(c, store, s.dbName)
 	ver, err = getBootstrapVersion(seV4)
 	c.Assert(err, IsNil)
-	c.Assert(ver, Equals, int64(currentBootstrapVersion))
+	c.Assert(ver, Equals, currentBootstrapVersion)
 	r := mustExecSQL(c, seV4, "select variable_value from mysql.tidb where variable_name='default_oom_action'")
 	req := r.NewChunk()
 	r.Next(ctx, req)
@@ -387,7 +388,7 @@ func (s *testBootstrapSuite) TestIssue20900_1(c *C) {
 	seV4 := newSession(c, store, s.dbName)
 	ver, err = getBootstrapVersion(seV4)
 	c.Assert(err, IsNil)
-	c.Assert(ver, Equals, int64(currentBootstrapVersion))
+	c.Assert(ver, Equals, currentBootstrapVersion)
 	r := mustExecSQL(c, seV4, "select @@tidb_mem_quota_query")
 	req := r.NewChunk()
 	r.Next(ctx, req)
@@ -428,7 +429,7 @@ func (s *testBootstrapSuite) TestIssue20900_2(c *C) {
 	seV4 := newSession(c, store, s.dbName)
 	ver, err = getBootstrapVersion(seV4)
 	c.Assert(err, IsNil)
-	c.Assert(ver, Equals, int64(currentBootstrapVersion))
+	c.Assert(ver, Equals, currentBootstrapVersion)
 	r := mustExecSQL(c, seV4, "select @@tidb_mem_quota_query")
 	req := r.NewChunk()
 	r.Next(ctx, req)
@@ -637,7 +638,7 @@ func (s *testBootstrapSuite) TestUpgradeVersion66(c *C) {
 	seV66 := newSession(c, store, s.dbName)
 	ver, err = getBootstrapVersion(seV66)
 	c.Assert(err, IsNil)
-	c.Assert(ver, Equals, int64(currentBootstrapVersion))
+	c.Assert(ver, Equals, currentBootstrapVersion)
 	r := mustExecSQL(c, seV66, `select @@global.tidb_track_aggregate_memory_usage, @@session.tidb_track_aggregate_memory_usage`)
 	req := r.NewChunk()
 	c.Assert(r.Next(ctx, req), IsNil)
@@ -646,3 +647,36 @@ func (s *testBootstrapSuite) TestUpgradeVersion66(c *C) {
 	c.Assert(row.GetInt64(0), Equals, int64(1))
 	c.Assert(row.GetInt64(1), Equals, int64(1))
 }
+
+func (s *testBootstrapSuite) TestForIssue23387(c *C) {
+	// For issue https://github.com/pingcap/tidb/issues/23387
+	saveCurrentBootstrapVersion := currentBootstrapVersion
+	currentBootstrapVersion = version57
+
+	// Bootstrap to an old version, create a user.
+	store, err := mockstore.NewMockStore()
+	c.Assert(err, IsNil)
+	defer store.Close()
+	_, err = BootstrapSession(store)
+	// domain leaked here, Close() is not called. For testing, it's OK.
+	// If we close it and BootstrapSession again, we'll get an error "session pool is closed".
+	// The problem is caused by some the global level variable, domain map is not intended for multiple instances.
+	c.Assert(err, IsNil)
+
+	se := newSession(c, store, s.dbName)
+	mustExecSQL(c, se, "create user quatest")
+
+	// Upgrade to a newer version, check the user's privilege.
+	currentBootstrapVersion = saveCurrentBootstrapVersion
+	dom, err := BootstrapSession(store)
+	c.Assert(err, IsNil)
+	defer dom.Close()
+
+	se = newSession(c, store, s.dbName)
+	rs, err := exec(se, "show grants for quatest")
+	c.Assert(err, IsNil)
+	rows, err := ResultSetToStringSlice(context.Background(), se, rs)
+	c.Assert(err, IsNil)
+	c.Assert(len(rows), Equals, 1)
+	c.Assert(rows[0][0], Equals, "GRANT USAGE ON *.* TO 'quatest'@'%'")
+}