v29-0002-Stop-accessing-checkAsUser-via-RTE-in-some-cases.patch

application/octet-stream

Filename: v29-0002-Stop-accessing-checkAsUser-via-RTE-in-some-cases.patch
Type: application/octet-stream
Part: 1
Message: Re: ExecRTCheckPerms() and many prunable partitions

Patch

Format: format-patch
Series: patch v29-0002
Subject: Stop accessing checkAsUser via RTE in some cases
File+
contrib/postgres_fdw/postgres_fdw.c 5 10
src/backend/optimizer/plan/createplan.c 5 1
src/backend/statistics/extended_stats.c 3 3
src/backend/utils/adt/selfuncs.c 7 6
src/include/nodes/pathnodes.h 1 1
src/include/nodes/plannodes.h 2 0
From 295b6c97400a308eaab2484eb32cc27ed6b43918 Mon Sep 17 00:00:00 2001
From: amitlan <amitlangote09@gmail.com>
Date: Tue, 29 Nov 2022 16:14:57 +0900
Subject: [PATCH v29 2/5] Stop accessing checkAsUser via RTE in some cases

A future commit will move the checkAsUser field from RangeTblEntry
to a new node that, unlike RTEs, will only be created for tables
mentioned in the query but not for the inheritance child relations
added to the query by the planner.  So, checkAsUser value for a
given child relation will have to be obtained by referring to that
for its ancestor mentioned in the query.

In preparation, it seems better to expand the use of RelOptInfo.userid
during planning in place of rte->checkAsUser so that there will be
fewer places to adjust for the above change.

Given that the child-to-ancestor mapping is not available during the
execution of a given "child" ForeignScan node, add a checkAsUser
field to ForeignScan to carry the child relation's RelOptInfo.userid.
---
 contrib/postgres_fdw/postgres_fdw.c     | 15 +++++----------
 src/backend/optimizer/plan/createplan.c |  6 +++++-
 src/backend/statistics/extended_stats.c |  6 +++---
 src/backend/utils/adt/selfuncs.c        | 13 +++++++------
 src/include/nodes/pathnodes.h           |  2 +-
 src/include/nodes/plannodes.h           |  2 ++
 6 files changed, 23 insertions(+), 21 deletions(-)

diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 8d7500abfb..afe234ea66 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -624,7 +624,6 @@ postgresGetForeignRelSize(PlannerInfo *root,
 {
 	PgFdwRelationInfo *fpinfo;
 	ListCell   *lc;
-	RangeTblEntry *rte = planner_rt_fetch(baserel->relid, root);
 
 	/*
 	 * We use PgFdwRelationInfo to pass various information to subsequent
@@ -663,7 +662,7 @@ postgresGetForeignRelSize(PlannerInfo *root,
 	 */
 	if (fpinfo->use_remote_estimate)
 	{
-		Oid			userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
+		Oid			userid = baserel->userid ? baserel->userid : GetUserId();
 
 		fpinfo->user = GetUserMapping(userid, fpinfo->server->serverid);
 	}
@@ -1510,16 +1509,14 @@ postgresBeginForeignScan(ForeignScanState *node, int eflags)
 
 	/*
 	 * Identify which user to do the remote access as.  This should match what
-	 * ExecCheckRTEPerms() does.  In case of a join or aggregate, use the
-	 * lowest-numbered member RTE as a representative; we would get the same
-	 * result from any.
+	 * ExecCheckRTEPerms() does.
 	 */
+	userid = fsplan->checkAsUser ? fsplan->checkAsUser : GetUserId();
 	if (fsplan->scan.scanrelid > 0)
 		rtindex = fsplan->scan.scanrelid;
 	else
 		rtindex = bms_next_member(fsplan->fs_relids, -1);
 	rte = exec_rt_fetch(rtindex, estate);
-	userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
 
 	/* Get info about foreign table. */
 	table = GetForeignTable(rte->relid);
@@ -2633,7 +2630,6 @@ postgresBeginDirectModify(ForeignScanState *node, int eflags)
 	EState	   *estate = node->ss.ps.state;
 	PgFdwDirectModifyState *dmstate;
 	Index		rtindex;
-	RangeTblEntry *rte;
 	Oid			userid;
 	ForeignTable *table;
 	UserMapping *user;
@@ -2655,11 +2651,10 @@ postgresBeginDirectModify(ForeignScanState *node, int eflags)
 	 * Identify which user to do the remote access as.  This should match what
 	 * ExecCheckRTEPerms() does.
 	 */
-	rtindex = node->resultRelInfo->ri_RangeTableIndex;
-	rte = exec_rt_fetch(rtindex, estate);
-	userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
+	userid = fsplan->checkAsUser ? fsplan->checkAsUser : GetUserId();
 
 	/* Get info about foreign table. */
+	rtindex = node->resultRelInfo->ri_RangeTableIndex;
 	if (fsplan->scan.scanrelid == 0)
 		dmstate->rel = ExecOpenScanRelation(estate, rtindex, eflags);
 	else
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index ac86ce9003..5013ac3377 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -4148,6 +4148,9 @@ create_foreignscan_plan(PlannerInfo *root, ForeignPath *best_path,
 	/* Copy cost data from Path to Plan; no need to make FDW do this */
 	copy_generic_path_info(&scan_plan->scan.plan, &best_path->path);
 
+	/* Copy user OID to access as; likewise no need to make FDW do this */
+	scan_plan->checkAsUser = rel->userid;
+
 	/* Copy foreign server OID; likewise, no need to make FDW do this */
 	scan_plan->fs_server = rel->serverid;
 
@@ -5794,7 +5797,8 @@ make_foreignscan(List *qptlist,
 	node->operation = CMD_SELECT;
 	node->resultRelation = 0;
 
-	/* fs_server will be filled in by create_foreignscan_plan */
+	/* checkAsUser, fs_server will be filled in by create_foreignscan_plan */
+	node->checkAsUser = InvalidOid;
 	node->fs_server = InvalidOid;
 	node->fdw_exprs = fdw_exprs;
 	node->fdw_private = fdw_private;
diff --git a/src/backend/statistics/extended_stats.c b/src/backend/statistics/extended_stats.c
index ab97e71dd7..88f03bb5a9 100644
--- a/src/backend/statistics/extended_stats.c
+++ b/src/backend/statistics/extended_stats.c
@@ -1598,6 +1598,7 @@ statext_is_compatible_clause(PlannerInfo *root, Node *clause, Index relid,
 							 Bitmapset **attnums, List **exprs)
 {
 	RangeTblEntry *rte = root->simple_rte_array[relid];
+	RelOptInfo *rel = root->simple_rel_array[relid];
 	RestrictInfo *rinfo;
 	int			clause_relid;
 	Oid			userid;
@@ -1646,10 +1647,9 @@ statext_is_compatible_clause(PlannerInfo *root, Node *clause, Index relid,
 		return false;
 
 	/*
-	 * Check that the user has permission to read all required attributes. Use
-	 * checkAsUser if it's set, in case we're accessing the table via a view.
+	 * Check that the user has permission to read all required attributes.
 	 */
-	userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
+	userid = rel->userid ? rel->userid : GetUserId();
 
 	/* Table-level SELECT privilege is sufficient for all columns */
 	if (pg_class_aclcheck(rte->relid, userid, ACL_SELECT) != ACLCHECK_OK)
diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index f116924d3c..eabb79db1d 100644
--- a/src/backend/utils/adt/selfuncs.c
+++ b/src/backend/utils/adt/selfuncs.c
@@ -5158,7 +5158,7 @@ examine_variable(PlannerInfo *root, Node *node, int varRelid,
 								 * Use checkAsUser if it's set, in case we're
 								 * accessing the table via a view.
 								 */
-								userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
+								userid = onerel->userid ? onerel->userid : GetUserId();
 
 								/*
 								 * For simplicity, we insist on the whole
@@ -5210,7 +5210,7 @@ examine_variable(PlannerInfo *root, Node *node, int varRelid,
 										rte = planner_rt_fetch(varno, root);
 										Assert(rte->rtekind == RTE_RELATION);
 
-										userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
+										userid = onerel->userid ? onerel->userid : GetUserId();
 
 										vardata->acl_ok =
 											rte->securityQuals == NIL &&
@@ -5293,7 +5293,7 @@ examine_variable(PlannerInfo *root, Node *node, int varRelid,
 					 * Use checkAsUser if it's set, in case we're accessing
 					 * the table via a view.
 					 */
-					userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
+					userid = onerel->userid ? onerel->userid : GetUserId();
 
 					/*
 					 * For simplicity, we insist on the whole table being
@@ -5341,7 +5341,7 @@ examine_variable(PlannerInfo *root, Node *node, int varRelid,
 							rte = planner_rt_fetch(varno, root);
 							Assert(rte->rtekind == RTE_RELATION);
 
-							userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
+							userid = onerel->userid ? onerel->userid : GetUserId();
 
 							vardata->acl_ok =
 								rte->securityQuals == NIL &&
@@ -5402,6 +5402,7 @@ examine_simple_variable(PlannerInfo *root, Var *var,
 
 		if (HeapTupleIsValid(vardata->statsTuple))
 		{
+			RelOptInfo *onerel = find_base_rel(root, var->varno);
 			Oid			userid;
 
 			/*
@@ -5410,7 +5411,7 @@ examine_simple_variable(PlannerInfo *root, Var *var,
 			 * from security barrier views or RLS policies.  Use checkAsUser
 			 * if it's set, in case we're accessing the table via a view.
 			 */
-			userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
+			userid = onerel->userid ? onerel->userid : GetUserId();
 
 			vardata->acl_ok =
 				rte->securityQuals == NIL &&
@@ -5479,7 +5480,7 @@ examine_simple_variable(PlannerInfo *root, Var *var,
 				rte = planner_rt_fetch(varno, root);
 				Assert(rte->rtekind == RTE_RELATION);
 
-				userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
+				userid = onerel->userid ? onerel->userid : GetUserId();
 
 				vardata->acl_ok =
 					rte->securityQuals == NIL &&
diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h
index a544b313d3..ef95429a0d 100644
--- a/src/include/nodes/pathnodes.h
+++ b/src/include/nodes/pathnodes.h
@@ -901,7 +901,7 @@ typedef struct RelOptInfo
 	 */
 	/* identifies server for the table or join */
 	Oid			serverid;
-	/* identifies user to check access as */
+	/* identifies user to check access as; 0 means to check as current user */
 	Oid			userid;
 	/* join is only valid for current user */
 	bool		useridiscurrent;
diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index 5c2ab1b379..61cae463fb 100644
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -703,6 +703,8 @@ typedef struct ForeignScan
 	Scan		scan;
 	CmdType		operation;		/* SELECT/INSERT/UPDATE/DELETE */
 	Index		resultRelation; /* direct modification target's RT index */
+	Oid			checkAsUser;	/* user to perform the scan as; 0 means to
+								 * check as current user */
 	Oid			fs_server;		/* OID of foreign server */
 	List	   *fdw_exprs;		/* expressions that FDW may evaluate */
 	List	   *fdw_private;	/* private data for FDW */
-- 
2.35.3