sql: add role_id and member_id columns to system.role_members table

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 <host>:<port>. If no port is specified, 4317 will be used.
 trace.span_registry.enabled	boolean	true	if set, ongoing traces can be seen at https://<ui>/#/debug/tracez
 trace.zipkin.collector	string		the address of a Zipkin instance to receive traces, as <host>:<port>. If no port is specified, 9411 will be used.
-version	version	1000022.2-8	set the active cluster version in the format '<major>.<minor>'
+version	version	1000022.2-14	set the active cluster version in the format '<major>.<minor>'

docs/generated/settings/settings.html

@@ -233,6 +233,6 @@
 <tr><td><code>trace.opentelemetry.collector</code></td><td>string</td><td><code></code></td><td>address of an OpenTelemetry trace collector to receive traces using the otel gRPC protocol, as <host>:<port>. If no port is specified, 4317 will be used.</td></tr>
 <tr><td><code>trace.span_registry.enabled</code></td><td>boolean</td><td><code>true</code></td><td>if set, ongoing traces can be seen at https://<ui>/#/debug/tracez</td></tr>
 <tr><td><code>trace.zipkin.collector</code></td><td>string</td><td><code></code></td><td>the address of a Zipkin instance to receive traces, as <host>:<port>. If no port is specified, 9411 will be used.</td></tr>
-<tr><td><code>version</code></td><td>version</td><td><code>1000022.2-8</code></td><td>set the active cluster version in the format '<major>.<minor>'</td></tr>
+<tr><td><code>version</code></td><td>version</td><td><code>1000022.2-14</code></td><td>set the active cluster version in the format '<major>.<minor>'</td></tr>
 </tbody>
 </table>

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.

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.

pkg/sql/grant_role.go

@@ -14,6 +14,8 @@ import (
 	"context"
 	"strings"
 
+	"github.com/cockroachdb/cockroach/pkg/clusterversion"
+	"github.com/cockroachdb/cockroach/pkg/kv"
 	"github.com/cockroachdb/cockroach/pkg/security/username"
 	"github.com/cockroachdb/cockroach/pkg/sql/decodeusername"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
@@ -22,6 +24,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
 	"github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry"
+	"github.com/cockroachdb/cockroach/pkg/sql/sqlutil"
 	"github.com/cockroachdb/cockroach/pkg/util/log/eventpb"
 	"github.com/cockroachdb/cockroach/pkg/util/tracing"
 	"github.com/cockroachdb/errors"
@@ -167,9 +170,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,21 +186,61 @@ 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 {
-		for _, m := range n.members {
-			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,
-			)
-			if err != nil {
+		qargs[0] = r.Normalized()
+		if roleMembersHasIDs {
+			var idRow tree.Datums
+			if err := params.p.WithInternalExecutor(params.ctx, func(ctx context.Context, txn *kv.Txn, ie sqlutil.InternalExecutor) error {
+				var err error
+				idRow, err = ie.QueryRowEx(
+					ctx, "get-user-id", txn,
+					sessiondata.NodeUserSessionDataOverride,
+					`SELECT user_id FROM system.users WHERE username = $1`, r.Normalized(),
+				)
+				return err
+			}); err != nil {
 				return err
 			}
+			qargs[3] = tree.MustBeDOid(idRow[0])
+		}
+		for _, m := range n.members {
+			qargs[1] = m.Normalized()
+			if roleMembersHasIDs {
+				var idRow tree.Datums
+				if err := params.p.WithInternalExecutor(params.ctx, func(ctx context.Context, txn *kv.Txn, ie sqlutil.InternalExecutor) error {
+					var err error
+					idRow, err = ie.QueryRowEx(
+						ctx, "get-user-id", txn,
+						sessiondata.NodeUserSessionDataOverride,
+						`SELECT user_id FROM system.users WHERE username = $1`, m.Normalized(),
+					)
+					return err
+				}); err != nil {
+					return err
+				}
+				qargs[4] = tree.MustBeDOid(idRow[0])
+			}
 
-			rowsAffected += affected
+			memberStmtRowsAffected := 0
+			if err := params.p.WithInternalExecutor(params.ctx, func(ctx context.Context, txn *kv.Txn, ie sqlutil.InternalExecutor) error {
+				var err error
+				memberStmtRowsAffected, err = ie.ExecEx(
+					ctx, opName, txn,
+					sessiondata.InternalExecutorOverride{User: username.RootUserName()},
+					memberStmt, qargs...,
+				)
+				return err
+			}); err != nil {
+				return err
+			}
+			rowsAffected += memberStmtRowsAffected
 		}
 	}
 

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 {

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",