Merge #65722 #65792

65722: sql: ensure schema only has valid privileges after reparent database is done r=ajwerner a=RichardJCai

sql: ensure schema only has valid privileges after reparent database is done

Release note (bug fix): Previously, ALTER DATABASE ... CONVERT TO SCHEMA
could potentially leave the schema with invalid privileges thus causing the
privilege descriptor to be invalid.

Fixes #65697

65792: kvserver: skip some long tests under short r=andreimatei a=andreimatei

TestResetQuorum - 33s
TestLearnerSnapshotFailsRollback - 90s
TestReliableIntentCleanup - 80s

Release note: None

Co-authored-by: richardjcai <[email protected]>
Co-authored-by: Andrei Matei <[email protected]>

Author: 3 people

pkg/kv/kvserver/intent_resolver_integration_test.go

@@ -219,6 +219,7 @@ func TestReliableIntentCleanup(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)
 	skip.WithIssue(t, 65447, "fixing the flake uncovered additional bugs in #65458")
+	skip.UnderShort(t) // takes 85s
 	skip.UnderRace(t, "timing-sensitive test")
 	skip.UnderStress(t, "multi-node test")
 

pkg/kv/kvserver/replica_learner_test.go

@@ -309,6 +309,8 @@ func TestLearnerSnapshotFailsRollback(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)
 
+	skip.UnderShort(t) // Takes 90s.
+
 	runTest := func(t *testing.T, replicaType roachpb.ReplicaType) {
 		var rejectSnapshots int64
 		knobs, ltk := makeReplicationTestKnobs()

pkg/kv/kvserver/reset_quorum_test.go

@@ -27,6 +27,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/testutils/skip"
 	"github.com/cockroachdb/cockroach/pkg/testutils/testcluster"
 	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
+	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/errors"
 	"github.com/stretchr/testify/require"
 )
@@ -51,9 +52,11 @@ import (
 // 6. A meta range (error expected).
 func TestResetQuorum(t *testing.T) {
 	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
 
 	skip.UnderStress(t, "too many nodes")
 	skip.UnderRace(t, "takes >1m under race")
+	skip.UnderShort(t)
 
 	ctx := context.Background()
 	livenessDuration := 3000 * time.Millisecond

pkg/sql/logictest/testdata/logic_test/reparent_database

@@ -69,3 +69,16 @@ CREATE VIEW with_views.v AS SELECT x FROM with_views.t
 
 statement error pq: could not convert database "with_views" into schema because "with_views.public.t" has dependent objects \[with_views.public.v\]
 ALTER DATABASE with_views CONVERT TO SCHEMA WITH PARENT pgdatabase
+
+# Ensure only valid privileges are copied over to the schema.
+statement ok
+CREATE DATABASE to_schema;
+GRANT CREATE, DROP, SELECT, INSERT, DELETE, UPDATE ON DATABASE to_schema TO testuser;
+CREATE DATABASE parent2;
+
+# No privilege validation error should occur.
+statement ok
+ALTER DATABASE to_schema CONVERT TO SCHEMA WITH PARENT parent2;
+
+statement ok
+GRANT USAGE ON SCHEMA parent2.to_schema TO testuser;

pkg/sql/reparent_database.go

@@ -25,6 +25,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/typedesc"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
+	"github.com/cockroachdb/cockroach/pkg/sql/privilege"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/sqlerrors"
 	"github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry"
@@ -114,6 +115,16 @@ func (n *reparentDatabaseNode) startExec(params runParams) error {
 	if err != nil {
 		return err
 	}
+
+	// Not all Privileges on databases are valid on schemas.
+	// Remove any privileges that are not valid for schemas.
+	schemaPrivs := privilege.GetValidPrivilegesForObject(privilege.Schema).ToBitField()
+	privs := n.db.GetPrivileges()
+	for i, u := range privs.Users {
+		// Remove privileges that are valid for databases but not for schemas.
+		privs.Users[i].Privileges = u.Privileges & schemaPrivs
+	}
+
 	schema := schemadesc.NewBuilder(&descpb.SchemaDescriptor{
 		ParentID:   n.newParent.ID,
 		Name:       n.db.Name,