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
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
