From c9ac1594d9526b5f933b1b7e76ffc93c2c246920 Mon Sep 17 00:00:00 2001 From: Andy Yang Date: Wed, 26 Oct 2022 13:41:15 -0400 Subject: [PATCH] sql: add role_id and member_id columns to system.role_members table --- .../settings/settings-for-tenants.txt | 2 +- docs/generated/settings/settings.html | 2 +- pkg/clusterversion/cockroach_versions.go | 23 +- pkg/sql/catalog/systemschema/system.go | 59 ++++- pkg/sql/grant_role.go | 39 ++- pkg/startupmigrations/migrations.go | 4 +- pkg/upgrade/upgrades/BUILD.bazel | 2 + .../upgrades/role_members_ids_migration.go | 123 ++++++++++ .../role_members_ids_migration_test.go | 222 ++++++++++++++++++ pkg/upgrade/upgrades/upgrades.go | 15 ++ 10 files changed, 480 insertions(+), 11 deletions(-) create mode 100644 pkg/upgrade/upgrades/role_members_ids_migration.go create mode 100644 pkg/upgrade/upgrades/role_members_ids_migration_test.go diff --git a/docs/generated/settings/settings-for-tenants.txt b/docs/generated/settings/settings-for-tenants.txt index 9c6ecd19204f..dc9254f71d9c 100644 --- a/docs/generated/settings/settings-for-tenants.txt +++ b/docs/generated/settings/settings-for-tenants.txt @@ -297,4 +297,4 @@ trace.jaeger.agent string the address of a Jaeger agent to receive traces using trace.opentelemetry.collector string address of an OpenTelemetry trace collector to receive traces using the otel gRPC protocol, as :. If no port is specified, 4317 will be used. trace.span_registry.enabled boolean true if set, ongoing traces can be seen at https:///#/debug/tracez trace.zipkin.collector string the address of a Zipkin instance to receive traces, as :. If no port is specified, 9411 will be used. -version version 1000022.2-8 set the active cluster version in the format '.' +version version 1000022.2-14 set the active cluster version in the format '.' diff --git a/docs/generated/settings/settings.html b/docs/generated/settings/settings.html index 3d68bda51a1d..364d9a068fb3 100644 --- a/docs/generated/settings/settings.html +++ b/docs/generated/settings/settings.html @@ -233,6 +233,6 @@ trace.opentelemetry.collectorstringaddress of an OpenTelemetry trace collector to receive traces using the otel gRPC protocol, as :. If no port is specified, 4317 will be used. trace.span_registry.enabledbooleantrueif set, ongoing traces can be seen at https:///#/debug/tracez trace.zipkin.collectorstringthe address of a Zipkin instance to receive traces, as :. If no port is specified, 9411 will be used. -versionversion1000022.2-8set the active cluster version in the format '.' +versionversion1000022.2-14set the active cluster version in the format '.' diff --git a/pkg/clusterversion/cockroach_versions.go b/pkg/clusterversion/cockroach_versions.go index 76e7f1fb6b5f..b4d26d220540 100644 --- a/pkg/clusterversion/cockroach_versions.go +++ b/pkg/clusterversion/cockroach_versions.go @@ -314,15 +314,22 @@ const ( // V23_1TenantNames adds a name column to system.tenants. V23_1TenantNames - // V23_1DescIDSequenceForSystemTenant migrates the descriptor ID generator // counter from a meta key to the system.descriptor_id_seq sequence for the // system tenant. V23_1DescIDSequenceForSystemTenant - // V23_1AddPartialStatisticsPredicateCol adds a column to store the predicate // for a partial statistics collection. V23_1AddPartialStatisticsPredicateCol + // V23_1RoleMembersTableHasIDColumns is the version where the role_members + // system table has columns for ids. + V23_1RoleMembersTableHasIDColumns + // V23_1RoleMembersTableHasIndexesForIDColumns is the version where the + // role_members system table has indexes for its id columns. + V23_1RoleMembersTableHasIndexesForIDColumns + // V23_1RoleMembersIDColumnsBackfilled is the version where the columns for + // ids in the role_members system table have been backfilled. + V23_1RoleMembersIDColumnsBackfilled // ************************************************* // Step (1): Add new versions here. @@ -538,6 +545,18 @@ var rawVersionsSingleton = keyedVersions{ Key: V23_1AddPartialStatisticsPredicateCol, Version: roachpb.Version{Major: 22, Minor: 2, Internal: 8}, }, + { + Key: V23_1RoleMembersTableHasIDColumns, + Version: roachpb.Version{Major: 22, Minor: 2, Internal: 10}, + }, + { + Key: V23_1RoleMembersTableHasIndexesForIDColumns, + Version: roachpb.Version{Major: 22, Minor: 2, Internal: 12}, + }, + { + Key: V23_1RoleMembersIDColumnsBackfilled, + Version: roachpb.Version{Major: 22, Minor: 2, Internal: 14}, + }, // ************************************************* // Step (2): Add new versions here. diff --git a/pkg/sql/catalog/systemschema/system.go b/pkg/sql/catalog/systemschema/system.go index a1662d55a9ba..5c523a8ddda7 100644 --- a/pkg/sql/catalog/systemschema/system.go +++ b/pkg/sql/catalog/systemschema/system.go @@ -274,12 +274,17 @@ CREATE TABLE system.locations ( // role_members stores relationships between roles (role->role and role->user). RoleMembersTableSchema = ` CREATE TABLE system.role_members ( - "role" STRING NOT NULL, - "member" STRING NOT NULL, - "isAdmin" BOOL NOT NULL, + "role" STRING NOT NULL, + "member" STRING NOT NULL, + "isAdmin" BOOL NOT NULL, + role_id OID, + member_id OID, CONSTRAINT "primary" PRIMARY KEY ("role", "member"), INDEX ("role"), - INDEX ("member") + INDEX ("member"), + INDEX (role_id), + INDEX (member_id), + UNIQUE INDEX (role_id, member_id) );` // comments stores comments(database, table, column...). @@ -1563,6 +1568,8 @@ var ( {Name: "role", ID: 1, Type: types.String}, {Name: "member", ID: 2, Type: types.String}, {Name: "isAdmin", ID: 3, Type: types.Bool}, + {Name: "role_id", ID: 4, Type: types.Oid, Nullable: true}, + {Name: "member_id", ID: 5, Type: types.Oid, Nullable: true}, }, []descpb.ColumnFamilyDescriptor{ { @@ -1578,6 +1585,20 @@ var ( ColumnIDs: []descpb.ColumnID{3}, DefaultColumnID: 3, }, + { + Name: "fam_4_role_id", + ID: 4, + ColumnNames: []string{"role_id"}, + ColumnIDs: []descpb.ColumnID{4}, + DefaultColumnID: 4, + }, + { + Name: "fam_5_member_id", + ID: 5, + ColumnNames: []string{"member_id"}, + ColumnIDs: []descpb.ColumnID{5}, + DefaultColumnID: 5, + }, }, descpb.IndexDescriptor{ Name: "primary", @@ -1607,6 +1628,36 @@ var ( KeySuffixColumnIDs: []descpb.ColumnID{1}, Version: descpb.StrictIndexColumnIDGuaranteesVersion, }, + descpb.IndexDescriptor{ + Name: "role_members_role_id_idx", + ID: 4, + Unique: false, + KeyColumnNames: []string{"role_id"}, + KeyColumnDirections: []catpb.IndexColumn_Direction{catpb.IndexColumn_ASC}, + KeyColumnIDs: []descpb.ColumnID{4}, + KeySuffixColumnIDs: []descpb.ColumnID{1, 2}, + Version: descpb.StrictIndexColumnIDGuaranteesVersion, + }, + descpb.IndexDescriptor{ + Name: "role_members_member_id_idx", + ID: 5, + Unique: false, + KeyColumnNames: []string{"member_id"}, + KeyColumnDirections: []catpb.IndexColumn_Direction{catpb.IndexColumn_ASC}, + KeyColumnIDs: []descpb.ColumnID{5}, + KeySuffixColumnIDs: []descpb.ColumnID{1, 2}, + Version: descpb.StrictIndexColumnIDGuaranteesVersion, + }, + descpb.IndexDescriptor{ + Name: "role_members_role_id_member_id_key", + ID: 6, + Unique: true, + KeyColumnNames: []string{"role_id", "member_id"}, + KeyColumnDirections: []catpb.IndexColumn_Direction{catpb.IndexColumn_ASC, catpb.IndexColumn_ASC}, + KeyColumnIDs: []descpb.ColumnID{4, 5}, + KeySuffixColumnIDs: []descpb.ColumnID{1, 2}, + Version: descpb.StrictIndexColumnIDGuaranteesVersion, + }, )) // CommentsTable is the descriptor for the comments table. diff --git a/pkg/sql/grant_role.go b/pkg/sql/grant_role.go index 808de5ec8fdc..8e698e8463c4 100644 --- a/pkg/sql/grant_role.go +++ b/pkg/sql/grant_role.go @@ -14,6 +14,7 @@ import ( "context" "strings" + "github.com/cockroachdb/cockroach/pkg/clusterversion" "github.com/cockroachdb/cockroach/pkg/security/username" "github.com/cockroachdb/cockroach/pkg/sql/decodeusername" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode" @@ -167,9 +168,13 @@ func (p *planner) GrantRoleNode(ctx context.Context, n *tree.GrantRole) (*GrantR func (n *GrantRoleNode) startExec(params runParams) error { opName := "grant-role" + roleMembersHasIDs := params.p.ExecCfg().Settings.Version.IsActive(params.ctx, clusterversion.V23_1RoleMembersTableHasIDColumns) // Add memberships. Existing memberships are allowed. // If admin option is false, we do not remove it from existing memberships. memberStmt := `INSERT INTO system.role_members ("role", "member", "isAdmin") VALUES ($1, $2, $3) ON CONFLICT ("role", "member")` + if roleMembersHasIDs { + memberStmt = `INSERT INTO system.role_members ("role", "member", "isAdmin", "role_id", "member_id") VALUES ($1, $2, $3, $4, $5) ON CONFLICT ("role", "member")` + } if n.adminOption { // admin option: true, set "isAdmin" even if the membership exists. memberStmt += ` DO UPDATE SET "isAdmin" = true` @@ -179,15 +184,47 @@ func (n *GrantRoleNode) startExec(params runParams) error { } var rowsAffected int + var qargs []interface{} + if roleMembersHasIDs { + qargs = make([]interface{}, 5) + } else { + qargs = make([]interface{}, 3) + } + qargs[2] = n.adminOption for _, r := range n.roles { + qargs[0] = r.Normalized() + if roleMembersHasIDs { + idRow, err := params.p.ExecCfg().InternalExecutor.QueryRowEx( + params.ctx, "get-user-id", params.p.Txn(), + sessiondata.NodeUserSessionDataOverride, + `SELECT user_id FROM system.users WHERE username = $1`, r.Normalized(), + ) + if err != nil { + return err + } + qargs[3] = tree.MustBeDOid(idRow[0]) + } for _, m := range n.members { + qargs[1] = m.Normalized() + if roleMembersHasIDs { + idRow, err := params.p.ExecCfg().InternalExecutor.QueryRowEx( + params.ctx, "get-user-id", params.p.Txn(), + sessiondata.NodeUserSessionDataOverride, + `SELECT user_id FROM system.users WHERE username = $1`, m.Normalized(), + ) + if err != nil { + return err + } + qargs[4] = tree.MustBeDOid(idRow[0]) + } + affected, err := params.extendedEvalCtx.ExecCfg.InternalExecutor.ExecEx( params.ctx, opName, params.p.txn, sessiondata.InternalExecutorOverride{User: username.RootUserName()}, memberStmt, - r.Normalized(), m.Normalized(), n.adminOption, + qargs..., ) if err != nil { return err diff --git a/pkg/startupmigrations/migrations.go b/pkg/startupmigrations/migrations.go index 1a4e48eb7a35..48d5451a52f7 100644 --- a/pkg/startupmigrations/migrations.go +++ b/pkg/startupmigrations/migrations.go @@ -734,10 +734,10 @@ func addAdminRole(ctx context.Context, r runner) error { func addRootToAdminRole(ctx context.Context, r runner) error { // Upsert the role membership into the table. We intentionally override any existing entry. const upsertAdminStmt = ` - UPSERT INTO system.role_members ("role", "member", "isAdmin") VALUES ($1, $2, true) + UPSERT INTO system.role_members ("role", "member", "isAdmin", role_id, member_id) VALUES ($1, $2, true, $3, $4) ` return r.execAsRootWithRetry( - ctx, "addRootToAdminRole", upsertAdminStmt, username.AdminRole, username.RootUser) + ctx, "addRootToAdminRole", upsertAdminStmt, username.AdminRole, username.RootUser, username.AdminRoleID, username.RootUserID) } func disallowPublicUserOrRole(ctx context.Context, r runner) error { diff --git a/pkg/upgrade/upgrades/BUILD.bazel b/pkg/upgrade/upgrades/BUILD.bazel index 48db9cd3a34a..ae3e15cc7d95 100644 --- a/pkg/upgrade/upgrades/BUILD.bazel +++ b/pkg/upgrade/upgrades/BUILD.bazel @@ -13,6 +13,7 @@ go_library( "fix_userfile_descriptor_corruption.go", "precondition_before_starting_an_upgrade.go", "role_id_sequence_migration.go", + "role_members_ids_migration.go", "role_options_table_migration.go", "sampled_stmt_diagnostics_requests.go", "schema_changes.go", @@ -85,6 +86,7 @@ go_test( "main_test.go", "precondition_before_starting_an_upgrade_external_test.go", "role_id_migration_test.go", + "role_members_ids_migration_test.go", "role_options_migration_test.go", "sampled_stmt_diagnostics_requests_test.go", "schema_changes_external_test.go", diff --git a/pkg/upgrade/upgrades/role_members_ids_migration.go b/pkg/upgrade/upgrades/role_members_ids_migration.go new file mode 100644 index 000000000000..6d44ebc2c812 --- /dev/null +++ b/pkg/upgrade/upgrades/role_members_ids_migration.go @@ -0,0 +1,123 @@ +// Copyright 2022 The Cockroach Authors. +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +package upgrades + +import ( + "context" + + "github.com/cockroachdb/cockroach/pkg/clusterversion" + "github.com/cockroachdb/cockroach/pkg/keys" + "github.com/cockroachdb/cockroach/pkg/sql/catalog/systemschema" + "github.com/cockroachdb/cockroach/pkg/sql/sessiondata" + "github.com/cockroachdb/cockroach/pkg/upgrade" +) + +const addIDColumnsToRoleMembersStmt = ` +ALTER TABLE system.role_members +ADD COLUMN IF NOT EXISTS role_id OID, +ADD COLUMN IF NOT EXISTS member_id OID +` + +func alterSystemRoleMembersAddIDColumns( + ctx context.Context, cs clusterversion.ClusterVersion, d upgrade.TenantDeps, +) error { + op := operation{ + name: "add-id-columns-system-role-members", + schemaList: []string{"role_id", "member_id"}, + query: addIDColumnsToRoleMembersStmt, + schemaExistsFn: columnExists, + } + return migrateTable(ctx, cs, d, op, keys.RoleMembersTableID, systemschema.RoleMembersTable) +} + +const addIndexOnRoleIDToRoleMembersStmt = ` +CREATE INDEX role_members_role_id_idx ON system.role_members (role_id ASC) +` + +const addIndexOnMemberIDToRoleMembersStmt = ` +CREATE INDEX role_members_member_id_idx ON system.role_members (member_id ASC) +` + +const addUniqueIndexOnIDsToRoleMembersStmt = ` +CREATE UNIQUE INDEX role_members_role_id_member_id_key ON system.role_members (role_id ASC, member_id ASC) +` + +func alterSystemRoleMembersAddIndexesForIDColumns( + ctx context.Context, cs clusterversion.ClusterVersion, d upgrade.TenantDeps, +) error { + for _, op := range []operation{ + { + name: "add-role-id-index-system-role-members", + schemaList: []string{"role_id"}, + query: addIndexOnRoleIDToRoleMembersStmt, + schemaExistsFn: hasIndex, + }, + { + name: "add-member-id-index-system-role-members", + schemaList: []string{"member_id"}, + query: addIndexOnMemberIDToRoleMembersStmt, + schemaExistsFn: hasIndex, + }, + { + name: "add-id-unique-index-system-role-members", + schemaList: []string{"role_id", "member_id"}, + query: addUniqueIndexOnIDsToRoleMembersStmt, + schemaExistsFn: hasIndex, + }, + } { + if err := migrateTable(ctx, cs, d, op, keys.RoleMembersTableID, systemschema.RoleMembersTable); err != nil { + return err + } + } + + return nil +} + +const backfillRoleIDColumnRoleMemberStmt = ` +UPDATE system.role_members +SET role_id = u.user_id +FROM system.users AS u +WHERE role_id IS NULL AND role = u.username +LIMIT 10000 +` + +const backfillMemberIDColumnRoleMembersStmt = ` +UPDATE system.role_members +SET member_id = u.user_id +FROM system.users AS u +WHERE member_id IS NULL AND member = u.username +LIMIT 10000 +` + +func backfillSystemRoleMembersIDColumns( + ctx context.Context, _ clusterversion.ClusterVersion, d upgrade.TenantDeps, +) error { + for _, backfillStmt := range []string{backfillRoleIDColumnRoleMemberStmt, backfillMemberIDColumnRoleMembersStmt} { + for { + rowsAffected, err := d.InternalExecutor.ExecEx(ctx, "backfill-id-columns-system-role-members", nil, + sessiondata.NodeUserSessionDataOverride, + backfillStmt, + ) + if err != nil { + return err + } + if rowsAffected == 0 { + break + } + } + } + + return nil +} + +// TODO(yang): Add a migration for making the ID columns not-null. Choosing to +// put this in a separate migration so that we can handle any BACKUP/RESTORE +// changes in a separate PR. diff --git a/pkg/upgrade/upgrades/role_members_ids_migration_test.go b/pkg/upgrade/upgrades/role_members_ids_migration_test.go new file mode 100644 index 000000000000..3280c77352fa --- /dev/null +++ b/pkg/upgrade/upgrades/role_members_ids_migration_test.go @@ -0,0 +1,222 @@ +// Copyright 2022 The Cockroach Authors. +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +package upgrades_test + +import ( + "context" + "fmt" + "math/rand" + "testing" + + "github.com/cockroachdb/cockroach/pkg/base" + "github.com/cockroachdb/cockroach/pkg/clusterversion" + "github.com/cockroachdb/cockroach/pkg/keys" + "github.com/cockroachdb/cockroach/pkg/security/username" + "github.com/cockroachdb/cockroach/pkg/server" + "github.com/cockroachdb/cockroach/pkg/settings/cluster" + "github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb" + "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" + "github.com/cockroachdb/cockroach/pkg/sql/catalog/systemschema" + "github.com/cockroachdb/cockroach/pkg/sql/privilege" + "github.com/cockroachdb/cockroach/pkg/sql/types" + "github.com/cockroachdb/cockroach/pkg/testutils/skip" + "github.com/cockroachdb/cockroach/pkg/testutils/sqlutils" + "github.com/cockroachdb/cockroach/pkg/testutils/testcluster" + "github.com/cockroachdb/cockroach/pkg/upgrade/upgrades" + "github.com/cockroachdb/cockroach/pkg/util/leaktest" + "github.com/stretchr/testify/require" +) + +func TestRoleMembersIDMigrationNoUsers(t *testing.T) { + runTestRoleMembersIDMigration(t, 0) +} + +func TestRoleMembersIDMigration10Users(t *testing.T) { + runTestRoleMembersIDMigration(t, 10) +} + +func TestRoleMembersIDMigration11000Users(t *testing.T) { + skip.UnderRace(t) + skip.UnderStress(t) + runTestRoleMembersIDMigration(t, 11000) +} + +func runTestRoleMembersIDMigration(t *testing.T, numUsers int) { + defer leaktest.AfterTest(t)() + ctx := context.Background() + + settings := cluster.MakeTestingClusterSettingsWithVersions( + clusterversion.TestingBinaryVersion, + clusterversion.ByKey(clusterversion.V23_1RoleMembersTableHasIDColumns-1), + false, + ) + + tc := testcluster.StartTestCluster(t, 1 /*nodes*/, base.TestClusterArgs{ + ServerArgs: base.TestServerArgs{ + Settings: settings, + Knobs: base.TestingKnobs{ + Server: &server.TestingKnobs{ + DisableAutomaticVersionUpgrade: make(chan struct{}), + BinaryVersionOverride: clusterversion.ByKey(clusterversion.V23_1RoleMembersTableHasIDColumns - 1), + }, + }, + }, + }) + defer tc.Stopper().Stop(ctx) + + db := tc.ServerConn(0) + defer db.Close() + tdb := sqlutils.MakeSQLRunner(db) + s := tc.Server(0) + + // Delete all rows from the system.role_members table. + tdb.Exec(t, "DELETE FROM system.role_members WHERE true") + tdb.CheckQueryResults(t, "SELECT * FROM system.role_members", [][]string{}) + + // Inject the descriptor for the system.role_members table from before the + // ID columns were added. + upgrades.InjectLegacyTable(ctx, t, s, systemschema.RoleMembersTable, getTableDescForSystemRoleMembersTableBeforeIDCols) + + // Insert row that would have been added in a legacy startup migration. + tdb.Exec(t, `INSERT INTO system.role_members ("role", "member", "isAdmin") VALUES ('admin', 'root', true)`) + tdb.CheckQueryResults(t, `SELECT * FROM system.role_members`, [][]string{ + {"admin", "root", "true"}, + }) + + // Create test users. + expectedNumRoleMembersRows := 1 + for i := 0; i < numUsers; i++ { + tdb.Exec(t, fmt.Sprintf("CREATE USER testuser%d", i)) + if i == 0 { + continue + } + // Randomly choose an earlier testuser to grant to the current testuser. + grantStmt := fmt.Sprintf("GRANT testuser%d to testuser%d", rand.Intn(i), i) + if rand.Intn(2) == 1 { + grantStmt += " WITH ADMIN OPTION" + } + tdb.Exec(t, grantStmt) + expectedNumRoleMembersRows += 1 + } + tdb.CheckQueryResults(t, "SELECT count(*) FROM system.role_members", [][]string{ + {fmt.Sprintf("%d", expectedNumRoleMembersRows)}, + }) + + // Run migrations. + _, err := tc.Conns[0].ExecContext(ctx, `SET CLUSTER SETTING version = $1`, + clusterversion.ByKey(clusterversion.V23_1RoleMembersTableHasIDColumns).String()) + require.NoError(t, err) + _, err = tc.Conns[0].ExecContext(ctx, `SET CLUSTER SETTING version = $1`, + clusterversion.ByKey(clusterversion.V23_1RoleMembersTableHasIndexesForIDColumns).String()) + require.NoError(t, err) + _, err = tc.Conns[0].ExecContext(ctx, `SET CLUSTER SETTING version = $1`, + clusterversion.ByKey(clusterversion.V23_1RoleMembersIDColumnsBackfilled).String()) + require.NoError(t, err) + + // Check some basic conditions on system.role_members after the migration is complete. + tdb.CheckQueryResults(t, "SELECT * FROM system.role_members WHERE role_id IS NULL OR member_id IS NULL", [][]string{}) + tdb.CheckQueryResults(t, "SELECT count(*) FROM system.role_members AS a JOIN system.users AS b on a.role = b.username AND a.role_id <> b.user_id", [][]string{{"0"}}) + tdb.CheckQueryResults(t, "SELECT count(*) FROM system.role_members AS a JOIN system.users AS b on a.member = b.username AND a.member_id <> b.user_id", [][]string{{"0"}}) + + tdb.CheckQueryResults(t, `SELECT * FROM system.role_members WHERE "role" = 'admin'`, [][]string{ + {"admin", "root", "true", "2", "1"}, + }) + + // Verify that the final schema matches the expected one. + expectedSchema := `CREATE TABLE public.role_members ( + "role" STRING NOT NULL, + member STRING NOT NULL, + "isAdmin" BOOL NOT NULL, + role_id OID NULL, + member_id OID NULL, + CONSTRAINT "primary" PRIMARY KEY ("role" ASC, member ASC), + INDEX role_members_role_idx ("role" ASC), + INDEX role_members_member_idx (member ASC), + INDEX role_members_role_id_idx (role_id ASC), + INDEX role_members_member_id_idx (member_id ASC), + UNIQUE INDEX role_members_role_id_member_id_key (role_id ASC, member_id ASC), + FAMILY "primary" ("role", member), + FAMILY "fam_3_isAdmin" ("isAdmin"), + FAMILY fam_4_role_id (role_id), + FAMILY fam_5_member_id (member_id) +)` + r := tc.Conns[0].QueryRow("SELECT create_statement FROM [SHOW CREATE TABLE system.role_members]") + var actualSchema string + require.NoError(t, r.Scan(&actualSchema)) + require.Equal(t, expectedSchema, actualSchema) +} + +func getTableDescForSystemRoleMembersTableBeforeIDCols() *descpb.TableDescriptor { + return &descpb.TableDescriptor{ + Name: "role_members", + ID: keys.RoleMembersTableID, + ParentID: keys.SystemDatabaseID, + UnexposedParentSchemaID: keys.PublicSchemaID, + Version: 1, + Columns: []descpb.ColumnDescriptor{ + {Name: "role", ID: 1, Type: types.String}, + {Name: "member", ID: 2, Type: types.String}, + {Name: "isAdmin", ID: 3, Type: types.Bool}, + }, + NextColumnID: 4, + Families: []descpb.ColumnFamilyDescriptor{ + { + Name: "primary", + ID: 0, + ColumnNames: []string{"role", "member"}, + ColumnIDs: []descpb.ColumnID{1, 2}, + }, + { + Name: "fam_3_isAdmin", + ID: 3, + ColumnNames: []string{"isAdmin"}, + ColumnIDs: []descpb.ColumnID{3}, + DefaultColumnID: 3, + }, + }, + NextFamilyID: 4, + PrimaryIndex: descpb.IndexDescriptor{ + Name: "primary", + ID: 1, + Unique: true, + KeyColumnNames: []string{"role", "member"}, + KeyColumnDirections: []catpb.IndexColumn_Direction{catpb.IndexColumn_ASC, catpb.IndexColumn_ASC}, + KeyColumnIDs: []descpb.ColumnID{1, 2}, + }, + Indexes: []descpb.IndexDescriptor{ + { + Name: "role_members_role_idx", + ID: 2, + Unique: false, + KeyColumnNames: []string{"role"}, + KeyColumnDirections: []catpb.IndexColumn_Direction{catpb.IndexColumn_ASC}, + KeyColumnIDs: []descpb.ColumnID{1}, + KeySuffixColumnIDs: []descpb.ColumnID{2}, + Version: descpb.StrictIndexColumnIDGuaranteesVersion, + }, + { + Name: "role_members_member_idx", + ID: 3, + Unique: false, + KeyColumnNames: []string{"member"}, + KeyColumnDirections: []catpb.IndexColumn_Direction{catpb.IndexColumn_ASC}, + KeyColumnIDs: []descpb.ColumnID{2}, + KeySuffixColumnIDs: []descpb.ColumnID{1}, + Version: descpb.StrictIndexColumnIDGuaranteesVersion, + }, + }, + NextIndexID: 4, + Privileges: catpb.NewCustomSuperuserPrivilegeDescriptor(privilege.ReadWriteData, username.NodeUserName()), + FormatVersion: descpb.InterleavedFormatVersion, + NextMutationID: 1, + NextConstraintID: 1, + } +} diff --git a/pkg/upgrade/upgrades/upgrades.go b/pkg/upgrade/upgrades/upgrades.go index 48932eff9041..d5c411417826 100644 --- a/pkg/upgrade/upgrades/upgrades.go +++ b/pkg/upgrade/upgrades/upgrades.go @@ -171,6 +171,21 @@ var upgrades = []upgrade.Upgrade{ NoPrecondition, alterSystemTableStatisticsAddPartialPredicate, ), + upgrade.NewTenantUpgrade("add role_id and member_id columns to system.role_members", + toCV(clusterversion.V23_1RoleMembersTableHasIDColumns), + NoPrecondition, + alterSystemRoleMembersAddIDColumns, + ), + upgrade.NewTenantUpgrade("add indexes to system.role_members for role_id and member_id columns", + toCV(clusterversion.V23_1RoleMembersTableHasIndexesForIDColumns), + NoPrecondition, + alterSystemRoleMembersAddIndexesForIDColumns, + ), + upgrade.NewTenantUpgrade("backfill role_id and member_id columns in system.role_members", + toCV(clusterversion.V23_1RoleMembersIDColumnsBackfilled), + NoPrecondition, + backfillSystemRoleMembersIDColumns, + ), } func init() {