Merge pull request #134970 from cockroachdb/blathers/backport-release…

…-24.3-134850

release-24.3: sql: Drop Role does not properly check if target user does not exist

pkg/sql/drop_role.go

@@ -104,8 +104,22 @@ func (n *DropRoleNode) startExec(params runParams) error {
 		if name.IsReserved() {
 			return pgerror.Newf(pgcode.ReservedName, "role name %q is reserved", name.Normalized())
 		}
+
 		// Non-admin users cannot drop admins.
 		if !hasAdmin {
+			if n.ifExists {
+				// If `IF EXISTS` was specified, then a non-existing role should be
+				// skipped without causing any error.
+				roleExists, err := RoleExists(params.ctx, params.p.InternalSQLTxn(), name)
+				if err != nil {
+					return err
+				}
+				if !roleExists {
+					// If the role does not exist, we can skip the check for targetIsAdmin.
+					continue
+				}
+			}
+
 			targetIsAdmin, err := params.p.UserHasAdminRole(params.ctx, name)
 			if err != nil {
 				return err
@@ -114,6 +128,7 @@ func (n *DropRoleNode) startExec(params runParams) error {
 				return pgerror.New(pgcode.InsufficientPrivilege, "must be superuser to drop superusers")
 			}
 		}
+
 	}
 
 	privilegeObjectFormatter := tree.NewFmtCtx(tree.FmtSimple)

pkg/sql/logictest/testdata/logic_test/drop_role_with_default_privileges

@@ -159,3 +159,27 @@ ALTER DEFAULT PRIVILEGES FOR ALL ROLES IN SCHEMA public REVOKE ALL ON SEQUENCES
 
 statement ok
 DROP ROLE testuser4;
+
+subtest fix_for_regression_bug_134538
+
+statement ok
+CREATE USER not_admin WITH PASSWORD '123';
+GRANT SYSTEM CREATEROLE TO not_admin;
+SET ROLE not_admin;
+
+statement error pq: role/user "a_user_that_does_not_exist" does not exist
+DROP USER a_user_that_does_not_exist;
+
+statement ok
+DROP USER IF EXISTS a_user_that_does_not_exist;
+
+statement ok
+SET ROLE admin;
+
+statement error pq: role/user "a_user_that_does_not_exist" does not exist
+DROP USER a_user_that_does_not_exist;
+
+statement ok
+DROP USER IF EXISTS a_user_that_does_not_exist;
+
+subtest end