public inbox for [email protected]
help / color / mirror / Atom feedFrom: Steve Chavez <[email protected]>
To: PostgreSQL-development <[email protected]>
Subject: [PATCH] refactor ExecCheckPermissionsModified for ACL_SELECT
Date: Mon, 23 Mar 2026 17:23:59 -0500
Message-ID: <CAGRrpzYP+3zEk__KZu-a5uWySfwgRFk6eoPXKrA5AdtBTXR=ng@mail.gmail.com> (raw)
Hello hackers,
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.
Best regards,
Steve Chavez
Attachments:
[text/x-patch] 0001-refactor-ExecCheckPermissionsModified-for-ACL_SELECT.patch (4.5K, 3-0001-refactor-ExecCheckPermissionsModified-for-ACL_SELECT.patch)
download | inline diff:
From 6111d2e02487e6de6726c6a5fe2746ce7f9d559f Mon Sep 17 00:00:00 2001
From: steve-chavez <[email protected]>
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
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected]
Subject: Re: [PATCH] refactor ExecCheckPermissionsModified for ACL_SELECT
In-Reply-To: <CAGRrpzYP+3zEk__KZu-a5uWySfwgRFk6eoPXKrA5AdtBTXR=ng@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox