v2-0001-Don-t-check-child-s-LOCK-privilege-when-locked-re.patch

From 77bf8ac02734b75f8a1e7c61bfc84e1b37189893 Mon Sep 17 00:00:00 2001
From: Amit Langote <amitlangote09@gmail.com>
Date: Wed, 22 Jan 2020 15:43:41 +0900
Subject: [PATCH v2] Don't check child's LOCK privilege when locked recursively

---
 src/backend/commands/lockcmds.c          | 33 +++++++++++---------------------
 src/test/regress/expected/lock.out       |  8 ++------
 src/test/regress/expected/privileges.out |  7 +++++++
 src/test/regress/sql/lock.sql            |  7 ++-----
 src/test/regress/sql/privileges.sql      |  6 ++++++
 5 files changed, 28 insertions(+), 33 deletions(-)

diff --git a/src/backend/commands/lockcmds.c b/src/backend/commands/lockcmds.c
index 329ab849c0..d8cafc42bb 100644
--- a/src/backend/commands/lockcmds.c
+++ b/src/backend/commands/lockcmds.c
@@ -28,7 +28,7 @@
 #include "utils/lsyscache.h"
 #include "utils/syscache.h"
 
-static void LockTableRecurse(Oid reloid, LOCKMODE lockmode, bool nowait, Oid userid);
+static void LockTableRecurse(Oid reloid, LOCKMODE lockmode, bool nowait);
 static AclResult LockTableAclCheck(Oid relid, LOCKMODE lockmode, Oid userid);
 static void RangeVarCallbackForLockTable(const RangeVar *rv, Oid relid,
 										 Oid oldrelid, void *arg);
@@ -59,7 +59,7 @@ LockTableCommand(LockStmt *lockstmt)
 		if (get_rel_relkind(reloid) == RELKIND_VIEW)
 			LockViewRecurse(reloid, lockstmt->mode, lockstmt->nowait, NIL);
 		else if (recurse)
-			LockTableRecurse(reloid, lockstmt->mode, lockstmt->nowait, GetUserId());
+			LockTableRecurse(reloid, lockstmt->mode, lockstmt->nowait);
 	}
 }
 
@@ -108,35 +108,26 @@ RangeVarCallbackForLockTable(const RangeVar *rv, Oid relid, Oid oldrelid,
 /*
  * Apply LOCK TABLE recursively over an inheritance tree
  *
- * We use find_inheritance_children not find_all_inheritors to avoid taking
- * locks far in advance of checking privileges.  This means we'll visit
- * multiply-inheriting children more than once, but that's no problem.
+ * This doesn't check permission to perform LOCK TABLE on the child tables,
+ * because getting here means that the user has permission to lock the
+ * parent which is enough.
  */
 static void
-LockTableRecurse(Oid reloid, LOCKMODE lockmode, bool nowait, Oid userid)
+LockTableRecurse(Oid reloid, LOCKMODE lockmode, bool nowait)
 {
 	List	   *children;
 	ListCell   *lc;
 
-	children = find_inheritance_children(reloid, NoLock);
+	children = find_all_inheritors(reloid, NoLock, NULL);
 
 	foreach(lc, children)
 	{
 		Oid			childreloid = lfirst_oid(lc);
-		AclResult	aclresult;
 
-		/* Check permissions before acquiring the lock. */
-		aclresult = LockTableAclCheck(childreloid, lockmode, userid);
-		if (aclresult != ACLCHECK_OK)
-		{
-			char	   *relname = get_rel_name(childreloid);
-
-			if (!relname)
-				continue;		/* child concurrently dropped, just skip it */
-			aclcheck_error(aclresult, get_relkind_objtype(get_rel_relkind(childreloid)), relname);
-		}
+		/* Parent already locked. */
+		if (childreloid == reloid)
+			continue;
 
-		/* We have enough rights to lock the relation; do so. */
 		if (!nowait)
 			LockRelationOid(childreloid, lockmode);
 		else if (!ConditionalLockRelationOid(childreloid, lockmode))
@@ -162,8 +153,6 @@ LockTableRecurse(Oid reloid, LOCKMODE lockmode, bool nowait, Oid userid)
 			UnlockRelationOid(childreloid, lockmode);
 			continue;
 		}
-
-		LockTableRecurse(childreloid, lockmode, nowait, userid);
 	}
 }
 
@@ -241,7 +230,7 @@ LockViewRecurse_walker(Node *node, LockViewRecurse_context *context)
 			if (relkind == RELKIND_VIEW)
 				LockViewRecurse(relid, context->lockmode, context->nowait, context->ancestor_views);
 			else if (rte->inh)
-				LockTableRecurse(relid, context->lockmode, context->nowait, context->viewowner);
+				LockTableRecurse(relid, context->lockmode, context->nowait);
 		}
 
 		return query_tree_walker(query,
diff --git a/src/test/regress/expected/lock.out b/src/test/regress/expected/lock.out
index 185fd2f879..898fd6accd 100644
--- a/src/test/regress/expected/lock.out
+++ b/src/test/regress/expected/lock.out
@@ -138,16 +138,12 @@ CREATE TABLE lock_tbl3 () INHERITS (lock_tbl2);
 BEGIN TRANSACTION;
 LOCK TABLE lock_tbl1 * IN ACCESS EXCLUSIVE MODE;
 ROLLBACK;
--- Verify that we can't lock a child table just because we have permission
--- on the parent, but that we can lock the parent only.
+-- Child tables are locked without granting explicit permission to do so as
+-- long as we have permission to lock the parent.
 GRANT UPDATE ON TABLE lock_tbl1 TO regress_rol_lock1;
 SET ROLE regress_rol_lock1;
 BEGIN;
 LOCK TABLE lock_tbl1 * IN ACCESS EXCLUSIVE MODE;
-ERROR:  permission denied for table lock_tbl2
-ROLLBACK;
-BEGIN;
-LOCK TABLE ONLY lock_tbl1;
 ROLLBACK;
 RESET ROLE;
 --
diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out
index 297cedbacf..c2d037b614 100644
--- a/src/test/regress/expected/privileges.out
+++ b/src/test/regress/expected/privileges.out
@@ -716,6 +716,13 @@ ERROR:  permission denied for table atestc
 TRUNCATE atestp1; -- ok
 TRUNCATE atestc; -- fail
 ERROR:  permission denied for table atestc
+BEGIN;
+LOCK atestp1;
+END;
+BEGIN;
+LOCK atestc;
+ERROR:  permission denied for table atestc
+END;
 -- privileges on functions, languages
 -- switch to superuser
 \c -
diff --git a/src/test/regress/sql/lock.sql b/src/test/regress/sql/lock.sql
index 26a7e59a13..8947d8e52e 100644
--- a/src/test/regress/sql/lock.sql
+++ b/src/test/regress/sql/lock.sql
@@ -101,16 +101,13 @@ BEGIN TRANSACTION;
 LOCK TABLE lock_tbl1 * IN ACCESS EXCLUSIVE MODE;
 ROLLBACK;
 
--- Verify that we can't lock a child table just because we have permission
--- on the parent, but that we can lock the parent only.
+-- Child tables are locked without granting explicit permission to do so as
+-- long as we have permission to lock the parent.
 GRANT UPDATE ON TABLE lock_tbl1 TO regress_rol_lock1;
 SET ROLE regress_rol_lock1;
 BEGIN;
 LOCK TABLE lock_tbl1 * IN ACCESS EXCLUSIVE MODE;
 ROLLBACK;
-BEGIN;
-LOCK TABLE ONLY lock_tbl1;
-ROLLBACK;
 RESET ROLE;
 
 --
diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql
index dfe2603fe2..2ba69617dc 100644
--- a/src/test/regress/sql/privileges.sql
+++ b/src/test/regress/sql/privileges.sql
@@ -459,6 +459,12 @@ UPDATE atestp1 SET f1 = 1; -- ok
 UPDATE atestc SET f1 = 1; -- fail
 TRUNCATE atestp1; -- ok
 TRUNCATE atestc; -- fail
+BEGIN;
+LOCK atestp1;
+END;
+BEGIN;
+LOCK atestc;
+END;
 
 -- privileges on functions, languages
 
-- 
2.16.5