Rework query relation permission checking

Currently, information about the permissions to be checked on relations
mentioned in a query is stored in their range table entries.  So the
executor must scan the entire range table looking for relations that
need to have permissions checked.  This can make the permission checking
part of the executor initialization needlessly expensive when many
inheritance children are present in the range range.  While the
permissions need not be checked on the individual child relations, the
executor still must visit every range table entry to filter them out.

This commit moves the permission checking information out of the range
table entries into a new plan node called RTEPermissionInfo.  Every
top-level (inheritance "root") RTE_RELATION entry in the range table
gets one and a list of those is maintained alongside the range table.
This new list is initialized by the parser when initializing the range
table.  The rewriter can add more entries to it as rules/views are
expanded.  Finally, the planner combines the lists of the individual
subqueries into one flat list that is passed to the executor for
checking.

To make it quick to find the RTEPermissionInfo entry belonging to a
given relation, RangeTblEntry gets a new Index field 'perminfoindex'
that stores the corresponding RTEPermissionInfo's index in the query's
list of the latter.

ExecutorCheckPerms_hook has gained another List * argument; the
signature is now:
typedef bool (*ExecutorCheckPerms_hook_type) (List *rangeTable,
					      List *rtePermInfos,
					      bool ereport_on_violation);
The first argument is no longer used by any in-core uses of the hook,
but we leave it in place because there may be other implementations that
do.  Implementations should likely scan the rtePermInfos list to
determine which operations to allow or deny.

Author: Amit Langote <amitlangote09@gmail.com>
Discussion: https://postgr.es/m/CA+HiwqGjJDmUhDSfv-U2qhKJjt9ST7Xh9JXC_irsAQ1TAUsJYg@mail.gmail.com

3 files changed, 32 insertions, 3 deletions

diff --git a/src/test/modules/test_oat_hooks/test_oat_hooks.c b/src/test/modules/test_oat_hooks/test_oat_hooks.c
index 4b4e259cd23..15f9006f2ea 100644
--- a/src/test/modules/test_oat_hooks/test_oat_hooks.c
+++ b/src/test/modules/test_oat_hooks/test_oat_hooks.c
@@ -55,7 +55,7 @@ static void REGRESS_object_access_hook_str(ObjectAccessType access,
 										   int subId, void *arg);
 static void REGRESS_object_access_hook(ObjectAccessType access, Oid classId,
 									   Oid objectId, int subId, void *arg);
-static bool REGRESS_exec_check_perms(List *rangeTabls, bool do_abort);
+static bool REGRESS_exec_check_perms(List *rangeTabls, List *rteperminfos, bool do_abort);
 static void REGRESS_utility_command(PlannedStmt *pstmt,
 									const char *queryString, bool readOnlyTree,
 									ProcessUtilityContext context,
@@ -345,7 +345,7 @@ REGRESS_object_access_hook(ObjectAccessType access, Oid classId, Oid objectId, i
 }
 
 static bool
-REGRESS_exec_check_perms(List *rangeTabls, bool do_abort)
+REGRESS_exec_check_perms(List *rangeTabls, List *rteperminfos, bool do_abort)
 {
 	bool		am_super = superuser_arg(GetUserId());
 	bool		allow = true;
@@ -361,7 +361,7 @@ REGRESS_exec_check_perms(List *rangeTabls, bool do_abort)
 
 	/* Forward to next hook in the chain */
 	if (next_exec_check_perms_hook &&
-		!(*next_exec_check_perms_hook) (rangeTabls, do_abort))
+		!(*next_exec_check_perms_hook) (rangeTabls, rteperminfos, do_abort))
 		allow = false;
 
 	if (allow)
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 532ea369901..fb9f936d43a 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -3569,6 +3569,18 @@ CREATE RULE rule1 AS ON INSERT TO ruletest_v1
 SET SESSION AUTHORIZATION regress_rule_user1;
 INSERT INTO ruletest_v1 VALUES (1);
 RESET SESSION AUTHORIZATION;
+-- Test that main query's relation's permissions are checked before
+-- the rule action's relation's.
+CREATE TABLE ruletest_t3 (x int);
+CREATE RULE rule2 AS ON UPDATE TO ruletest_t1
+    DO INSTEAD INSERT INTO ruletest_t2 VALUES (OLD.*);
+REVOKE ALL ON ruletest_t2 FROM regress_rule_user1;
+REVOKE ALL ON ruletest_t3 FROM regress_rule_user1;
+ALTER TABLE ruletest_t1 OWNER TO regress_rule_user1;
+SET SESSION AUTHORIZATION regress_rule_user1;
+UPDATE ruletest_t1 t1 SET x = 0 FROM ruletest_t3 t3 WHERE t1.x = t3.x;
+ERROR:  permission denied for table ruletest_t3
+RESET SESSION AUTHORIZATION;
 SELECT * FROM ruletest_t1;
  x 
 ---
@@ -3581,6 +3593,8 @@ SELECT * FROM ruletest_t2;
 (1 row)
 
 DROP VIEW ruletest_v1;
+DROP RULE rule2 ON ruletest_t1;
+DROP TABLE ruletest_t3;
 DROP TABLE ruletest_t2;
 DROP TABLE ruletest_t1;
 DROP USER regress_rule_user1;
diff --git a/src/test/regress/sql/rules.sql b/src/test/regress/sql/rules.sql
index e9261da5e06..1f858129b84 100644
--- a/src/test/regress/sql/rules.sql
+++ b/src/test/regress/sql/rules.sql
@@ -1294,10 +1294,25 @@ SET SESSION AUTHORIZATION regress_rule_user1;
 INSERT INTO ruletest_v1 VALUES (1);
 
 RESET SESSION AUTHORIZATION;
+
+-- Test that main query's relation's permissions are checked before
+-- the rule action's relation's.
+CREATE TABLE ruletest_t3 (x int);
+CREATE RULE rule2 AS ON UPDATE TO ruletest_t1
+    DO INSTEAD INSERT INTO ruletest_t2 VALUES (OLD.*);
+REVOKE ALL ON ruletest_t2 FROM regress_rule_user1;
+REVOKE ALL ON ruletest_t3 FROM regress_rule_user1;
+ALTER TABLE ruletest_t1 OWNER TO regress_rule_user1;
+SET SESSION AUTHORIZATION regress_rule_user1;
+UPDATE ruletest_t1 t1 SET x = 0 FROM ruletest_t3 t3 WHERE t1.x = t3.x;
+
+RESET SESSION AUTHORIZATION;
 SELECT * FROM ruletest_t1;
 SELECT * FROM ruletest_t2;
 
 DROP VIEW ruletest_v1;
+DROP RULE rule2 ON ruletest_t1;
+DROP TABLE ruletest_t3;
 DROP TABLE ruletest_t2;
 DROP TABLE ruletest_t1;