From 6111d2e02487e6de6726c6a5fe2746ce7f9d559f Mon Sep 17 00:00:00 2001
From: steve-chavez <stevechavezast@gmail.com>
Date: Mon, 23 Mar 2026 17:08:41 -0500
Subject: [PATCH] refactor ExecCheckPermissionsModified for ACL_SELECT

Currently the code on ExecCheckOneRelPerms duplicates the logic
in ExecCheckPermissionsModified.

This change accommodates ExecCheckPermissionsModified to handle
ACL_SELECT and makes ExecCheckOneRelPerms reuse code. It also merges
similar comments.

Main benefit is that it reduces LOCs and centralizes column privilege
logic.
---
 src/backend/executor/execMain.c | 70 +++++++++++----------------------
 1 file changed, 24 insertions(+), 46 deletions(-)

diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 58b84955c2b..c1cc8251186 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -676,8 +676,6 @@ ExecCheckOneRelPerms(RTEPermissionInfo *perminfo)
 	remainingPerms = requiredPerms & ~relPerms;
 	if (remainingPerms != 0)
 	{
-		int			col = -1;
-
 		/*
 		 * If we lack any permissions that exist only as relation permissions,
 		 * we can fail straight away.
@@ -692,45 +690,13 @@ ExecCheckOneRelPerms(RTEPermissionInfo *perminfo)
 		 * to report a column-level error if we have some but not all of the
 		 * column privileges.
 		 */
-		if (remainingPerms & ACL_SELECT)
-		{
-			/*
-			 * When the query doesn't explicitly reference any columns (for
-			 * example, SELECT COUNT(*) FROM table), allow the query if we
-			 * have SELECT on any column of the rel, as per SQL spec.
-			 */
-			if (bms_is_empty(perminfo->selectedCols))
-			{
-				if (pg_attribute_aclcheck_all(relOid, userid, ACL_SELECT,
-											  ACLMASK_ANY) != ACLCHECK_OK)
-					return false;
-			}
-
-			while ((col = bms_next_member(perminfo->selectedCols, col)) >= 0)
-			{
-				/* bit #s are offset by FirstLowInvalidHeapAttributeNumber */
-				AttrNumber	attno = col + FirstLowInvalidHeapAttributeNumber;
-
-				if (attno == InvalidAttrNumber)
-				{
-					/* Whole-row reference, must have priv on all cols */
-					if (pg_attribute_aclcheck_all(relOid, userid, ACL_SELECT,
-												  ACLMASK_ALL) != ACLCHECK_OK)
-						return false;
-				}
-				else
-				{
-					if (pg_attribute_aclcheck(relOid, attno, userid,
-											  ACL_SELECT) != ACLCHECK_OK)
-						return false;
-				}
-			}
-		}
+		if (remainingPerms & ACL_SELECT &&
+			!ExecCheckPermissionsModified(relOid,
+										  userid,
+										  perminfo->selectedCols,
+										  ACL_SELECT))
+			return false;

-		/*
-		 * Basically the same for the mod columns, for both INSERT and UPDATE
-		 * privilege as specified by remainingPerms.
-		 */
 		if (remainingPerms & ACL_INSERT &&
 			!ExecCheckPermissionsModified(relOid,
 										  userid,
@@ -750,7 +716,7 @@ ExecCheckOneRelPerms(RTEPermissionInfo *perminfo)

 /*
  * ExecCheckPermissionsModified
- *		Check INSERT or UPDATE access permissions for a single relation (these
+ *		Check SELECT, INSERT or UPDATE access permissions for a single relation (these
  *		are processed uniformly).
  */
 static bool
@@ -760,9 +726,11 @@ ExecCheckPermissionsModified(Oid relOid, Oid userid, Bitmapset *modifiedCols,
 	int			col = -1;

 	/*
-	 * When the query doesn't explicitly update any columns, allow the query
-	 * if we have permission on any column of the rel.  This is to handle
-	 * SELECT FOR UPDATE as well as possible corner cases in UPDATE.
+	 * When the query doesn't explicitly reference any columns (for
+	 * example, SELECT COUNT(*) FROM table or INSERT DEFAULT VALUES),
+   * allow the query if we have permission on any column of the rel, as per SQL spec.
+	 *
+	 * This handles SELECT FOR UPDATE as well as possible corner cases in UPDATE.
 	 */
 	if (bms_is_empty(modifiedCols))
 	{
@@ -776,10 +744,20 @@ ExecCheckPermissionsModified(Oid relOid, Oid userid, Bitmapset *modifiedCols,
 		/* bit #s are offset by FirstLowInvalidHeapAttributeNumber */
 		AttrNumber	attno = col + FirstLowInvalidHeapAttributeNumber;

+		/* Whole-row reference, must have priv on all cols */
 		if (attno == InvalidAttrNumber)
 		{
-			/* whole-row reference can't happen here */
-			elog(ERROR, "whole-row update is not implemented");
+
+			/* In the case of SELECT * we have to check for all column permissions */
+			if (requiredPerms == ACL_SELECT)
+			{
+				if (pg_attribute_aclcheck_all(relOid, userid, requiredPerms,
+											  ACLMASK_ALL) != ACLCHECK_OK)
+					return false;
+			}
+			else
+				/* whole-row reference can't happen here */
+				elog(ERROR, "whole-row update is not implemented");
 		}
 		else
 		{
--
2.40.1