Merge #149708

149708: catalog/replication: make system.privileges a view too r=dt a=dt

Release note (enterprise change): 'SYSTEM' privileges are now also inherited in read-only mode standby PCR clusters.
Epic: CRDB-50820.

Co-authored-by: David Taylor <[email protected]>

Author: craig[bot]

pkg/sql/catalog/replication/BUILD.bazel

@@ -19,6 +19,7 @@ go_library(
         "//pkg/sql/catalog/schemadesc",
         "//pkg/sql/catalog/tabledesc",
         "//pkg/sql/catalog/typedesc",
+        "//pkg/sql/sem/catconstants",
         "//pkg/util/hlc",
         "//pkg/util/protoutil",
         "@com_github_cockroachdb_errors//:errors",

pkg/sql/catalog/replication/reader_catalog.go

@@ -21,6 +21,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/schemadesc"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/typedesc"
+	"github.com/cockroachdb/cockroach/pkg/sql/sem/catconstants"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
 	"github.com/cockroachdb/cockroach/pkg/util/protoutil"
 	"github.com/cockroachdb/errors"
@@ -64,7 +65,7 @@ func SetupOrAdvanceStandbyReaderCatalog(
 			// below.
 			descriptorsToWrite := make([]catalog.MutableDescriptor, 0, len(allExistingDescs.OrderedDescriptorIDs()))
 			if err := extracted.ForEachDescriptor(func(fromDesc catalog.Descriptor) error {
-				if !shouldSetupForReader(fromDesc.GetID(), fromDesc.GetParentID()) {
+				if !shouldSetupForReader(fromDesc.GetID(), fromDesc.GetName(), fromDesc.GetParentID()) {
 					return nil
 				}
 				// Track this descriptor was updated.
@@ -138,7 +139,7 @@ func SetupOrAdvanceStandbyReaderCatalog(
 				}
 			}
 			if err := extracted.ForEachNamespaceEntry(func(e nstree.NamespaceEntry) error {
-				if !shouldSetupForReader(e.GetID(), e.GetParentID()) {
+				if !shouldSetupForReader(e.GetID(), e.GetName(), e.GetParentID()) {
 					return nil
 				}
 				// Do not upsert entries if one already exists.
@@ -153,7 +154,7 @@ func SetupOrAdvanceStandbyReaderCatalog(
 			// Figure out which descriptors should be deleted.
 			if err := allExistingDescs.ForEachDescriptor(func(desc catalog.Descriptor) error {
 				// Skip descriptors that were updated above
-				if !shouldSetupForReader(desc.GetID(), desc.GetParentID()) ||
+				if !shouldSetupForReader(desc.GetID(), desc.GetName(), desc.GetParentID()) ||
 					descriptorsUpdated.Contains(desc.GetID()) {
 					return nil
 				}
@@ -167,7 +168,7 @@ func SetupOrAdvanceStandbyReaderCatalog(
 			if err := allExistingDescs.ForEachNamespaceEntry(func(e nstree.NamespaceEntry) error {
 				// Skip descriptors that were updated above that were
 				// not renamed.
-				if !shouldSetupForReader(e.GetID(), e.GetParentID()) ||
+				if !shouldSetupForReader(e.GetID(), e.GetName(), e.GetParentID()) ||
 					(descriptorsUpdated.Contains(e.GetID()) &&
 						!descriptorsRenamed.Contains(e.GetID())) {
 					return nil
@@ -292,15 +293,23 @@ func replicateDescriptorForReader(
 }
 
 // shouldSetupForReader determines if a descriptor should be setup
-// access via external row data.
-func shouldSetupForReader(id descpb.ID, parentID descpb.ID) bool {
+// access via external row data, based on the ID for tables with fixed IDs or on
+// the name and parentID for tables with dynamic IDs.
+func shouldSetupForReader(id descpb.ID, name string, parentID descpb.ID) bool {
 	switch id {
 	case keys.UsersTableID, keys.RoleMembersTableID, keys.RoleOptionsTableID,
 		keys.DatabaseRoleSettingsTableID, keys.TableStatisticsTableID:
 		return true
 	default:
-		return parentID != keys.SystemDatabaseID &&
-			id != keys.SystemDatabaseID
+		if parentID == keys.SystemDatabaseID {
+			switch name {
+			case string(catconstants.SystemPrivilegeTableName):
+				return true
+			default:
+				return false
+			}
+		}
+		return id != keys.SystemDatabaseID
 	}
 }
 

pkg/sql/catalog/replication/reader_catalog_test.go

@@ -185,6 +185,7 @@ func TestReaderCatalog(t *testing.T) {
 	r.srcRunner.Exec(t, `
 CREATE USER roacher WITH CREATEROLE;
 GRANT ADMIN TO roacher;
+GRANT SYSTEM VIEWACTIVITY TO roacher;
 ALTER USER roacher SET timezone='America/New_York';
 CREATE DATABASE db1;
 CREATE SCHEMA db1.sc1;
@@ -226,6 +227,7 @@ INSERT INTO t3(n) VALUES (3);
 		"INSERT INTO t1(val) VALUES('inactive');",
 		"CREATE USER roacher2 WITH CREATEROLE;",
 		"GRANT ADMIN TO roacher2;",
+		"GRANT SYSTEM VIEWACTIVITY TO roacher2;",
 		"ALTER USER roacher2 SET timezone='America/New_York';",
 		"CREATE TABLE t4(n int)",
 		"INSERT INTO t4 VALUES (32)",
@@ -239,6 +241,7 @@ INSERT INTO t3(n) VALUES (3);
 	r.compareEqual(t, "SELECT * FROM t1 ORDER BY n")
 	r.compareEqual(t, "SELECT * FROM v1 ORDER BY 1")
 	r.compareEqual(t, "SELECT * FROM system.users")
+	r.compareEqual(t, "SHOW SYSTEM GRANTS FOR roacher")
 	r.compareEqual(t, "SELECT * FROM system.table_statistics")
 	r.compareEqual(t, "SELECT * FROM system.role_options")
 	r.compareEqual(t, "SELECT * FROM system.database_role_settings")
@@ -266,6 +269,8 @@ INSERT INTO t3(n) VALUES (3);
 	r.compareEqual(t, "SELECT * FROM system.table_statistics")
 	r.compareEqual(t, "SELECT * FROM system.role_options")
 	r.compareEqual(t, "SELECT * FROM system.database_role_settings")
+	r.compareEqual(t, "SHOW SYSTEM GRANTS FOR roacher")
+	r.compareEqual(t, "SHOW SYSTEM GRANTS FOR roacher2")
 	r.compareEqual(t, "SELECT * FROM t4 ORDER BY n")
 	r.compareEqual(t, "SELECT * FROM t5 ORDER BY n")
 	r.compareEqual(t, "SELECT name FROM system.namespace ORDER BY name")