0002-Refactor-parent-ACL-check-20220115.patch

From 95fc05ff7822e55c570c8fd6282dd3d6459bb594 Mon Sep 17 00:00:00 2001
From: Tomas Vondra <tomas@2ndquadrant.com>
Date: Sun, 12 Dec 2021 02:28:41 +0100
Subject: [PATCH 2/2] Refactor parent ACL check

selfuncs.c is 8k lines long, and this makes it 30 LOC shorter.
---
 src/backend/utils/adt/selfuncs.c | 140 ++++++++++++-------------------
 1 file changed, 52 insertions(+), 88 deletions(-)

diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index b00ee6a3bef..b1ceb01775f 100644
--- a/src/backend/utils/adt/selfuncs.c
+++ b/src/backend/utils/adt/selfuncs.c
@@ -187,6 +187,8 @@ static char *convert_string_datum(Datum value, Oid typid, Oid collid,
 								  bool *failure);
 static double convert_timevalue_to_scalar(Datum value, Oid typid,
 										  bool *failure);
+static void recheck_parent_acl(PlannerInfo *root, VariableStatData *vardata,
+								Oid relid);
 static void examine_simple_variable(PlannerInfo *root, Var *var,
 									VariableStatData *vardata);
 static bool get_variable_range(PlannerInfo *root, VariableStatData *vardata,
@@ -5153,51 +5155,7 @@ examine_variable(PlannerInfo *root, Node *node, int varRelid,
 									(pg_class_aclcheck(rte->relid, userid,
 													   ACL_SELECT) == ACLCHECK_OK);
 
-								/*
-								 * If the user doesn't have permissions to
-								 * access an inheritance child relation, check
-								 * the permissions of the table actually
-								 * mentioned in the query, since most likely
-								 * the user does have that permission.  Note
-								 * that whole-table select privilege on the
-								 * parent doesn't quite guarantee that the
-								 * user could read all columns of the child.
-								 * But in practice it's unlikely that any
-								 * interesting security violation could result
-								 * from allowing access to the expression
-								 * index's stats, so we allow it anyway.  See
-								 * similar code in examine_simple_variable()
-								 * for additional comments.
-								 */
-								if (!vardata->acl_ok &&
-									root->append_rel_array != NULL)
-								{
-									AppendRelInfo *appinfo;
-									Index		varno = index->rel->relid;
-
-									appinfo = root->append_rel_array[varno];
-									while (appinfo &&
-										   planner_rt_fetch(appinfo->parent_relid,
-															root)->rtekind == RTE_RELATION)
-									{
-										varno = appinfo->parent_relid;
-										appinfo = root->append_rel_array[varno];
-									}
-									if (varno != index->rel->relid)
-									{
-										/* Repeat access check on this rel */
-										rte = planner_rt_fetch(varno, root);
-										Assert(rte->rtekind == RTE_RELATION);
-
-										userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
-
-										vardata->acl_ok =
-											rte->securityQuals == NIL &&
-											(pg_class_aclcheck(rte->relid,
-															   userid,
-															   ACL_SELECT) == ACLCHECK_OK);
-									}
-								}
+								recheck_parent_acl(root, vardata, index->rel->relid);
 							}
 							else
 							{
@@ -5285,49 +5243,7 @@ examine_variable(PlannerInfo *root, Node *node, int varRelid,
 						(pg_class_aclcheck(rte->relid, userid,
 										   ACL_SELECT) == ACLCHECK_OK);
 
-					/*
-					 * If the user doesn't have permissions to access an
-					 * inheritance child relation, check the permissions of
-					 * the table actually mentioned in the query, since most
-					 * likely the user does have that permission.  Note that
-					 * whole-table select privilege on the parent doesn't
-					 * quite guarantee that the user could read all columns of
-					 * the child. But in practice it's unlikely that any
-					 * interesting security violation could result from
-					 * allowing access to the expression stats, so we allow it
-					 * anyway.  See similar code in examine_simple_variable()
-					 * for additional comments.
-					 */
-					if (!vardata->acl_ok &&
-						root->append_rel_array != NULL)
-					{
-						AppendRelInfo *appinfo;
-						Index		varno = onerel->relid;
-
-						appinfo = root->append_rel_array[varno];
-						while (appinfo &&
-							   planner_rt_fetch(appinfo->parent_relid,
-												root)->rtekind == RTE_RELATION)
-						{
-							varno = appinfo->parent_relid;
-							appinfo = root->append_rel_array[varno];
-						}
-						if (varno != onerel->relid)
-						{
-							/* Repeat access check on this rel */
-							rte = planner_rt_fetch(varno, root);
-							Assert(rte->rtekind == RTE_RELATION);
-
-							userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
-
-							vardata->acl_ok =
-								rte->securityQuals == NIL &&
-								(pg_class_aclcheck(rte->relid,
-												   userid,
-												   ACL_SELECT) == ACLCHECK_OK);
-						}
-					}
-
+					recheck_parent_acl(root, vardata, onerel->relid);
 					break;
 				}
 
@@ -5337,6 +5253,54 @@ examine_variable(PlannerInfo *root, Node *node, int varRelid,
 	}
 }
 
+/*
+ * If the user doesn't have permissions to access an inheritance child
+ * relation, check the permissions of the table actually mentioned in the
+ * query, since most likely the user does have that permission.  Note that
+ * whole-table select privilege on the parent doesn't quite guarantee that the
+ * user could read all columns of the child.  But in practice it's unlikely
+ * that any interesting security violation could result from allowing access to
+ * the expression stats, so we allow it anyway.  See similar code in
+ * examine_simple_variable() for additional comments.
+ */
+static void
+recheck_parent_acl(PlannerInfo *root, VariableStatData *vardata, Oid relid)
+{
+	RangeTblEntry	*rte;
+	Oid		userid;
+
+	if (!vardata->acl_ok &&
+		root->append_rel_array != NULL)
+	{
+		AppendRelInfo *appinfo;
+		Index		varno = relid;
+
+		appinfo = root->append_rel_array[varno];
+		while (appinfo &&
+			   planner_rt_fetch(appinfo->parent_relid,
+								root)->rtekind == RTE_RELATION)
+		{
+			varno = appinfo->parent_relid;
+			appinfo = root->append_rel_array[varno];
+		}
+
+		if (varno != relid)
+		{
+			/* Repeat access check on this rel */
+			rte = planner_rt_fetch(varno, root);
+			Assert(rte->rtekind == RTE_RELATION);
+
+			userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
+
+			vardata->acl_ok =
+				rte->securityQuals == NIL &&
+				(pg_class_aclcheck(rte->relid,
+								   userid,
+								   ACL_SELECT) == ACLCHECK_OK);
+		}
+	}
+}
+
 /*
  * examine_simple_variable
  *		Handle a simple Var for examine_variable
-- 
2.31.1