wip updates, can't add index due to system config span

pkg/ccl/streamingccl/streamclient/random_stream_client.go

@@ -304,8 +304,8 @@ func (m *randomStreamClient) getDescriptorAndNamespaceKVForTableID(
 		tableID,
 		fmt.Sprintf(RandomStreamSchemaPlaceholder, tableName),
 		catpb.NewBasePrivilegeDescriptor(username.RootUserName()),
-		nil,
-		nil,
+		nil, /* txn */
+		nil, /* collection */
 	)
 	if err != nil {
 		return nil, nil, err

pkg/security/username/username.go

@@ -74,6 +74,9 @@ func (s SQLUsername) IsNodeUser() bool { return s.u == NodeUser }
 // RootUser is the default cluster administrator.
 const RootUser = "root"
 
+// RootUserID is the ID for RootUser.
+const RootUserID = 1
+
 // RootUserName is the SQLUsername for RootUser.
 func RootUserName() SQLUsername { return SQLUsername{RootUser} }
 
@@ -83,6 +86,9 @@ func (s SQLUsername) IsRootUser() bool { return s.u == RootUser }
 // AdminRole is the default (and non-droppable) role with superuser privileges.
 const AdminRole = "admin"
 
+// AdminRoleID is the ID for admin.
+const AdminRoleID = 2
+
 // AdminRoleName is the SQLUsername for AdminRole.
 func AdminRoleName() SQLUsername { return SQLUsername{AdminRole} }
 

pkg/sql/catalog/systemschema/system.go

@@ -55,14 +55,16 @@ CREATE TABLE system.descriptor (
   CONSTRAINT "primary" PRIMARY KEY (id)
 );`
 
+	// UsersTableSchema represents the system.users table.
+	// Note: 48 is the ID of the system.role_id_seq sequence.
 	UsersTableSchema = `
 CREATE TABLE system.users (
   username         STRING,
   "hashedPassword" BYTES,
   "isRole"         BOOL NOT NULL DEFAULT false,
-	user_id          OID NOT NULL DEFAULT oid(nextval(48:::OID)),
+  user_id          OID NOT NULL DEFAULT oid(nextval(48:::OID)), 
   CONSTRAINT "primary" PRIMARY KEY (username),
-	INDEX (user_id)
+  UNIQUE INDEX (user_id)
 );`
 
 	RoleOptionsTableSchema = `
@@ -104,6 +106,8 @@ CREATE TABLE system.tenants (
 	FAMILY "primary" (id, active, info)
 );`
 
+	// RoleIDSequenceSchema starts at 100 so we have reserved IDs for special
+	// roles such as root and admin.
 	RoleIDSequenceSchema = `
 CREATE SEQUENCE system.role_id_seq START 100;`
 )
@@ -877,10 +881,10 @@ var (
 			pk("id"),
 		))
 
-	falseBoolString  = "false"
-	trueBoolString   = "true"
-	zeroIntString    = "0:::INT8"
-	genNextOIDString = "oid(nextval(48:::OID))"
+	falseBoolString      = "false"
+	trueBoolString       = "true"
+	zeroIntString        = "0:::INT8"
+	genNextRoleOIDString = "oid(nextval(48:::OID))"
 
 	// UsersTable is the descriptor for the users table.
 	UsersTable = registerSystemTable(
@@ -892,7 +896,7 @@ var (
 				{Name: "username", ID: 1, Type: types.String},
 				{Name: "hashedPassword", ID: 2, Type: types.Bytes, Nullable: true},
 				{Name: "isRole", ID: 3, Type: types.Bool, DefaultExpr: &falseBoolString},
-				{Name: "user_id", ID: 4, Type: types.Oid, DefaultExpr: &genNextOIDString, UsesSequenceIds: []descpb.ID{keys.RoleIDSequenceID}},
+				{Name: "user_id", ID: 4, Type: types.Oid, DefaultExpr: &genNextRoleOIDString, UsesSequenceIds: []descpb.ID{keys.RoleIDSequenceID}},
 			},
 			[]descpb.ColumnFamilyDescriptor{
 				{Name: "primary", ID: 0, ColumnNames: []string{"username"}, ColumnIDs: singleID1},
@@ -909,6 +913,7 @@ var (
 				KeyColumnIDs:        []descpb.ColumnID{4},
 				KeySuffixColumnIDs:  []descpb.ColumnID{1},
 				Version:             3,
+				Unique:              true,
 			},
 		))
 
@@ -1032,8 +1037,8 @@ var (
 		func(tbl *descpb.TableDescriptor) {
 			opts := &descpb.TableDescriptor_SequenceOpts{
 				Increment: 1,
-				MinValue:  1,
-				MaxValue:  math.MaxInt64,
+				MinValue:  100,
+				MaxValue:  math.MaxInt32,
 				Start:     100,
 				CacheSize: 1,
 			}

pkg/upgrade/upgrades/role_id_migration_test.go

@@ -115,6 +115,8 @@ func runTestRoleIDMigration(t *testing.T, numUsers int) {
 		{"testuser0", "NULL", "false", "101"},
 		{"testuser_last", "NULL", "false", fmt.Sprint(101 + numUsers)},
 	})
+
+	tdb.CheckQueryResults(t, `SHOW CREATE TABLE system.users`, [][]string{{systemschema.UsersTableSchema}})
 }
 
 func TestRoleIDMigration1User(t *testing.T) {

pkg/upgrade/upgrades/system_users_role_id_migration.go

@@ -12,11 +12,13 @@ package upgrades
 
 import (
 	"context"
+	"fmt"
 	"strings"
 
 	"github.com/cockroachdb/cockroach/pkg/clusterversion"
 	"github.com/cockroachdb/cockroach/pkg/jobs"
 	"github.com/cockroachdb/cockroach/pkg/keys"
+	"github.com/cockroachdb/cockroach/pkg/security/username"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/systemschema"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
@@ -33,7 +35,8 @@ import (
 // default expression.
 // It results in: unimplemented: cannot evaluate scalar expressions
 // containing sequence operations in this context.
-
+// The column family is specified such that the bootstrapped table
+// definition and the migration end up with the same table state.
 const addUserIDColumn = `
 ALTER TABLE system.users
 ADD COLUMN IF NOT EXISTS "user_id" OID CREATE FAMILY "fam_4_user_id" 
@@ -47,6 +50,10 @@ const updateUserIDColumnSetNotNull = `
 ALTER TABLE system.users ALTER COLUMN "user_id" SET NOT NULL
 `
 
+const addUserIDIndex = `
+CREATE UNIQUE INDEX users_user_id_idx ON system.users ("user_id" ASC)
+`
+
 func alterSystemUsersAddUserIDColumn(
 	ctx context.Context, cs clusterversion.ClusterVersion, d upgrade.TenantDeps, _ *jobs.Job,
 ) error {
@@ -79,13 +86,13 @@ func alterSystemUsersAddUserIDColumn(
 		}
 	}
 
-	const upsertRootStmt = `
-		       UPSERT INTO system.users (username, "hashedPassword", "isRole", "user_id") VALUES ('root', '', false, 1)
-		       `
+	var upsertRootStmt = fmt.Sprintf(`
+		       UPSERT INTO system.users (username, "hashedPassword", "isRole", "user_id") VALUES ('%s', '', false, %d)
+		       `, username.RootUser, username.RootUserID)
 
-	const upsertAdminStmt = `
-		       UPSERT INTO system.users (username, "hashedPassword", "isRole", "user_id") VALUES ('admin', '', true,  2)
-		       `
+	var upsertAdminStmt = fmt.Sprintf(`
+		       UPSERT INTO system.users (username, "hashedPassword", "isRole", "user_id") VALUES ('%s', '', true,  %d)
+		       `, username.AdminRole, username.AdminRoleID)
 
 	const updateSequenceValues = `
 					UPDATE system.users SET user_id = nextval(48:::OID) WHERE user_id IS NULL`
@@ -107,22 +114,34 @@ func alterSystemUsersAddUserIDColumn(
 		return err
 	}
 
-	op := operation{
-		name:       "alter-system-users-user-id-column-not-null",
-		schemaList: []string{"user_id"},
-		query:      updateUserIDColumnSetNotNull,
-		schemaExistsFn: func(storedTable, _ catalog.TableDescriptor, colName string) (bool, error) {
-			storedCol, err := storedTable.FindColumnWithName(tree.Name(colName))
-			if err != nil {
-				if strings.Contains(err.Error(), "does not exist") {
-					return false, nil
+	for _, op := range []operation{
+		{
+			name:       "alter-system-users-user-id-column-not-null",
+			schemaList: []string{"user_id"},
+			query:      updateUserIDColumnSetNotNull,
+			schemaExistsFn: func(storedTable, _ catalog.TableDescriptor, colName string) (bool, error) {
+				storedCol, err := storedTable.FindColumnWithName(tree.Name(colName))
+				if err != nil {
+					if strings.Contains(err.Error(), "does not exist") {
+						return false, nil
+					}
+					return false, err
 				}
-				return false, err
-			}
 
-			return !storedCol.IsNullable(), nil
+				return !storedCol.IsNullable(), nil
+			},
+		},
+		{
+			name:           "alter-system-users-add-index",
+			schemaList:     []string{"users_user_id_idx"},
+			query:          addUserIDIndex,
+			schemaExistsFn: hasIndex,
 		},
+	} {
+		if err := migrateTable(ctx, cs, d, op, keys.UsersTableID, systemschema.UsersTable); err != nil {
+			return err
+		}
 	}
 
-	return migrateTable(ctx, cs, d, op, keys.UsersTableID, systemschema.UsersTable)
+	return nil
 }