Merge #116874 #116879

116874: scripts/bump-pebble: markdown format commit list links r=dt a=dt Release note: none. Epic: none. 116879: Revert "schemachanger: Implement CREATE DATABASE" r=fqazi a=fqazi fixes #116840 fixes #116837 fixes #116829 informs #103324 This reverts commit 77372a6. Co-authored-by: David Taylor <[email protected]> Co-authored-by: Faizan Qazi <[email protected]>
cockroachdb · Dec 20, 2023 · 7a7bbff · 7a7bbff
3 parents 2c4c877 + ca71eb9 + 6bb93f2
commit 7a7bbff
Show file tree

Hide file tree

Showing 54 changed files with 28 additions and 3,492 deletions.
diff --git a/pkg/ccl/logictestccl/testdata/logic_test/as_of b/pkg/ccl/logictestccl/testdata/logic_test/as_of
@@ -46,9 +46,7 @@ SET DEFAULT_TRANSACTION_USE_FOLLOWER_READS TO TRUE
 statement error pgcode 3D000 pq: database "test" does not exist
 SELECT * FROM t
 
-# LSC and DSC would return slightly different error message when attempting to create a
-# database as of system time follower_read_timestamp() soon after a node has started.
-statement error pq: (cannot execute CREATE DATABASE in a read-only transaction|referenced descriptor ID 1: looking up ID 1: descriptor not found)
+statement error pq: cannot execute CREATE DATABASE in a read-only transaction
 CREATE DATABASE IF NOT EXISTS d2
 
 statement error pgcode 3D000 pq: database "test" does not exist
@@ -90,9 +88,7 @@ SET SESSION CHARACTERISTICS AS TRANSACTION AS OF SYSTEM TIME follower_read_times
 statement error pgcode 3D000 pq: database "test" does not exist
 SELECT * FROM t
 
-# LSC and DSC would return slightly different error message when attempting to create a
-# database as of system time follower_read_timestamp() soon after a node has started.
-statement error pq: (cannot execute CREATE DATABASE in a read-only transaction|referenced descriptor ID 1: looking up ID 1: descriptor not found)
+statement error pq: cannot execute CREATE DATABASE in a read-only transaction
 CREATE DATABASE IF NOT EXISTS d2
 
 statement error pgcode 3D000 pq: database "test" does not exist

diff --git a/pkg/ccl/schemachangerccl/backup_base_generated_test.go b/pkg/ccl/schemachangerccl/backup_base_generated_test.go
diff --git a/pkg/internal/team/TEAMS.yaml b/pkg/internal/team/TEAMS.yaml
diff --git a/pkg/sql/create_database.go b/pkg/sql/create_database.go
@@ -146,8 +146,8 @@ func (p *planner) CreateDatabase(ctx context.Context, n *tree.CreateDatabase) (p
 	return &createDatabaseNode{n: n}, nil
 }
 
-// CanCreateDatabase returns nil if current user has CREATEDB system privilege
-// or the equivalent, legacy role options.
+// CanCreateDatabase verifies that the current user has the CREATEDB
+// role option.
 func (p *planner) CanCreateDatabase(ctx context.Context) error {
 	hasCreateDB, err := p.HasGlobalPrivilegeOrRoleOption(ctx, privilege.CREATEDB)
 	if err != nil {

diff --git a/pkg/sql/logictest/testdata/logic_test/bytes b/pkg/sql/logictest/testdata/logic_test/bytes
@@ -148,12 +148,7 @@ DROP TABLE t
 subtest Regression_4312
 
 statement ok
-PREPARE r1(bytes) AS
-	SELECT descriptor::STRING
-	FROM system.descriptor
-	WHERE id = (
-		SELECT id FROM system.namespace WHERE name = 'defaultdb'
-	);
+PREPARE r1(bytes) AS SELECT descriptor::STRING FROM system.descriptor WHERE descriptor != $1 ORDER BY descriptor DESC LIMIT 1
 
 query T
 EXECUTE r1('abc')

diff --git a/pkg/sql/schemachanger/scbuild/builder_state.go b/pkg/sql/schemachanger/scbuild/builder_state.go
@@ -11,7 +11,6 @@
 package scbuild
 
 import (
-	"context"
 	"sort"
 
 	"github.com/cockroachdb/cockroach/pkg/keys"
@@ -298,7 +297,6 @@ func (b *builderState) CheckPrivilege(e scpb.Element, privilege privilege.Kind)
 	b.checkPrivilege(screl.GetDescID(e), privilege)
 }
 
-// checkPrivilege checks if current user has privilege `priv` on descriptor with `id`.
 func (b *builderState) checkPrivilege(id catid.DescID, priv privilege.Kind) {
 	b.ensureDescriptor(id)
 	c := b.descCache[id]
@@ -327,13 +325,6 @@ func (b *builderState) checkPrivilege(id catid.DescID, priv privilege.Kind) {
 	}
 }
 
-// HasGlobalPrivilegeOrRoleOption implements the scbuildstmt.PrivilegeChecker interface.
-func (b *builderState) HasGlobalPrivilegeOrRoleOption(
-	ctx context.Context, privilege privilege.Kind,
-) (bool, error) {
-	return b.auth.HasGlobalPrivilegeOrRoleOption(ctx, privilege)
-}
-
 // CurrentUserHasAdminOrIsMemberOf implements the scbuildstmt.PrivilegeChecker interface.
 func (b *builderState) CurrentUserHasAdminOrIsMemberOf(role username.SQLUsername) bool {
 	if b.hasAdmin {
@@ -354,11 +345,6 @@ func (b *builderState) CurrentUser() username.SQLUsername {
 	return b.evalCtx.SessionData().User()
 }
 
-// CheckRoleExists implements the scbuild.AuthorizationAccessor interface.
-func (b *builderState) CheckRoleExists(ctx context.Context, role username.SQLUsername) error {
-	return b.auth.CheckRoleExists(ctx, role)
-}
-
 var _ scbuildstmt.TableHelpers = (*builderState)(nil)
 
 // NextTableColumnID implements the scbuildstmt.TableHelpers interface.

diff --git a/pkg/sql/schemachanger/scbuild/dependencies.go b/pkg/sql/schemachanger/scbuild/dependencies.go
@@ -178,14 +178,14 @@ type AuthorizationAccessor interface {
 		ctx context.Context, privilegeObject privilege.Object, privilege privilege.Kind,
 	) error
 
-	// HasAdminRole verifies if current user has an admin role.
+	// HasAdminRole verifies if a user has an admin role.
 	HasAdminRole(ctx context.Context) (bool, error)
 
 	// HasOwnership returns true iff the role, or any role the role is a member
 	// of, has ownership privilege of the desc.
 	HasOwnership(ctx context.Context, privilegeObject privilege.Object) (bool, error)
 
-	// CheckPrivilegeForUser verifies that `user` has `privilege` on `descriptor`.
+	// CheckPrivilegeForUser verifies that the user has `privilege` on `descriptor`.
 	CheckPrivilegeForUser(
 		ctx context.Context, privilegeObject privilege.Object, privilege privilege.Kind, user username.SQLUsername,
 	) error
@@ -194,18 +194,11 @@ type AuthorizationAccessor interface {
 	// and indirect) and returns a map of "role" -> "isAdmin".
 	MemberOfWithAdminOption(ctx context.Context, member username.SQLUsername) (map[username.SQLUsername]bool, error)
 
-	// HasPrivilege checks if the `user` has `privilege` on `privilegeObject`.
+	// HasPrivilege checks if the user has `privilege` on `descriptor`.
 	HasPrivilege(ctx context.Context, privilegeObject privilege.Object, privilege privilege.Kind, user username.SQLUsername) (bool, error)
 
 	// HasAnyPrivilege returns true if user has any privileges at all.
 	HasAnyPrivilege(ctx context.Context, privilegeObject privilege.Object) (bool, error)
-
-	// HasGlobalPrivilegeOrRoleOption returns a bool representing whether the current user
-	// has a global privilege or the corresponding legacy role option.
-	HasGlobalPrivilegeOrRoleOption(ctx context.Context, privilege privilege.Kind) (bool, error)
-
-	// CheckRoleExists returns nil if `role` exists.
-	CheckRoleExists(ctx context.Context, role username.SQLUsername) error
 }
 
 // AstFormatter provides interfaces for formatting AST nodes.

diff --git a/pkg/sql/schemachanger/scbuild/event_log.go b/pkg/sql/schemachanger/scbuild/event_log.go
@@ -284,9 +284,7 @@ func (pb payloadBuilder) build(b buildCtx) logpb.EventPayload {
 	switch e := pb.Element().(type) {
 	case *scpb.Database:
 		if pb.TargetStatus == scpb.Status_PUBLIC {
-			return &eventpb.CreateDatabase{
-				DatabaseName: fullyQualifiedName(b, e),
-			}
+			return nil
 		} else {
 			return &eventpb.DropDatabase{
 				DatabaseName:         fullyQualifiedName(b, e),