I didn't like very much this business of setting the perminfoindex
directly to '2' and '1'. It looks ugly with no explanation. What do
you think of creating the as we go along and set each index
correspondingly, as in the attached?
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"Ellos andaban todos desnudos como su madre los parió, y también las mujeres,
aunque no vi más que una, harto moza, y todos los que yo vi eran todos
mancebos, que ninguno vi de edad de más de XXX años" (Cristóbal Colón)
>From f0041bf1ac2f0c727d6b8495a63a548240c3b9c5 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvhe...@alvh.no-ip.org>
Date: Thu, 9 Mar 2023 11:33:18 +0100
Subject: [PATCH] fix ExecCheckPermissions call in RI_Initial_Check
---
src/backend/executor/execMain.c | 22 ++++++++++++++++
src/backend/utils/adt/ri_triggers.c | 40 ++++++++++++++++-------------
2 files changed, 44 insertions(+), 18 deletions(-)
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index b32f419176..6230f5e3d4 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -577,6 +577,28 @@ ExecCheckPermissions(List *rangeTable, List *rteperminfos,
ListCell *l;
bool result = true;
+#ifdef USE_ASSERT_CHECKING
+ Bitmapset *indexset = NULL;
+
+ /* Check that rteperminfos is consistent with rangeTable */
+ foreach(l, rangeTable)
+ {
+ RangeTblEntry *rte = lfirst_node(RangeTblEntry, l);
+
+ if (rte->perminfoindex != 0)
+ {
+ /* Sanity checks */
+ (void) getRTEPermissionInfo(rteperminfos, rte);
+ /* Many-to-one mapping not allowed */
+ Assert(!bms_is_member(rte->perminfoindex, indexset));
+ indexset = bms_add_member(indexset, rte->perminfoindex);
+ }
+ }
+
+ /* All rteperminfos are referenced */
+ Assert(bms_num_members(indexset) == list_length(rteperminfos));
+#endif
+
foreach(l, rteperminfos)
{
RTEPermissionInfo *perminfo = lfirst_node(RTEPermissionInfo, l);
diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
index 375b17b9f3..b4e789899e 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -1373,10 +1373,11 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel)
char fkrelname[MAX_QUOTED_REL_NAME_LEN];
char pkattname[MAX_QUOTED_NAME_LEN + 3];
char fkattname[MAX_QUOTED_NAME_LEN + 3];
- RangeTblEntry *pkrte;
- RangeTblEntry *fkrte;
+ RangeTblEntry *rte;
RTEPermissionInfo *pk_perminfo;
RTEPermissionInfo *fk_perminfo;
+ List *rtes = NIL;
+ List *perminfos = NIL;
const char *sep;
const char *fk_only;
const char *pk_only;
@@ -1394,25 +1395,29 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel)
*
* XXX are there any other show-stopper conditions to check?
*/
- pkrte = makeNode(RangeTblEntry);
- pkrte->rtekind = RTE_RELATION;
- pkrte->relid = RelationGetRelid(pk_rel);
- pkrte->relkind = pk_rel->rd_rel->relkind;
- pkrte->rellockmode = AccessShareLock;
-
pk_perminfo = makeNode(RTEPermissionInfo);
pk_perminfo->relid = RelationGetRelid(pk_rel);
pk_perminfo->requiredPerms = ACL_SELECT;
-
- fkrte = makeNode(RangeTblEntry);
- fkrte->rtekind = RTE_RELATION;
- fkrte->relid = RelationGetRelid(fk_rel);
- fkrte->relkind = fk_rel->rd_rel->relkind;
- fkrte->rellockmode = AccessShareLock;
+ perminfos = lappend(perminfos, pk_perminfo);
+ rte = makeNode(RangeTblEntry);
+ rte->rtekind = RTE_RELATION;
+ rte->relid = RelationGetRelid(pk_rel);
+ rte->relkind = pk_rel->rd_rel->relkind;
+ rte->rellockmode = AccessShareLock;
+ rte->perminfoindex = list_length(perminfos);
+ rtes = lappend(rtes, rte);
fk_perminfo = makeNode(RTEPermissionInfo);
fk_perminfo->relid = RelationGetRelid(fk_rel);
fk_perminfo->requiredPerms = ACL_SELECT;
+ perminfos = lappend(perminfos, fk_perminfo);
+ rte = makeNode(RangeTblEntry);
+ rte->rtekind = RTE_RELATION;
+ rte->relid = RelationGetRelid(fk_rel);
+ rte->relkind = fk_rel->rd_rel->relkind;
+ rte->rellockmode = AccessShareLock;
+ rte->perminfoindex = list_length(perminfos);
+ rtes = lappend(rtes, rte);
for (int i = 0; i < riinfo->nkeys; i++)
{
@@ -1425,8 +1430,7 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel)
fk_perminfo->selectedCols = bms_add_member(fk_perminfo->selectedCols, attno);
}
- if (!ExecCheckPermissions(list_make2(fkrte, pkrte),
- list_make2(fk_perminfo, pk_perminfo), false))
+ if (!ExecCheckPermissions(rtes, perminfos, false))
return false;
/*
@@ -1436,9 +1440,9 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel)
*/
if (!has_bypassrls_privilege(GetUserId()) &&
((pk_rel->rd_rel->relrowsecurity &&
- !object_ownercheck(RelationRelationId, pkrte->relid, GetUserId())) ||
+ !object_ownercheck(RelationRelationId, RelationGetRelid(pk_rel), GetUserId())) ||
(fk_rel->rd_rel->relrowsecurity &&
- !object_ownercheck(RelationRelationId, fkrte->relid, GetUserId()))))
+ !object_ownercheck(RelationRelationId, RelationGetRelid(fk_rel), GetUserId()))))
return false;
/*----------
--
2.30.2
