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

4 files changed, 32 insertions, 36 deletions

diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 20c7b1ad05..1ceac2e0cf 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -31,6 +31,7 @@
 #include "optimizer/appendinfo.h"
 #include "optimizer/clauses.h"
 #include "optimizer/cost.h"
+#include "optimizer/inherit.h"
 #include "optimizer/optimizer.h"
 #include "optimizer/pathnode.h"
 #include "optimizer/paths.h"
@@ -657,8 +658,8 @@ postgresGetForeignRelSize(PlannerInfo *root,
 	/*
 	 * If the table or the server is configured to use remote estimates,
 	 * identify which user to do remote access as during planning.  This
-	 * should match what ExecCheckRTEPerms() does.  If we fail due to lack of
-	 * permissions, the query would have failed at runtime anyway.
+	 * should match what ExecCheckPermissions() does.  If we fail due to lack
+	 * of permissions, the query would have failed at runtime anyway.
 	 */
 	if (fpinfo->use_remote_estimate)
 	{
@@ -1809,7 +1810,8 @@ postgresPlanForeignModify(PlannerInfo *root,
 	else if (operation == CMD_UPDATE)
 	{
 		int			col;
-		Bitmapset  *allUpdatedCols = bms_union(rte->updatedCols, rte->extraUpdatedCols);
+		RelOptInfo *rel = find_base_rel(root, resultRelation);
+		Bitmapset  *allUpdatedCols = get_rel_all_updated_cols(root, rel);
 
 		col = -1;
 		while ((col = bms_next_member(allUpdatedCols, col)) >= 0)
@@ -2650,7 +2652,7 @@ postgresBeginDirectModify(ForeignScanState *node, int eflags)
 
 	/*
 	 * Identify which user to do the remote access as.  This should match what
-	 * ExecCheckRTEPerms() does.
+	 * ExecCheckPermissions() does.
 	 */
 	userid = OidIsValid(fsplan->checkAsUser) ? fsplan->checkAsUser : GetUserId();
 
@@ -3975,11 +3977,8 @@ create_foreign_modify(EState *estate,
 	fmstate = (PgFdwModifyState *) palloc0(sizeof(PgFdwModifyState));
 	fmstate->rel = rel;
 
-	/*
-	 * Identify which user to do the remote access as.  This should match what
-	 * ExecCheckRTEPerms() does.
-	 */
-	userid = OidIsValid(rte->checkAsUser) ? rte->checkAsUser : GetUserId();
+	/* Identify which user to do the remote access as. */
+	userid = ExecGetResultRelCheckAsUser(resultRelInfo, estate);
 
 	/* Get info about foreign table. */
 	table = GetForeignTable(RelationGetRelid(rel));
diff --git a/contrib/sepgsql/dml.c b/contrib/sepgsql/dml.c
index d75335e327..e010594283 100644
--- a/contrib/sepgsql/dml.c
+++ b/contrib/sepgsql/dml.c
@@ -23,6 +23,7 @@
 #include "commands/tablecmds.h"
 #include "executor/executor.h"
 #include "nodes/bitmapset.h"
+#include "parser/parsetree.h"
 #include "sepgsql.h"
 #include "utils/lsyscache.h"
 #include "utils/syscache.h"
@@ -277,38 +278,33 @@ check_relation_privileges(Oid relOid,
  * Entrypoint of the DML permission checks
  */
 bool
-sepgsql_dml_privileges(List *rangeTabls, bool abort_on_violation)
+sepgsql_dml_privileges(List *rangeTbls, List *rteperminfos,
+					   bool abort_on_violation)
 {
 	ListCell   *lr;
 
-	foreach(lr, rangeTabls)
+	foreach(lr, rteperminfos)
 	{
-		RangeTblEntry *rte = lfirst(lr);
+		RTEPermissionInfo *perminfo = lfirst_node(RTEPermissionInfo, lr);
 		uint32		required = 0;
 		List	   *tableIds;
 		ListCell   *li;
 
 		/*
-		 * Only regular relations shall be checked
-		 */
-		if (rte->rtekind != RTE_RELATION)
-			continue;
-
-		/*
 		 * Find out required permissions
 		 */
-		if (rte->requiredPerms & ACL_SELECT)
+		if (perminfo->requiredPerms & ACL_SELECT)
 			required |= SEPG_DB_TABLE__SELECT;
-		if (rte->requiredPerms & ACL_INSERT)
+		if (perminfo->requiredPerms & ACL_INSERT)
 			required |= SEPG_DB_TABLE__INSERT;
-		if (rte->requiredPerms & ACL_UPDATE)
+		if (perminfo->requiredPerms & ACL_UPDATE)
 		{
-			if (!bms_is_empty(rte->updatedCols))
+			if (!bms_is_empty(perminfo->updatedCols))
 				required |= SEPG_DB_TABLE__UPDATE;
 			else
 				required |= SEPG_DB_TABLE__LOCK;
 		}
-		if (rte->requiredPerms & ACL_DELETE)
+		if (perminfo->requiredPerms & ACL_DELETE)
 			required |= SEPG_DB_TABLE__DELETE;
 
 		/*
@@ -323,10 +319,10 @@ sepgsql_dml_privileges(List *rangeTabls, bool abort_on_violation)
 		 * expand rte->relid into list of OIDs of inheritance hierarchy, then
 		 * checker routine will be invoked for each relations.
 		 */
-		if (!rte->inh)
-			tableIds = list_make1_oid(rte->relid);
+		if (!perminfo->inh)
+			tableIds = list_make1_oid(perminfo->relid);
 		else
-			tableIds = find_all_inheritors(rte->relid, NoLock, NULL);
+			tableIds = find_all_inheritors(perminfo->relid, NoLock, NULL);
 
 		foreach(li, tableIds)
 		{
@@ -339,12 +335,12 @@ sepgsql_dml_privileges(List *rangeTabls, bool abort_on_violation)
 			 * child table has different attribute numbers, so we need to fix
 			 * up them.
 			 */
-			selectedCols = fixup_inherited_columns(rte->relid, tableOid,
-												   rte->selectedCols);
-			insertedCols = fixup_inherited_columns(rte->relid, tableOid,
-												   rte->insertedCols);
-			updatedCols = fixup_inherited_columns(rte->relid, tableOid,
-												  rte->updatedCols);
+			selectedCols = fixup_inherited_columns(perminfo->relid, tableOid,
+												   perminfo->selectedCols);
+			insertedCols = fixup_inherited_columns(perminfo->relid, tableOid,
+												   perminfo->insertedCols);
+			updatedCols = fixup_inherited_columns(perminfo->relid, tableOid,
+												  perminfo->updatedCols);
 
 			/*
 			 * check permissions on individual tables
diff --git a/contrib/sepgsql/hooks.c b/contrib/sepgsql/hooks.c
index 363ac06700..4e1fe7ee5b 100644
--- a/contrib/sepgsql/hooks.c
+++ b/contrib/sepgsql/hooks.c
@@ -287,17 +287,17 @@ sepgsql_object_access(ObjectAccessType access,
  * Entrypoint of DML permissions
  */
 static bool
-sepgsql_exec_check_perms(List *rangeTabls, bool abort)
+sepgsql_exec_check_perms(List *rangeTbls, List *rteperminfos, bool abort)
 {
 	/*
 	 * If security provider is stacking and one of them replied 'false' at
 	 * least, we don't need to check any more.
 	 */
 	if (next_exec_check_perms_hook &&
-		!(*next_exec_check_perms_hook) (rangeTabls, abort))
+		!(*next_exec_check_perms_hook) (rangeTbls, rteperminfos, abort))
 		return false;
 
-	if (!sepgsql_dml_privileges(rangeTabls, abort))
+	if (!sepgsql_dml_privileges(rangeTbls, rteperminfos, abort))
 		return false;
 
 	return true;
diff --git a/contrib/sepgsql/sepgsql.h b/contrib/sepgsql/sepgsql.h
index f2a2c795bf..9e292271b7 100644
--- a/contrib/sepgsql/sepgsql.h
+++ b/contrib/sepgsql/sepgsql.h
@@ -274,7 +274,8 @@ extern void sepgsql_object_relabel(const ObjectAddress *object,
 /*
  * dml.c
  */
-extern bool sepgsql_dml_privileges(List *rangeTabls, bool abort_on_violation);
+extern bool sepgsql_dml_privileges(List *rangeTabls, List *rteperminfos,
+								   bool abort_on_violation);
 
 /*
  * database.c