Move privilege check to the right place

Now that ATExecDropConstraint doesn't recurse anymore, so it's wrong to
test privileges "during recursion" there.  Move the check to
dropconstraint_internal, which is the place where recursion occurs.

In passing, remove now-useless 'recursing' argument to
ATExecDropConstraint.

Discussion: https://postgr.es/m/202309051744.y4mndw5gwzhh@alvherre.pgsql

3 files changed, 50 insertions, 10 deletions

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 47c556669f3..8a2c671b662 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -542,8 +542,7 @@ static void GetForeignKeyCheckTriggers(Relation trigrel,
 									   Oid *insertTriggerOid,
 									   Oid *updateTriggerOid);
 static void ATExecDropConstraint(Relation rel, const char *constrName,
-								 DropBehavior behavior,
-								 bool recurse, bool recursing,
+								 DropBehavior behavior, bool recurse,
 								 bool missing_ok, LOCKMODE lockmode);
 static ObjectAddress dropconstraint_internal(Relation rel,
 											 HeapTuple constraintTup, DropBehavior behavior,
@@ -5236,7 +5235,7 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab,
 			break;
 		case AT_DropConstraint: /* DROP CONSTRAINT */
 			ATExecDropConstraint(rel, cmd->name, cmd->behavior,
-								 cmd->recurse, false,
+								 cmd->recurse,
 								 cmd->missing_ok, lockmode);
 			break;
 		case AT_AlterColumnType:	/* ALTER COLUMN TYPE */
@@ -12263,8 +12262,7 @@ createForeignKeyCheckTriggers(Oid myRelOid, Oid refRelOid,
  */
 static void
 ATExecDropConstraint(Relation rel, const char *constrName,
-					 DropBehavior behavior,
-					 bool recurse, bool recursing,
+					 DropBehavior behavior, bool recurse,
 					 bool missing_ok, LOCKMODE lockmode)
 {
 	Relation	conrel;
@@ -12273,10 +12271,6 @@ ATExecDropConstraint(Relation rel, const char *constrName,
 	HeapTuple	tuple;
 	bool		found = false;
 
-	/* At top level, permission check was done in ATPrepCmd, else do it */
-	if (recursing)
-		ATSimplePermissions(AT_DropConstraint, rel, ATT_TABLE | ATT_FOREIGN_TABLE);
-
 	conrel = table_open(ConstraintRelationId, RowExclusiveLock);
 
 	/*
@@ -12302,7 +12296,7 @@ ATExecDropConstraint(Relation rel, const char *constrName,
 	{
 		List	   *readyRels = NIL;
 
-		dropconstraint_internal(rel, tuple, behavior, recurse, recursing,
+		dropconstraint_internal(rel, tuple, behavior, recurse, false,
 								missing_ok, &readyRels, lockmode);
 		found = true;
 	}
@@ -12353,6 +12347,10 @@ dropconstraint_internal(Relation rel, HeapTuple constraintTup, DropBehavior beha
 		return InvalidObjectAddress;
 	*readyRels = lappend_oid(*readyRels, RelationGetRelid(rel));
 
+	/* At top level, permission check was done in ATPrepCmd, else do it */
+	if (recursing)
+		ATSimplePermissions(AT_DropConstraint, rel, ATT_TABLE | ATT_FOREIGN_TABLE);
+
 	conrel = table_open(ConstraintRelationId, RowExclusiveLock);
 
 	con = (Form_pg_constraint) GETSTRUCT(constraintTup);
diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out
index 59583e1e417..08d93884d87 100644
--- a/src/test/regress/expected/inherit.out
+++ b/src/test/regress/expected/inherit.out
@@ -2430,6 +2430,27 @@ NOTICE:  drop cascades to 2 other objects
 DETAIL:  drop cascades to table inh_multiparent
 drop cascades to table inh_multiparent2
 --
+-- Mixed ownership inheritance tree
+--
+create role regress_alice;
+create role regress_bob;
+grant all on schema public to regress_alice, regress_bob;
+grant regress_alice to regress_bob;
+set session authorization regress_alice;
+create table inh_parent (a int not null);
+set session authorization regress_bob;
+create table inh_child () inherits (inh_parent);
+set session authorization regress_alice;
+-- alice can't do this: she doesn't own inh_child
+alter table inh_parent alter a drop not null;
+ERROR:  must be owner of table inh_child
+set session authorization regress_bob;
+alter table inh_parent alter a drop not null;
+reset session authorization;
+drop table inh_parent, inh_child;
+revoke all on schema public from regress_alice, regress_bob;
+drop role regress_alice, regress_bob;
+--
 -- Check use of temporary tables with inheritance trees
 --
 create table inh_perm_parent (a1 int);
diff --git a/src/test/regress/sql/inherit.sql b/src/test/regress/sql/inherit.sql
index abe8602682c..3d57c7ee950 100644
--- a/src/test/regress/sql/inherit.sql
+++ b/src/test/regress/sql/inherit.sql
@@ -921,6 +921,27 @@ select conrelid::regclass, contype, conname,
 drop table inh_p1, inh_p2, inh_p3, inh_p4 cascade;
 
 --
+-- Mixed ownership inheritance tree
+--
+create role regress_alice;
+create role regress_bob;
+grant all on schema public to regress_alice, regress_bob;
+grant regress_alice to regress_bob;
+set session authorization regress_alice;
+create table inh_parent (a int not null);
+set session authorization regress_bob;
+create table inh_child () inherits (inh_parent);
+set session authorization regress_alice;
+-- alice can't do this: she doesn't own inh_child
+alter table inh_parent alter a drop not null;
+set session authorization regress_bob;
+alter table inh_parent alter a drop not null;
+reset session authorization;
+drop table inh_parent, inh_child;
+revoke all on schema public from regress_alice, regress_bob;
+drop role regress_alice, regress_bob;
+
+--
 -- Check use of temporary tables with inheritance trees
 --
 create table inh_perm_parent (a1 int);