Hello, This didn't apply, so I rebased it on current master, excluding the one I already pushed. No further changes.
-- Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/ "No me acuerdo, pero no es cierto. No es cierto, y si fuera cierto, no me acuerdo." (Augusto Pinochet a una corte de justicia)
>From c8a9bcd071ad88d5ae286bcb1a241b4fccd2449a Mon Sep 17 00:00:00 2001 From: amitlan <amitlangot...@gmail.com> Date: Mon, 28 Nov 2022 16:12:15 +0900 Subject: [PATCH v30 1/2] Add ri_RootToChildMap and ExecGetRootToChildMap() It's a AttrMap provided for converting "root" table column bitmapsets into their child relation counterpart, as a more generalized alternative to using ri_RootToPartitionMap.attrMap to do the same. More generalized in the sense that it can also be requested for regular inheritance child relations, whereas ri_RootToPartitionMap is currently only initialized in tuple-routing "partition" result relations. One of the differences between the two cases is that the regular inheritance child relations can have their own columns that are not present in the "root" table, so the map must be created in a way that ignores such columns. To that end, ExecGetRootToChildMap() passes true for the missing_ok argument of build_attrmap_by_name(), so that it puts 0 (InvalidAttr) in the map for the columns of a child table that are not present in the root table. root-table-to-child-table bitmapset conversions that would need ri_RootToChildMap (cannot be done with ri_RootToPartitionMap) are as of this commit unnecessary, but will become necessary in a subsequent commit that will remove the insertedCols et al bitmapset fields from RangeTblEntry node in favor of a new type of node that will only be created and added to the plan for root tables in a query and never for children. The child table bitmapsets will be created on-the-fly during execution if needed, by copying the root table bitmapset and converting with the aforementioned map as required. --- src/backend/executor/execUtils.c | 39 ++++++++++++++++++++++++++++++++ src/include/executor/executor.h | 2 ++ src/include/nodes/execnodes.h | 8 +++++++ 3 files changed, 49 insertions(+) diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c index 0e595ffa6e..e2b4272d90 100644 --- a/src/backend/executor/execUtils.c +++ b/src/backend/executor/execUtils.c @@ -1252,6 +1252,45 @@ ExecGetChildToRootMap(ResultRelInfo *resultRelInfo) return resultRelInfo->ri_ChildToRootMap; } +/* + * Return the map needed to convert "root" table column bitmapsets to the + * rowtype of an individual child table. A NULL result is valid and means + * that no conversion is needed. + */ +AttrMap * +ExecGetRootToChildMap(ResultRelInfo *resultRelInfo, + EState *estate) +{ + /* If we didn't already do so, compute the map for this child. */ + if (!resultRelInfo->ri_RootToChildMapValid) + { + ResultRelInfo *rootRelInfo = resultRelInfo->ri_RootResultRelInfo; + MemoryContext oldcontext = MemoryContextSwitchTo(estate->es_query_cxt); + + if (rootRelInfo) + { + /* + * Passing 'true' below means any columns present in the child + * table but not in the root parent are to be ignored; note that + * such a case is possible with traditional inheritance but never + * with partitioning. + */ + resultRelInfo->ri_RootToChildMap = + build_attrmap_by_name_if_req(RelationGetDescr(rootRelInfo->ri_RelationDesc), + RelationGetDescr(resultRelInfo->ri_RelationDesc), + true); + } + else /* this isn't a child result rel */ + resultRelInfo->ri_RootToChildMap = NULL; + + resultRelInfo->ri_RootToChildMapValid = true; + + MemoryContextSwitchTo(oldcontext); + } + + return resultRelInfo->ri_RootToChildMap; +} + /* Return a bitmap representing columns being inserted */ Bitmapset * ExecGetInsertedCols(ResultRelInfo *relinfo, EState *estate) diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index ed95ed1176..5c02a1521f 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -600,6 +600,8 @@ extern TupleTableSlot *ExecGetTriggerOldSlot(EState *estate, ResultRelInfo *relI extern TupleTableSlot *ExecGetTriggerNewSlot(EState *estate, ResultRelInfo *relInfo); extern TupleTableSlot *ExecGetReturningSlot(EState *estate, ResultRelInfo *relInfo); extern TupleConversionMap *ExecGetChildToRootMap(ResultRelInfo *resultRelInfo); +extern AttrMap *ExecGetRootToChildMap(ResultRelInfo *resultRelInfo, + EState *estate); extern Bitmapset *ExecGetInsertedCols(ResultRelInfo *relinfo, EState *estate); extern Bitmapset *ExecGetUpdatedCols(ResultRelInfo *relinfo, EState *estate); diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 18e572f171..313840fe32 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -563,6 +563,14 @@ typedef struct ResultRelInfo TupleConversionMap *ri_ChildToRootMap; bool ri_ChildToRootMapValid; + /* + * Map used to convert "root" table column bitmapsets into the ones that + * describe a given child table's columns; see ExecGetInsertedCols() et + * al. Like ri_ChildToRootMap, computed only if needed. + */ + AttrMap *ri_RootToChildMap; + bool ri_RootToChildMapValid; + /* for use by copyfrom.c when performing multi-inserts */ struct CopyMultiInsertBuffer *ri_CopyMultiInsertBuffer; -- 2.30.2
>From de6b5595a0a1c8b5e309b0b9416123efeb501c40 Mon Sep 17 00:00:00 2001 From: amitlan <amitlangot...@gmail.com> Date: Wed, 21 Jul 2021 21:33:19 +0900 Subject: [PATCH v30 2/2] Rework query relation permission checking Currently, information about the permissions to be checked on relations mentioned in a query is stored in their range table entries. So the executor must scan the entire range table looking for relations that need to have permissions checked. This can make the permission checking part of the executor initialization needlessly expensive when many inheritance children are present in the range range. While the permissions need not be checked on the individual child relations, the executor still must visit every range table entry to filter them out. This commit moves the permission checking information out of the range table entries into a new plan node called RTEPermissionInfo. Every top-level (inheritance "root") RTE_RELATION entry in the range table gets one and a list of those is maintained alongside the range table. The list is initialized by the parser when initializing the range table. The rewriter can add more entries to it as rules/views are expanded. Finally, the planner combines the lists of the individual subqueries into one flat list that is passed down to the executor. To make it quick to find the RTEPermissionInfo entry belonging to a given relation, RangeTblEntry gets a new Index field 'perminfoindex' that stores the correponding RTEPermissionInfo's index in the query's list of the latter. --- contrib/postgres_fdw/postgres_fdw.c | 17 +- contrib/sepgsql/dml.c | 42 ++-- contrib/sepgsql/hooks.c | 12 +- contrib/sepgsql/sepgsql.h | 3 +- src/backend/commands/copy.c | 23 ++- src/backend/commands/copyfrom.c | 11 +- src/backend/commands/view.c | 33 ++- src/backend/executor/execMain.c | 123 ++++++----- src/backend/executor/execParallel.c | 1 + src/backend/executor/execUtils.c | 148 ++++++++----- src/backend/nodes/outfuncs.c | 6 +- src/backend/nodes/readfuncs.c | 6 +- src/backend/optimizer/plan/planner.c | 6 + src/backend/optimizer/plan/setrefs.c | 64 ++++-- src/backend/optimizer/plan/subselect.c | 9 +- src/backend/optimizer/prep/prepjointree.c | 19 +- src/backend/optimizer/util/inherit.c | 173 ++++++++++++---- src/backend/optimizer/util/relnode.c | 21 +- src/backend/parser/analyze.c | 68 ++++-- src/backend/parser/parse_clause.c | 9 +- src/backend/parser/parse_merge.c | 9 +- src/backend/parser/parse_relation.c | 178 +++++++++------- src/backend/parser/parse_target.c | 18 +- src/backend/parser/parse_utilcmd.c | 6 +- src/backend/replication/logical/worker.c | 11 +- src/backend/rewrite/rewriteDefine.c | 27 +-- src/backend/rewrite/rewriteHandler.c | 195 +++++++++--------- src/backend/rewrite/rewriteManip.c | 34 +++ src/backend/rewrite/rowsecurity.c | 25 ++- src/backend/utils/adt/ri_triggers.c | 19 +- src/backend/utils/cache/relcache.c | 4 +- src/include/commands/copyfrom_internal.h | 3 +- src/include/executor/executor.h | 13 +- src/include/nodes/execnodes.h | 3 +- src/include/nodes/parsenodes.h | 94 ++++++--- src/include/nodes/pathnodes.h | 3 + src/include/nodes/plannodes.h | 3 + src/include/optimizer/inherit.h | 2 + src/include/parser/parse_node.h | 9 +- src/include/parser/parse_relation.h | 4 + src/include/rewrite/rewriteHandler.h | 1 + src/include/rewrite/rewriteManip.h | 3 + .../modules/test_oat_hooks/test_oat_hooks.c | 12 +- src/test/regress/expected/rules.out | 14 ++ src/test/regress/sql/rules.sql | 15 ++ src/tools/pgindent/typedefs.list | 2 + 46 files changed, 960 insertions(+), 541 deletions(-) diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c index 20c7b1ad05..1ceac2e0cf 100644 --- a/contrib/postgres_fdw/postgres_fdw.c +++ b/contrib/postgres_fdw/postgres_fdw.c @@ -31,6 +31,7 @@ #include "optimizer/appendinfo.h" #include "optimizer/clauses.h" #include "optimizer/cost.h" +#include "optimizer/inherit.h" #include "optimizer/optimizer.h" #include "optimizer/pathnode.h" #include "optimizer/paths.h" @@ -657,8 +658,8 @@ postgresGetForeignRelSize(PlannerInfo *root, /* * If the table or the server is configured to use remote estimates, * identify which user to do remote access as during planning. This - * should match what ExecCheckRTEPerms() does. If we fail due to lack of - * permissions, the query would have failed at runtime anyway. + * should match what ExecCheckPermissions() does. If we fail due to lack + * of permissions, the query would have failed at runtime anyway. */ if (fpinfo->use_remote_estimate) { @@ -1809,7 +1810,8 @@ postgresPlanForeignModify(PlannerInfo *root, else if (operation == CMD_UPDATE) { int col; - Bitmapset *allUpdatedCols = bms_union(rte->updatedCols, rte->extraUpdatedCols); + RelOptInfo *rel = find_base_rel(root, resultRelation); + Bitmapset *allUpdatedCols = get_rel_all_updated_cols(root, rel); col = -1; while ((col = bms_next_member(allUpdatedCols, col)) >= 0) @@ -2650,7 +2652,7 @@ postgresBeginDirectModify(ForeignScanState *node, int eflags) /* * Identify which user to do the remote access as. This should match what - * ExecCheckRTEPerms() does. + * ExecCheckPermissions() does. */ userid = OidIsValid(fsplan->checkAsUser) ? fsplan->checkAsUser : GetUserId(); @@ -3975,11 +3977,8 @@ create_foreign_modify(EState *estate, fmstate = (PgFdwModifyState *) palloc0(sizeof(PgFdwModifyState)); fmstate->rel = rel; - /* - * Identify which user to do the remote access as. This should match what - * ExecCheckRTEPerms() does. - */ - userid = OidIsValid(rte->checkAsUser) ? rte->checkAsUser : GetUserId(); + /* Identify which user to do the remote access as. */ + userid = ExecGetResultRelCheckAsUser(resultRelInfo, estate); /* Get info about foreign table. */ table = GetForeignTable(RelationGetRelid(rel)); diff --git a/contrib/sepgsql/dml.c b/contrib/sepgsql/dml.c index 3bb98dfb06..3509358fbe 100644 --- a/contrib/sepgsql/dml.c +++ b/contrib/sepgsql/dml.c @@ -23,6 +23,7 @@ #include "commands/tablecmds.h" #include "executor/executor.h" #include "nodes/bitmapset.h" +#include "parser/parsetree.h" #include "sepgsql.h" #include "utils/lsyscache.h" #include "utils/syscache.h" @@ -277,38 +278,33 @@ check_relation_privileges(Oid relOid, * Entrypoint of the DML permission checks */ bool -sepgsql_dml_privileges(List *rangeTabls, bool abort_on_violation) +sepgsql_dml_privileges(List *rangeTbls, List *rtepermlist, + bool abort_on_violation) { ListCell *lr; - foreach(lr, rangeTabls) + foreach(lr, rtepermlist) { - RangeTblEntry *rte = lfirst(lr); + RTEPermissionInfo *perminfo = lfirst_node(RTEPermissionInfo, lr); uint32 required = 0; List *tableIds; ListCell *li; - /* - * Only regular relations shall be checked - */ - if (rte->rtekind != RTE_RELATION) - continue; - /* * Find out required permissions */ - if (rte->requiredPerms & ACL_SELECT) + if (perminfo->requiredPerms & ACL_SELECT) required |= SEPG_DB_TABLE__SELECT; - if (rte->requiredPerms & ACL_INSERT) + if (perminfo->requiredPerms & ACL_INSERT) required |= SEPG_DB_TABLE__INSERT; - if (rte->requiredPerms & ACL_UPDATE) + if (perminfo->requiredPerms & ACL_UPDATE) { - if (!bms_is_empty(rte->updatedCols)) + if (!bms_is_empty(perminfo->updatedCols)) required |= SEPG_DB_TABLE__UPDATE; else required |= SEPG_DB_TABLE__LOCK; } - if (rte->requiredPerms & ACL_DELETE) + if (perminfo->requiredPerms & ACL_DELETE) required |= SEPG_DB_TABLE__DELETE; /* @@ -323,10 +319,10 @@ sepgsql_dml_privileges(List *rangeTabls, bool abort_on_violation) * expand rte->relid into list of OIDs of inheritance hierarchy, then * checker routine will be invoked for each relations. */ - if (!rte->inh) - tableIds = list_make1_oid(rte->relid); + if (!perminfo->inh) + tableIds = list_make1_oid(perminfo->relid); else - tableIds = find_all_inheritors(rte->relid, NoLock, NULL); + tableIds = find_all_inheritors(perminfo->relid, NoLock, NULL); foreach(li, tableIds) { @@ -339,12 +335,12 @@ sepgsql_dml_privileges(List *rangeTabls, bool abort_on_violation) * child table has different attribute numbers, so we need to fix * up them. */ - selectedCols = fixup_inherited_columns(rte->relid, tableOid, - rte->selectedCols); - insertedCols = fixup_inherited_columns(rte->relid, tableOid, - rte->insertedCols); - updatedCols = fixup_inherited_columns(rte->relid, tableOid, - rte->updatedCols); + selectedCols = fixup_inherited_columns(perminfo->relid, tableOid, + perminfo->selectedCols); + insertedCols = fixup_inherited_columns(perminfo->relid, tableOid, + perminfo->insertedCols); + updatedCols = fixup_inherited_columns(perminfo->relid, tableOid, + perminfo->updatedCols); /* * check permissions on individual tables diff --git a/contrib/sepgsql/hooks.c b/contrib/sepgsql/hooks.c index 363ac06700..6e7f96e7db 100644 --- a/contrib/sepgsql/hooks.c +++ b/contrib/sepgsql/hooks.c @@ -35,7 +35,7 @@ PG_MODULE_MAGIC; * Saved hook entries (if stacked) */ static object_access_hook_type next_object_access_hook = NULL; -static ExecutorCheckPerms_hook_type next_exec_check_perms_hook = NULL; +static ExecutorCheckPermissions_hook_type next_exec_check_perms_hook = NULL; static ProcessUtility_hook_type next_ProcessUtility_hook = NULL; /* @@ -287,17 +287,17 @@ sepgsql_object_access(ObjectAccessType access, * Entrypoint of DML permissions */ static bool -sepgsql_exec_check_perms(List *rangeTabls, bool abort) +sepgsql_exec_check_perms(List *rangeTbls, List *rtepermlist, bool abort) { /* * If security provider is stacking and one of them replied 'false' at * least, we don't need to check any more. */ if (next_exec_check_perms_hook && - !(*next_exec_check_perms_hook) (rangeTabls, abort)) + !(*next_exec_check_perms_hook) (rangeTbls, rtepermlist, abort)) return false; - if (!sepgsql_dml_privileges(rangeTabls, abort)) + if (!sepgsql_dml_privileges(rangeTbls, rtepermlist, abort)) return false; return true; @@ -471,8 +471,8 @@ _PG_init(void) object_access_hook = sepgsql_object_access; /* DML permission check */ - next_exec_check_perms_hook = ExecutorCheckPerms_hook; - ExecutorCheckPerms_hook = sepgsql_exec_check_perms; + next_exec_check_perms_hook = ExecutorCheckPermissions_hook; + ExecutorCheckPermissions_hook = sepgsql_exec_check_perms; /* ProcessUtility hook */ next_ProcessUtility_hook = ProcessUtility_hook; diff --git a/contrib/sepgsql/sepgsql.h b/contrib/sepgsql/sepgsql.h index f2a2c795bf..7aa6df92ec 100644 --- a/contrib/sepgsql/sepgsql.h +++ b/contrib/sepgsql/sepgsql.h @@ -274,7 +274,8 @@ extern void sepgsql_object_relabel(const ObjectAddress *object, /* * dml.c */ -extern bool sepgsql_dml_privileges(List *rangeTabls, bool abort_on_violation); +extern bool sepgsql_dml_privileges(List *rangeTabls, List *rtepermlist, + bool abort_on_violation); /* * database.c diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c index db4c9dbc23..b8bd78d358 100644 --- a/src/backend/commands/copy.c +++ b/src/backend/commands/copy.c @@ -109,7 +109,7 @@ DoCopy(ParseState *pstate, const CopyStmt *stmt, { LOCKMODE lockmode = is_from ? RowExclusiveLock : AccessShareLock; ParseNamespaceItem *nsitem; - RangeTblEntry *rte; + RTEPermissionInfo *perminfo; TupleDesc tupDesc; List *attnums; ListCell *cur; @@ -123,8 +123,9 @@ DoCopy(ParseState *pstate, const CopyStmt *stmt, nsitem = addRangeTableEntryForRelation(pstate, rel, lockmode, NULL, false, false); - rte = nsitem->p_rte; - rte->requiredPerms = (is_from ? ACL_INSERT : ACL_SELECT); + + perminfo = nsitem->p_perminfo; + perminfo->requiredPerms = (is_from ? ACL_INSERT : ACL_SELECT); if (stmt->whereClause) { @@ -150,15 +151,15 @@ DoCopy(ParseState *pstate, const CopyStmt *stmt, attnums = CopyGetAttnums(tupDesc, rel, stmt->attlist); foreach(cur, attnums) { - int attno = lfirst_int(cur) - - FirstLowInvalidHeapAttributeNumber; + int attno; + Bitmapset **bms; - if (is_from) - rte->insertedCols = bms_add_member(rte->insertedCols, attno); - else - rte->selectedCols = bms_add_member(rte->selectedCols, attno); + attno = lfirst_int(cur) - FirstLowInvalidHeapAttributeNumber; + bms = is_from ? &perminfo->insertedCols : &perminfo->selectedCols; + + *bms = bms_add_member(*bms, attno); } - ExecCheckRTPerms(pstate->p_rtable, true); + ExecCheckPermissions(pstate->p_rtable, list_make1(perminfo), true); /* * Permission check for row security policies. @@ -174,7 +175,7 @@ DoCopy(ParseState *pstate, const CopyStmt *stmt, * If RLS is not enabled for this, then just fall through to the * normal non-filtering relation handling. */ - if (check_enable_rls(rte->relid, InvalidOid, false) == RLS_ENABLED) + if (check_enable_rls(relid, InvalidOid, false) == RLS_ENABLED) { SelectStmt *select; ColumnRef *cr; diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c index a079c70152..03f8ec459a 100644 --- a/src/backend/commands/copyfrom.c +++ b/src/backend/commands/copyfrom.c @@ -761,6 +761,12 @@ CopyFrom(CopyFromState cstate) resultRelInfo = target_resultRelInfo = makeNode(ResultRelInfo); ExecInitResultRelation(estate, resultRelInfo, 1); + /* + * Copy the RTEPermissionInfos into estate as well, so that + * ExecGetInsertedCols() et al will work correctly. + */ + estate->es_rtepermlist = cstate->rtepermlist; + /* Verify the named relation is a valid target for INSERT */ CheckValidResultRel(resultRelInfo, CMD_INSERT); @@ -1525,9 +1531,12 @@ BeginCopyFrom(ParseState *pstate, initStringInfo(&cstate->attribute_buf); - /* Assign range table, we'll need it in CopyFrom. */ + /* Assign range table and rtepermlist, we'll need them in CopyFrom. */ if (pstate) + { cstate->range_table = pstate->p_rtable; + cstate->rtepermlist = pstate->p_rtepermlist; + } tupDesc = RelationGetDescr(cstate->rel); num_phys_attrs = tupDesc->natts; diff --git a/src/backend/commands/view.c b/src/backend/commands/view.c index b5a0fc02e5..cdd73e148e 100644 --- a/src/backend/commands/view.c +++ b/src/backend/commands/view.c @@ -367,7 +367,7 @@ DefineViewRules(Oid viewOid, Query *viewParse, bool replace) * by 2... * * These extra RT entries are not actually used in the query, - * except for run-time locking and permission checking. + * except for run-time locking. *--------------------------------------------------------------- */ static Query * @@ -378,7 +378,9 @@ UpdateRangeTableOfViewParse(Oid viewOid, Query *viewParse) ParseNamespaceItem *nsitem; RangeTblEntry *rt_entry1, *rt_entry2; + RTEPermissionInfo *rte_perminfo1; ParseState *pstate; + ListCell *lc; /* * Make a copy of the given parsetree. It's not so much that we don't @@ -405,15 +407,38 @@ UpdateRangeTableOfViewParse(Oid viewOid, Query *viewParse) makeAlias("old", NIL), false, false); rt_entry1 = nsitem->p_rte; + rte_perminfo1 = nsitem->p_perminfo; nsitem = addRangeTableEntryForRelation(pstate, viewRel, AccessShareLock, makeAlias("new", NIL), false, false); rt_entry2 = nsitem->p_rte; - /* Must override addRangeTableEntry's default access-check flags */ - rt_entry1->requiredPerms = 0; - rt_entry2->requiredPerms = 0; + /* + * Add only the "old" RTEPermissionInfo at the head of view query's list + * and update the other RTEs' perminfoindex accordingly. When rewriting a + * query on the view, ApplyRetrieveRule() will transfer the view + * relation's permission details into this RTEPermissionInfo. That's + * needed because the view's RTE itself will be transposed into a subquery + * RTE that can't carry the permission details; see the code stanza toward + * the end of ApplyRetrieveRule() for how that's done. + */ + viewParse->rtepermlist = lcons(rte_perminfo1, viewParse->rtepermlist); + foreach(lc, viewParse->rtable) + { + RangeTblEntry *rte = lfirst(lc); + + if (rte->perminfoindex > 0) + rte->perminfoindex += 1; + } + + /* + * Also make the "new" RTE's RTEPermissionInfo undiscoverable. This is a + * bit of a hack given that all the non-child RTE_RELATION entries really + * should have a RTEPermissionInfo, but this dummy "new" RTE is going to + * go away anyway in the very near future. + */ + rt_entry2->perminfoindex = 0; new_rt = lcons(rt_entry1, lcons(rt_entry2, viewParse->rtable)); diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index 8bf2ba1c04..f536863a15 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -54,6 +54,7 @@ #include "jit/jit.h" #include "mb/pg_wchar.h" #include "miscadmin.h" +#include "parser/parse_relation.h" #include "parser/parsetree.h" #include "storage/bufmgr.h" #include "storage/lmgr.h" @@ -74,8 +75,8 @@ ExecutorRun_hook_type ExecutorRun_hook = NULL; ExecutorFinish_hook_type ExecutorFinish_hook = NULL; ExecutorEnd_hook_type ExecutorEnd_hook = NULL; -/* Hook for plugin to get control in ExecCheckRTPerms() */ -ExecutorCheckPerms_hook_type ExecutorCheckPerms_hook = NULL; +/* Hook for plugin to get control in ExecCheckPermissions() */ +ExecutorCheckPermissions_hook_type ExecutorCheckPermissions_hook = NULL; /* decls for local routines only used within this module */ static void InitPlan(QueryDesc *queryDesc, int eflags); @@ -90,10 +91,10 @@ static void ExecutePlan(EState *estate, PlanState *planstate, ScanDirection direction, DestReceiver *dest, bool execute_once); -static bool ExecCheckRTEPerms(RangeTblEntry *rte); -static bool ExecCheckRTEPermsModified(Oid relOid, Oid userid, - Bitmapset *modifiedCols, - AclMode requiredPerms); +static bool ExecCheckOneRelPerms(RTEPermissionInfo *perminfo); +static bool ExecCheckPermissionsModified(Oid relOid, Oid userid, + Bitmapset *modifiedCols, + AclMode requiredPerms); static void ExecCheckXactReadOnly(PlannedStmt *plannedstmt); static char *ExecBuildSlotValueDescription(Oid reloid, TupleTableSlot *slot, @@ -554,8 +555,8 @@ ExecutorRewind(QueryDesc *queryDesc) /* - * ExecCheckRTPerms - * Check access permissions for all relations listed in a range table. + * ExecCheckPermissions + * Check access permissions of relations mentioned in a query * * Returns true if permissions are adequate. Otherwise, throws an appropriate * error if ereport_on_violation is true, or simply returns false otherwise. @@ -565,73 +566,65 @@ ExecutorRewind(QueryDesc *queryDesc) * passing, then RLS also needs to be consulted (and check_enable_rls()). * * See rewrite/rowsecurity.c. + * + * NB: rangeTable is no longer used by us, but kept around for the hooks that + * might still want to look at the RTEs. */ bool -ExecCheckRTPerms(List *rangeTable, bool ereport_on_violation) +ExecCheckPermissions(List *rangeTable, List *rtepermlist, + bool ereport_on_violation) { ListCell *l; bool result = true; - foreach(l, rangeTable) + foreach(l, rtepermlist) { - RangeTblEntry *rte = (RangeTblEntry *) lfirst(l); + RTEPermissionInfo *perminfo = lfirst_node(RTEPermissionInfo, l); - result = ExecCheckRTEPerms(rte); + Assert(OidIsValid(perminfo->relid)); + result = ExecCheckOneRelPerms(perminfo); if (!result) { - Assert(rte->rtekind == RTE_RELATION); if (ereport_on_violation) - aclcheck_error(ACLCHECK_NO_PRIV, get_relkind_objtype(get_rel_relkind(rte->relid)), - get_rel_name(rte->relid)); + aclcheck_error(ACLCHECK_NO_PRIV, + get_relkind_objtype(get_rel_relkind(perminfo->relid)), + get_rel_name(perminfo->relid)); return false; } } - if (ExecutorCheckPerms_hook) - result = (*ExecutorCheckPerms_hook) (rangeTable, - ereport_on_violation); + if (ExecutorCheckPermissions_hook) + result = (*ExecutorCheckPermissions_hook) (rangeTable, rtepermlist, + ereport_on_violation); return result; } /* - * ExecCheckRTEPerms - * Check access permissions for a single RTE. + * ExecCheckOneRelPerms + * Check access permissions for a single relation. */ static bool -ExecCheckRTEPerms(RangeTblEntry *rte) +ExecCheckOneRelPerms(RTEPermissionInfo *perminfo) { AclMode requiredPerms; AclMode relPerms; AclMode remainingPerms; - Oid relOid; Oid userid; + Oid relOid = perminfo->relid; - /* - * Only plain-relation RTEs need to be checked here. Function RTEs are - * checked when the function is prepared for execution. Join, subquery, - * and special RTEs need no checks. - */ - if (rte->rtekind != RTE_RELATION) - return true; - - /* - * No work if requiredPerms is empty. - */ - requiredPerms = rte->requiredPerms; - if (requiredPerms == 0) - return true; - - relOid = rte->relid; + requiredPerms = perminfo->requiredPerms; + Assert(requiredPerms != 0); /* * userid to check as: current user unless we have a setuid indication. * * Note: GetUserId() is presently fast enough that there's no harm in - * calling it separately for each RTE. If that stops being true, we could - * call it once in ExecCheckRTPerms and pass the userid down from there. - * But for now, no need for the extra clutter. + * calling it separately for each relation. If that stops being true, we + * could call it once in ExecCheckPermissions and pass the userid down + * from there. But for now, no need for the extra clutter. */ - userid = OidIsValid(rte->checkAsUser) ? rte->checkAsUser : GetUserId(); + userid = OidIsValid(perminfo->checkAsUser) ? + perminfo->checkAsUser : GetUserId(); /* * We must have *all* the requiredPerms bits, but some of the bits can be @@ -665,14 +658,14 @@ ExecCheckRTEPerms(RangeTblEntry *rte) * 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(rte->selectedCols)) + 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(rte->selectedCols, col)) >= 0) + while ((col = bms_next_member(perminfo->selectedCols, col)) >= 0) { /* bit #s are offset by FirstLowInvalidHeapAttributeNumber */ AttrNumber attno = col + FirstLowInvalidHeapAttributeNumber; @@ -697,29 +690,31 @@ ExecCheckRTEPerms(RangeTblEntry *rte) * Basically the same for the mod columns, for both INSERT and UPDATE * privilege as specified by remainingPerms. */ - if (remainingPerms & ACL_INSERT && !ExecCheckRTEPermsModified(relOid, - userid, - rte->insertedCols, - ACL_INSERT)) + if (remainingPerms & ACL_INSERT && + !ExecCheckPermissionsModified(relOid, + userid, + perminfo->insertedCols, + ACL_INSERT)) return false; - if (remainingPerms & ACL_UPDATE && !ExecCheckRTEPermsModified(relOid, - userid, - rte->updatedCols, - ACL_UPDATE)) + if (remainingPerms & ACL_UPDATE && + !ExecCheckPermissionsModified(relOid, + userid, + perminfo->updatedCols, + ACL_UPDATE)) return false; } return true; } /* - * ExecCheckRTEPermsModified - * Check INSERT or UPDATE access permissions for a single RTE (these + * ExecCheckPermissionsModified + * Check INSERT or UPDATE access permissions for a single relation (these * are processed uniformly). */ static bool -ExecCheckRTEPermsModified(Oid relOid, Oid userid, Bitmapset *modifiedCols, - AclMode requiredPerms) +ExecCheckPermissionsModified(Oid relOid, Oid userid, Bitmapset *modifiedCols, + AclMode requiredPerms) { int col = -1; @@ -773,17 +768,14 @@ ExecCheckXactReadOnly(PlannedStmt *plannedstmt) * Fail if write permissions are requested in parallel mode for table * (temp or non-temp), otherwise fail for any non-temp table. */ - foreach(l, plannedstmt->rtable) + foreach(l, plannedstmt->permInfos) { - RangeTblEntry *rte = (RangeTblEntry *) lfirst(l); + RTEPermissionInfo *perminfo = lfirst_node(RTEPermissionInfo, l); - if (rte->rtekind != RTE_RELATION) + if ((perminfo->requiredPerms & (~ACL_SELECT)) == 0) continue; - if ((rte->requiredPerms & (~ACL_SELECT)) == 0) - continue; - - if (isTempNamespace(get_rel_namespace(rte->relid))) + if (isTempNamespace(get_rel_namespace(perminfo->relid))) continue; PreventCommandIfReadOnly(CreateCommandName((Node *) plannedstmt)); @@ -815,9 +807,10 @@ InitPlan(QueryDesc *queryDesc, int eflags) int i; /* - * Do permissions checks + * Do permissions checks and save the list for later use. */ - ExecCheckRTPerms(rangeTable, true); + ExecCheckPermissions(rangeTable, plannedstmt->permInfos, true); + estate->es_rtepermlist = plannedstmt->permInfos; /* * initialize the node's execution state diff --git a/src/backend/executor/execParallel.c b/src/backend/executor/execParallel.c index 99512826c5..2ee84f7612 100644 --- a/src/backend/executor/execParallel.c +++ b/src/backend/executor/execParallel.c @@ -184,6 +184,7 @@ ExecSerializePlan(Plan *plan, EState *estate) pstmt->parallelModeNeeded = false; pstmt->planTree = plan; pstmt->rtable = estate->es_range_table; + pstmt->permInfos = estate->es_rtepermlist; pstmt->resultRelations = NIL; pstmt->appendRelations = NIL; diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c index e2b4272d90..8e9375aa6a 100644 --- a/src/backend/executor/execUtils.c +++ b/src/backend/executor/execUtils.c @@ -57,6 +57,7 @@ #include "miscadmin.h" #include "nodes/nodeFuncs.h" #include "parser/parsetree.h" +#include "parser/parse_relation.h" #include "partitioning/partdesc.h" #include "storage/lmgr.h" #include "utils/builtins.h" @@ -67,6 +68,7 @@ static bool tlist_matches_tupdesc(PlanState *ps, List *tlist, int varno, TupleDesc tupdesc); static void ShutdownExprContext(ExprContext *econtext, bool isCommit); +static inline RTEPermissionInfo *GetResultRTEPermissionInfo(ResultRelInfo *relinfo, EState *estate); /* ---------------------------------------------------------------- @@ -1295,72 +1297,48 @@ ExecGetRootToChildMap(ResultRelInfo *resultRelInfo, Bitmapset * ExecGetInsertedCols(ResultRelInfo *relinfo, EState *estate) { - /* - * The columns are stored in the range table entry. If this ResultRelInfo - * represents a partition routing target, and doesn't have an entry of its - * own in the range table, fetch the parent's RTE and map the columns to - * the order they are in the partition. - */ - if (relinfo->ri_RangeTableIndex != 0) - { - RangeTblEntry *rte = exec_rt_fetch(relinfo->ri_RangeTableIndex, estate); + RTEPermissionInfo *perminfo = GetResultRTEPermissionInfo(relinfo, estate); - return rte->insertedCols; - } - else if (relinfo->ri_RootResultRelInfo) - { - ResultRelInfo *rootRelInfo = relinfo->ri_RootResultRelInfo; - RangeTblEntry *rte = exec_rt_fetch(rootRelInfo->ri_RangeTableIndex, estate); - - if (relinfo->ri_RootToPartitionMap != NULL) - return execute_attr_map_cols(relinfo->ri_RootToPartitionMap->attrMap, - rte->insertedCols); - else - return rte->insertedCols; - } - else - { - /* - * The relation isn't in the range table and it isn't a partition - * routing target. This ResultRelInfo must've been created only for - * firing triggers and the relation is not being inserted into. (See - * ExecGetTriggerResultRel.) - */ + if (perminfo == NULL) return NULL; + + /* Map the columns to child's attribute numbers if needed. */ + if (relinfo->ri_RootResultRelInfo) + { + AttrMap *map = ExecGetRootToChildMap(relinfo, estate); + + if (map) + return execute_attr_map_cols(map, perminfo->insertedCols); } + + return perminfo->insertedCols; } /* Return a bitmap representing columns being updated */ Bitmapset * ExecGetUpdatedCols(ResultRelInfo *relinfo, EState *estate) { - /* see ExecGetInsertedCols() */ - if (relinfo->ri_RangeTableIndex != 0) - { - RangeTblEntry *rte = exec_rt_fetch(relinfo->ri_RangeTableIndex, estate); + RTEPermissionInfo *perminfo = GetResultRTEPermissionInfo(relinfo, estate); - return rte->updatedCols; - } - else if (relinfo->ri_RootResultRelInfo) - { - ResultRelInfo *rootRelInfo = relinfo->ri_RootResultRelInfo; - RangeTblEntry *rte = exec_rt_fetch(rootRelInfo->ri_RangeTableIndex, estate); - - if (relinfo->ri_RootToPartitionMap != NULL) - return execute_attr_map_cols(relinfo->ri_RootToPartitionMap->attrMap, - rte->updatedCols); - else - return rte->updatedCols; - } - else + if (perminfo == NULL) return NULL; + + /* Map the columns to child's attribute numbers if needed. */ + if (relinfo->ri_RootResultRelInfo) + { + AttrMap *map = ExecGetRootToChildMap(relinfo, estate); + + if (map) + return execute_attr_map_cols(map, perminfo->updatedCols); + } + + return perminfo->updatedCols; } /* Return a bitmap representing generated columns being updated */ Bitmapset * ExecGetExtraUpdatedCols(ResultRelInfo *relinfo, EState *estate) { - /* see ExecGetInsertedCols() */ if (relinfo->ri_RangeTableIndex != 0) { RangeTblEntry *rte = exec_rt_fetch(relinfo->ri_RangeTableIndex, estate); @@ -1389,3 +1367,75 @@ ExecGetAllUpdatedCols(ResultRelInfo *relinfo, EState *estate) return bms_union(ExecGetUpdatedCols(relinfo, estate), ExecGetExtraUpdatedCols(relinfo, estate)); } + +/* + * GetResultRTEPermissionInfo + * Helper routine for ExecGet*Cols() routines + * + * The column bitmapsets are stored in RTEPermissionInfos. For inheritance + * child result relations (a partition routing target of an INSERT or a child + * UPDATE target), use the root parent's RTE to fetch the RTEPermissionInfo + * because that's the only one that actually points to any. + */ +static inline RTEPermissionInfo * +GetResultRTEPermissionInfo(ResultRelInfo *relinfo, EState *estate) +{ + Index rti; + RangeTblEntry *rte; + RTEPermissionInfo *perminfo = NULL; + + if (relinfo->ri_RootResultRelInfo) + rti = relinfo->ri_RootResultRelInfo->ri_RangeTableIndex; + else if (relinfo->ri_RangeTableIndex != 0) + rti = relinfo->ri_RangeTableIndex; + else + { + /* + * The relation isn't in the range table and it isn't a partition + * routing target. This ResultRelInfo must've been created only for + * firing triggers and the relation is not being inserted into. (See + * ExecGetTriggerResultRel.) + */ + rti = 0; + } + + if (rti > 0) + { + rte = exec_rt_fetch(rti, estate); + perminfo = ExecGetRTEPermissionInfo(rte, estate); + } + + return perminfo; +} + +/* + * ExecGetRTEPermissionInfo + * Returns the RTEPermissionInfo contained in estate->es_rtepermlist + * pointed to by the RTE + */ +RTEPermissionInfo * +ExecGetRTEPermissionInfo(RangeTblEntry *rte, EState *estate) +{ + Assert(estate->es_rtepermlist != NIL); + return GetRTEPermissionInfo(estate->es_rtepermlist, rte); +} + +/* + * GetResultRelCheckAsUser + * Returns the user to modify passed-in result relation as + * + * The user is chosen by looking up the relation's or, if a child table, its + * root parent's RTEPermissionInfo. + */ +Oid +ExecGetResultRelCheckAsUser(ResultRelInfo *relInfo, EState *estate) +{ + RTEPermissionInfo *perminfo = GetResultRTEPermissionInfo(relInfo, estate); + + /* XXX - maybe ok to return GetUserId() in this case? */ + if (perminfo == NULL) + elog(ERROR, "no RTEPermissionInfo found for result relation with OID %u", + RelationGetRelid(relInfo->ri_RelationDesc)); + + return perminfo->checkAsUser ? perminfo->checkAsUser : GetUserId(); +} diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index 8f150e9a2e..59b0fdeb62 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -507,6 +507,7 @@ _outRangeTblEntry(StringInfo str, const RangeTblEntry *node) WRITE_CHAR_FIELD(relkind); WRITE_INT_FIELD(rellockmode); WRITE_NODE_FIELD(tablesample); + WRITE_UINT_FIELD(perminfoindex); break; case RTE_SUBQUERY: WRITE_NODE_FIELD(subquery); @@ -560,11 +561,6 @@ _outRangeTblEntry(StringInfo str, const RangeTblEntry *node) WRITE_BOOL_FIELD(lateral); WRITE_BOOL_FIELD(inh); WRITE_BOOL_FIELD(inFromCl); - WRITE_UINT64_FIELD(requiredPerms); - WRITE_OID_FIELD(checkAsUser); - WRITE_BITMAPSET_FIELD(selectedCols); - WRITE_BITMAPSET_FIELD(insertedCols); - WRITE_BITMAPSET_FIELD(updatedCols); WRITE_BITMAPSET_FIELD(extraUpdatedCols); WRITE_NODE_FIELD(securityQuals); } diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c index 23776367c5..966b75f5a6 100644 --- a/src/backend/nodes/readfuncs.c +++ b/src/backend/nodes/readfuncs.c @@ -473,6 +473,7 @@ _readRangeTblEntry(void) READ_CHAR_FIELD(relkind); READ_INT_FIELD(rellockmode); READ_NODE_FIELD(tablesample); + READ_UINT_FIELD(perminfoindex); break; case RTE_SUBQUERY: READ_NODE_FIELD(subquery); @@ -536,11 +537,6 @@ _readRangeTblEntry(void) READ_BOOL_FIELD(lateral); READ_BOOL_FIELD(inh); READ_BOOL_FIELD(inFromCl); - READ_UINT_FIELD(requiredPerms); - READ_OID_FIELD(checkAsUser); - READ_BITMAPSET_FIELD(selectedCols); - READ_BITMAPSET_FIELD(insertedCols); - READ_BITMAPSET_FIELD(updatedCols); READ_BITMAPSET_FIELD(extraUpdatedCols); READ_NODE_FIELD(securityQuals); diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index 493a3af0fa..0bf772d4ee 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -57,6 +57,7 @@ #include "optimizer/tlist.h" #include "parser/analyze.h" #include "parser/parse_agg.h" +#include "parser/parse_relation.h" #include "parser/parsetree.h" #include "partitioning/partdesc.h" #include "rewrite/rewriteManip.h" @@ -306,6 +307,7 @@ standard_planner(Query *parse, const char *query_string, int cursorOptions, glob->subroots = NIL; glob->rewindPlanIDs = NULL; glob->finalrtable = NIL; + glob->finalrtepermlist = NIL; glob->finalrowmarks = NIL; glob->resultRelations = NIL; glob->appendRelations = NIL; @@ -493,6 +495,7 @@ standard_planner(Query *parse, const char *query_string, int cursorOptions, /* final cleanup of the plan */ Assert(glob->finalrtable == NIL); + Assert(glob->finalrtepermlist == NIL); Assert(glob->finalrowmarks == NIL); Assert(glob->resultRelations == NIL); Assert(glob->appendRelations == NIL); @@ -520,6 +523,7 @@ standard_planner(Query *parse, const char *query_string, int cursorOptions, result->parallelModeNeeded = glob->parallelModeNeeded; result->planTree = top_plan; result->rtable = glob->finalrtable; + result->permInfos = glob->finalrtepermlist; result->resultRelations = glob->resultRelations; result->appendRelations = glob->appendRelations; result->subplans = glob->subplans; @@ -6265,6 +6269,7 @@ plan_cluster_use_sort(Oid tableOid, Oid indexOid) rte->inh = false; rte->inFromCl = true; query->rtable = list_make1(rte); + AddRTEPermissionInfo(&query->rtepermlist, rte); /* Set up RTE/RelOptInfo arrays */ setup_simple_rel_arrays(root); @@ -6392,6 +6397,7 @@ plan_create_index_workers(Oid tableOid, Oid indexOid) rte->inh = true; rte->inFromCl = true; query->rtable = list_make1(rte); + AddRTEPermissionInfo(&query->rtepermlist, rte); /* Set up RTE/RelOptInfo arrays */ setup_simple_rel_arrays(root); diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c index 1cb0abdbc1..e9e30617ad 100644 --- a/src/backend/optimizer/plan/setrefs.c +++ b/src/backend/optimizer/plan/setrefs.c @@ -24,6 +24,7 @@ #include "optimizer/planmain.h" #include "optimizer/planner.h" #include "optimizer/tlist.h" +#include "parser/parse_relation.h" #include "tcop/utility.h" #include "utils/lsyscache.h" #include "utils/syscache.h" @@ -78,6 +79,13 @@ typedef struct int newvarno; } fix_windowagg_cond_context; +/* Context info for flatten_rtes_walker() */ +typedef struct +{ + PlannerGlobal *glob; + Query *query; +} flatten_rtes_walker_context; + /* * Selecting the best alternative in an AlternativeSubPlan expression requires * estimating how many times that expression will be evaluated. For an @@ -113,8 +121,9 @@ typedef struct static void add_rtes_to_flat_rtable(PlannerInfo *root, bool recursing); static void flatten_unplanned_rtes(PlannerGlobal *glob, RangeTblEntry *rte); -static bool flatten_rtes_walker(Node *node, PlannerGlobal *glob); -static void add_rte_to_flat_rtable(PlannerGlobal *glob, RangeTblEntry *rte); +static bool flatten_rtes_walker(Node *node, flatten_rtes_walker_context *cxt); +static void add_rte_to_flat_rtable(PlannerGlobal *glob, List *rtepermlist, + RangeTblEntry *rte); static Plan *set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset); static Plan *set_indexonlyscan_references(PlannerInfo *root, IndexOnlyScan *plan, @@ -355,6 +364,9 @@ set_plan_references(PlannerInfo *root, Plan *plan) * Extract RangeTblEntries from the plan's rangetable, and add to flat rtable * * This can recurse into subquery plans; "recursing" is true if so. + * + * This also seems like a good place to add the query's RTEPermissionInfos to + * the flat rtepermlist. */ static void add_rtes_to_flat_rtable(PlannerInfo *root, bool recursing) @@ -375,7 +387,7 @@ add_rtes_to_flat_rtable(PlannerInfo *root, bool recursing) RangeTblEntry *rte = (RangeTblEntry *) lfirst(lc); if (!recursing || rte->rtekind == RTE_RELATION) - add_rte_to_flat_rtable(glob, rte); + add_rte_to_flat_rtable(glob, root->parse->rtepermlist, rte); } /* @@ -442,18 +454,21 @@ add_rtes_to_flat_rtable(PlannerInfo *root, bool recursing) /* * Extract RangeTblEntries from a subquery that was never planned at all */ + static void flatten_unplanned_rtes(PlannerGlobal *glob, RangeTblEntry *rte) { + flatten_rtes_walker_context cxt = {glob, rte->subquery}; + /* Use query_tree_walker to find all RTEs in the parse tree */ (void) query_tree_walker(rte->subquery, flatten_rtes_walker, - (void *) glob, + (void *) &cxt, QTW_EXAMINE_RTES_BEFORE); } static bool -flatten_rtes_walker(Node *node, PlannerGlobal *glob) +flatten_rtes_walker(Node *node, flatten_rtes_walker_context *cxt) { if (node == NULL) return false; @@ -463,33 +478,38 @@ flatten_rtes_walker(Node *node, PlannerGlobal *glob) /* As above, we need only save relation RTEs */ if (rte->rtekind == RTE_RELATION) - add_rte_to_flat_rtable(glob, rte); + add_rte_to_flat_rtable(cxt->glob, cxt->query->rtepermlist, rte); return false; } if (IsA(node, Query)) { - /* Recurse into subselects */ + /* + * Recurse into subselects. Must update cxt->query to this query so + * that the rtable and rtepermlist correspond with each other. + */ + cxt->query = (Query *) node; return query_tree_walker((Query *) node, flatten_rtes_walker, - (void *) glob, + (void *) cxt, QTW_EXAMINE_RTES_BEFORE); } return expression_tree_walker(node, flatten_rtes_walker, - (void *) glob); + (void *) cxt); } /* - * Add (a copy of) the given RTE to the final rangetable + * Add (a copy of) the given RTE to the final rangetable and also the + * corresponding RTEPermissionInfo, if any, to final rtepermlist. * * In the flat rangetable, we zero out substructure pointers that are not * needed by the executor; this reduces the storage space and copying cost - * for cached plans. We keep only the ctename, alias and eref Alias fields, - * which are needed by EXPLAIN, and the selectedCols, insertedCols, - * updatedCols, and extraUpdatedCols bitmaps, which are needed for - * executor-startup permissions checking and for trigger event checking. + * for cached plans. We keep only the ctename, alias, eref Alias fields, + * which are needed by EXPLAIN, and perminfoindex which is needed by the + * executor to fetch the RTE's RTEPermissionInfo. */ static void -add_rte_to_flat_rtable(PlannerGlobal *glob, RangeTblEntry *rte) +add_rte_to_flat_rtable(PlannerGlobal *glob, List *rtepermlist, + RangeTblEntry *rte) { RangeTblEntry *newrte; @@ -527,6 +547,20 @@ add_rte_to_flat_rtable(PlannerGlobal *glob, RangeTblEntry *rte) */ if (newrte->rtekind == RTE_RELATION) glob->relationOids = lappend_oid(glob->relationOids, newrte->relid); + + /* + * Add the RTEPermissionInfo, if any, corresponding to this RTE to the + * flattened global list. Also update the perminfoindex in newrte to + * reflect the RTEPermissionInfo's position in this other list. + */ + if (rte->perminfoindex > 0) + { + RTEPermissionInfo *perminfo; + + perminfo = GetRTEPermissionInfo(rtepermlist, newrte); + glob->finalrtepermlist = lappend(glob->finalrtepermlist, perminfo); + newrte->perminfoindex = list_length(glob->finalrtepermlist); + } } /* diff --git a/src/backend/optimizer/plan/subselect.c b/src/backend/optimizer/plan/subselect.c index 92e3338584..bee4072301 100644 --- a/src/backend/optimizer/plan/subselect.c +++ b/src/backend/optimizer/plan/subselect.c @@ -1496,8 +1496,13 @@ convert_EXISTS_sublink_to_join(PlannerInfo *root, SubLink *sublink, if (!bms_is_subset(upper_varnos, available_rels)) return NULL; - /* Now we can attach the modified subquery rtable to the parent */ - parse->rtable = list_concat(parse->rtable, subselect->rtable); + /* + * Now we can attach the modified subquery rtable to the parent. This also + * adds subquery's RTEPermissionInfos into the upper query. + */ + parse->rtable = CombineRangeTables(parse->rtable, subselect->rtable, + subselect->rtepermlist, + &parse->rtepermlist); /* * And finally, build the JoinExpr node. diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c index 2ea3ca734e..140af439f2 100644 --- a/src/backend/optimizer/prep/prepjointree.c +++ b/src/backend/optimizer/prep/prepjointree.c @@ -176,13 +176,6 @@ transform_MERGE_to_join(Query *parse) joinrte->lateral = false; joinrte->inh = false; joinrte->inFromCl = true; - joinrte->requiredPerms = 0; - joinrte->checkAsUser = InvalidOid; - joinrte->selectedCols = NULL; - joinrte->insertedCols = NULL; - joinrte->updatedCols = NULL; - joinrte->extraUpdatedCols = NULL; - joinrte->securityQuals = NIL; /* * Add completed RTE to pstate's range table list, so that we know its @@ -1209,8 +1202,12 @@ pull_up_simple_subquery(PlannerInfo *root, Node *jtnode, RangeTblEntry *rte, * Now append the adjusted rtable entries to upper query. (We hold off * until after fixing the upper rtable entries; no point in running that * code on the subquery ones too.) + * + * This also adds subquery's RTEPermissionInfos into the upper query. */ - parse->rtable = list_concat(parse->rtable, subquery->rtable); + parse->rtable = CombineRangeTables(parse->rtable, subquery->rtable, + subquery->rtepermlist, + &parse->rtepermlist); /* * Pull up any FOR UPDATE/SHARE markers, too. (OffsetVarNodes already @@ -1347,8 +1344,12 @@ pull_up_simple_union_all(PlannerInfo *root, Node *jtnode, RangeTblEntry *rte) /* * Append child RTEs to parent rtable. + * + * This also adds subquery's RTEPermissionInfos into the upper query. */ - root->parse->rtable = list_concat(root->parse->rtable, rtable); + root->parse->rtable = CombineRangeTables(root->parse->rtable, rtable, + subquery->rtepermlist, + &root->parse->rtepermlist); /* * Recursively scan the subquery's setOperations tree and add diff --git a/src/backend/optimizer/util/inherit.c b/src/backend/optimizer/util/inherit.c index 3d270e91d6..b22422b166 100644 --- a/src/backend/optimizer/util/inherit.c +++ b/src/backend/optimizer/util/inherit.c @@ -30,6 +30,7 @@ #include "optimizer/prep.h" #include "optimizer/restrictinfo.h" #include "parser/parsetree.h" +#include "parser/parse_relation.h" #include "partitioning/partdesc.h" #include "partitioning/partprune.h" #include "utils/rel.h" @@ -38,6 +39,7 @@ static void expand_partitioned_rtentry(PlannerInfo *root, RelOptInfo *relinfo, RangeTblEntry *parentrte, Index parentRTindex, Relation parentrel, + Bitmapset *parent_updatedCols, PlanRowMark *top_parentrc, LOCKMODE lockmode); static void expand_single_inheritance_child(PlannerInfo *root, RangeTblEntry *parentrte, @@ -47,6 +49,10 @@ static void expand_single_inheritance_child(PlannerInfo *root, Index *childRTindex_p); static Bitmapset *translate_col_privs(const Bitmapset *parent_privs, List *translated_vars); +static Bitmapset *translate_col_privs_multilevel(PlannerInfo *root, + RelOptInfo *rel, + RelOptInfo *top_parent_rel, + Bitmapset *top_parent_cols); static void expand_appendrel_subquery(PlannerInfo *root, RelOptInfo *rel, RangeTblEntry *rte, Index rti); @@ -131,6 +137,10 @@ expand_inherited_rtentry(PlannerInfo *root, RelOptInfo *rel, /* Scan the inheritance set and expand it */ if (oldrelation->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) { + RTEPermissionInfo *perminfo; + + perminfo = GetRTEPermissionInfo(root->parse->rtepermlist, rte); + /* * Partitioned table, so set up for partitioning. */ @@ -141,7 +151,9 @@ expand_inherited_rtentry(PlannerInfo *root, RelOptInfo *rel, * extract the partition key columns of all the partitioned tables. */ expand_partitioned_rtentry(root, rel, rte, rti, - oldrelation, oldrc, lockmode); + oldrelation, + perminfo->updatedCols, + oldrc, lockmode); } else { @@ -305,6 +317,7 @@ static void expand_partitioned_rtentry(PlannerInfo *root, RelOptInfo *relinfo, RangeTblEntry *parentrte, Index parentRTindex, Relation parentrel, + Bitmapset *parent_updatedCols, PlanRowMark *top_parentrc, LOCKMODE lockmode) { PartitionDesc partdesc; @@ -324,14 +337,13 @@ expand_partitioned_rtentry(PlannerInfo *root, RelOptInfo *relinfo, /* * Note down whether any partition key cols are being updated. Though it's - * the root partitioned table's updatedCols we are interested in, we - * instead use parentrte to get the updatedCols. This is convenient - * because parentrte already has the root partrel's updatedCols translated - * to match the attribute ordering of parentrel. + * the root partitioned table's updatedCols we are interested in, + * parent_updatedCols provided by the caller contains the root partrel's + * updatedCols translated to match the attribute ordering of parentrel. */ if (!root->partColsUpdated) root->partColsUpdated = - has_partition_attrs(parentrel, parentrte->updatedCols, NULL); + has_partition_attrs(parentrel, parent_updatedCols, NULL); /* * There shouldn't be any generated columns in the partition key. @@ -402,9 +414,19 @@ expand_partitioned_rtentry(PlannerInfo *root, RelOptInfo *relinfo, /* If this child is itself partitioned, recurse */ if (childrel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) + { + AppendRelInfo *appinfo = root->append_rel_array[childRTindex]; + Bitmapset *child_updatedCols; + + child_updatedCols = translate_col_privs(parent_updatedCols, + appinfo->translated_vars); + expand_partitioned_rtentry(root, childrelinfo, childrte, childRTindex, - childrel, top_parentrc, lockmode); + childrel, + child_updatedCols, + top_parentrc, lockmode); + } /* Close child relation, but keep locks */ table_close(childrel, NoLock); @@ -451,17 +473,15 @@ expand_single_inheritance_child(PlannerInfo *root, RangeTblEntry *parentrte, /* * Build an RTE for the child, and attach to query's rangetable list. We * copy most scalar fields of the parent's RTE, but replace relation OID, - * relkind, and inh for the child. Also, set requiredPerms to zero since - * all required permissions checks are done on the original RTE. Likewise, - * set the child's securityQuals to empty, because we only want to apply - * the parent's RLS conditions regardless of what RLS properties - * individual children may have. (This is an intentional choice to make - * inherited RLS work like regular permissions checks.) The parent - * securityQuals will be propagated to children along with other base - * restriction clauses, so we don't need to do it here. Other - * infrastructure of the parent RTE has to be translated to match the - * child table's column ordering, which we do below, so a "flat" copy is - * sufficient to start with. + * relkind, and inh for the child. Set the child's securityQuals to + * empty, because we only want to apply the parent's RLS conditions + * regardless of what RLS properties individual children may have. + * (This is an intentional choice to make inherited RLS work like regular + * permissions checks.) The parent securityQuals will be propagated to + * children along with other base restriction clauses, so we don't need + * to do it here. Other infrastructure of the parent RTE has to be + * translated to match the child table's column ordering, which we do + * below, so a "flat" copy is sufficient to start with. */ childrte = makeNode(RangeTblEntry); memcpy(childrte, parentrte, sizeof(RangeTblEntry)); @@ -476,9 +496,16 @@ expand_single_inheritance_child(PlannerInfo *root, RangeTblEntry *parentrte, } else childrte->inh = false; - childrte->requiredPerms = 0; childrte->securityQuals = NIL; + /* + * No permission checking for the child RTE unless it's the parent + * relation in its child role, which only applies to traditional + * inheritance. + */ + if (childOID != parentOID) + childrte->perminfoindex = 0; + /* Link not-yet-fully-filled child RTE into data structures */ parse->rtable = lappend(parse->rtable, childrte); childRTindex = list_length(parse->rtable); @@ -539,33 +566,12 @@ expand_single_inheritance_child(PlannerInfo *root, RangeTblEntry *parentrte, childrte->alias = childrte->eref = makeAlias(parentrte->eref->aliasname, child_colnames); - /* - * Translate the column permissions bitmaps to the child's attnums (we - * have to build the translated_vars list before we can do this). But if - * this is the parent table, we can just duplicate the parent's bitmaps. - * - * Note: we need to do this even though the executor won't run any - * permissions checks on the child RTE. The insertedCols/updatedCols - * bitmaps may be examined for trigger-firing purposes. - */ + /* Translate the bitmapset of generated columns being updated. */ if (childOID != parentOID) - { - childrte->selectedCols = translate_col_privs(parentrte->selectedCols, - appinfo->translated_vars); - childrte->insertedCols = translate_col_privs(parentrte->insertedCols, - appinfo->translated_vars); - childrte->updatedCols = translate_col_privs(parentrte->updatedCols, - appinfo->translated_vars); childrte->extraUpdatedCols = translate_col_privs(parentrte->extraUpdatedCols, appinfo->translated_vars); - } else - { - childrte->selectedCols = bms_copy(parentrte->selectedCols); - childrte->insertedCols = bms_copy(parentrte->insertedCols); - childrte->updatedCols = bms_copy(parentrte->updatedCols); childrte->extraUpdatedCols = bms_copy(parentrte->extraUpdatedCols); - } /* * Store the RTE and appinfo in the respective PlannerInfo arrays, which @@ -648,6 +654,54 @@ expand_single_inheritance_child(PlannerInfo *root, RangeTblEntry *parentrte, } } +/* + * get_rel_all_updated_cols + * Returns the set of columns of a given "simple" relation that are + * updated by this query. + */ +Bitmapset * +get_rel_all_updated_cols(PlannerInfo *root, RelOptInfo *rel) +{ + Index relid; + RangeTblEntry *rte; + RTEPermissionInfo *perminfo; + Bitmapset *updatedCols, + *extraUpdatedCols; + + Assert(root->parse->commandType == CMD_UPDATE); + Assert(IS_SIMPLE_REL(rel)); + + /* + * We obtain updatedCols and extraUpdatedCols for the query's result + * relation. Then, if necessary, we map it to the column numbers of the + * relation for which they were requested. + */ + relid = root->parse->resultRelation; + rte = planner_rt_fetch(relid, root); + perminfo = GetRTEPermissionInfo(root->parse->rtepermlist, rte); + + updatedCols = perminfo->updatedCols; + extraUpdatedCols = rte->extraUpdatedCols; + + /* + * For "other" rels, we must look up the root parent relation mentioned in + * the query, and translate the column numbers. + */ + if (rel->relid != relid) + { + RelOptInfo *top_parent_rel = find_base_rel(root, relid); + + Assert(IS_OTHER_REL(rel)); + + updatedCols = translate_col_privs_multilevel(root, rel, top_parent_rel, + updatedCols); + extraUpdatedCols = translate_col_privs_multilevel(root, rel, top_parent_rel, + extraUpdatedCols); + } + + return bms_union(updatedCols, extraUpdatedCols); +} + /* * translate_col_privs * Translate a bitmapset representing per-column privileges from the @@ -866,3 +920,40 @@ apply_child_basequals(PlannerInfo *root, RelOptInfo *parentrel, return true; } + +/* + * translate_col_privs_multilevel + * Recursively translates the column numbers contained in + * 'top_parent_cols' to the columns numbers of a descendent relation + * given by 'relid' + */ +static Bitmapset * +translate_col_privs_multilevel(PlannerInfo *root, RelOptInfo *rel, + RelOptInfo *top_parent_rel, + Bitmapset *top_parent_cols) +{ + Bitmapset *result; + AppendRelInfo *appinfo; + + if (top_parent_cols == NULL) + return NULL; + + /* Recurse if immediate parent is not the top parent. */ + if (rel->parent != top_parent_rel) + { + if (rel->parent) + result = translate_col_privs_multilevel(root, rel->parent, + top_parent_rel, + top_parent_cols); + else + elog(ERROR, "rel with relid %u is not a child rel", rel->relid); + } + + Assert(root->append_rel_array != NULL); + appinfo = root->append_rel_array[rel->relid]; + Assert(appinfo != NULL); + + result = translate_col_privs(top_parent_cols, appinfo->translated_vars); + + return result; +} diff --git a/src/backend/optimizer/util/relnode.c b/src/backend/optimizer/util/relnode.c index d7b4434e7f..800fe490d9 100644 --- a/src/backend/optimizer/util/relnode.c +++ b/src/backend/optimizer/util/relnode.c @@ -28,6 +28,7 @@ #include "optimizer/plancat.h" #include "optimizer/restrictinfo.h" #include "optimizer/tlist.h" +#include "parser/parse_relation.h" #include "utils/hsearch.h" #include "utils/lsyscache.h" @@ -223,7 +224,25 @@ build_simple_rel(PlannerInfo *root, int relid, RelOptInfo *parent) rel->rel_parallel_workers = -1; /* set up in get_relation_info */ rel->amflags = 0; rel->serverid = InvalidOid; - rel->userid = rte->checkAsUser; + if (rte->rtekind == RTE_RELATION) + { + /* + * Get the userid from the relation's RTEPermissionInfo, though only + * the tables mentioned in query are assigned RTEPermissionInfos. + * Child relations (otherrels) simply use the parent's value. + */ + if (parent == NULL) + { + RTEPermissionInfo *perminfo; + + perminfo = GetRTEPermissionInfo(root->parse->rtepermlist, rte); + rel->userid = perminfo->checkAsUser; + } + else + rel->userid = parent->userid; + } + else + rel->userid = InvalidOid; rel->useridiscurrent = false; rel->fdwroutine = NULL; rel->fdw_private = NULL; diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c index 6688c2a865..40473fe86f 100644 --- a/src/backend/parser/analyze.c +++ b/src/backend/parser/analyze.c @@ -518,6 +518,7 @@ transformDeleteStmt(ParseState *pstate, DeleteStmt *stmt) /* done building the range table and jointree */ qry->rtable = pstate->p_rtable; + qry->rtepermlist = pstate->p_rtepermlist; qry->jointree = makeFromExpr(pstate->p_joinlist, qual); qry->hasSubLinks = pstate->p_hasSubLinks; @@ -546,11 +547,12 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt) List *exprList = NIL; bool isGeneralSelect; List *sub_rtable; + List *sub_rtepermlist; List *sub_namespace; List *icolumns; List *attrnos; ParseNamespaceItem *nsitem; - RangeTblEntry *rte; + RTEPermissionInfo *perminfo; ListCell *icols; ListCell *attnos; ListCell *lc; @@ -594,17 +596,19 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt) /* * If a non-nil rangetable/namespace was passed in, and we are doing - * INSERT/SELECT, arrange to pass the rangetable/namespace down to the - * SELECT. This can only happen if we are inside a CREATE RULE, and in - * that case we want the rule's OLD and NEW rtable entries to appear as - * part of the SELECT's rtable, not as outer references for it. (Kluge!) - * The SELECT's joinlist is not affected however. We must do this before - * adding the target table to the INSERT's rtable. + * INSERT/SELECT, arrange to pass the rangetable/rtepermlist/namespace + * down to the SELECT. This can only happen if we are inside a CREATE + * RULE, and in that case we want the rule's OLD and NEW rtable entries to + * appear as part of the SELECT's rtable, not as outer references for it. + * (Kluge!) The SELECT's joinlist is not affected however. We must do + * this before adding the target table to the INSERT's rtable. */ if (isGeneralSelect) { sub_rtable = pstate->p_rtable; pstate->p_rtable = NIL; + sub_rtepermlist = pstate->p_rtepermlist; + pstate->p_rtepermlist = NIL; sub_namespace = pstate->p_namespace; pstate->p_namespace = NIL; } @@ -669,6 +673,7 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt) * the target column's type, which we handle below. */ sub_pstate->p_rtable = sub_rtable; + sub_pstate->p_rtepermlist = sub_rtepermlist; sub_pstate->p_joinexprs = NIL; /* sub_rtable has no joins */ sub_pstate->p_namespace = sub_namespace; sub_pstate->p_resolve_unknowns = false; @@ -894,7 +899,7 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt) * Generate query's target list using the computed list of expressions. * Also, mark all the target columns as needing insert permissions. */ - rte = pstate->p_target_nsitem->p_rte; + perminfo = pstate->p_target_nsitem->p_perminfo; qry->targetList = NIL; Assert(list_length(exprList) <= list_length(icolumns)); forthree(lc, exprList, icols, icolumns, attnos, attrnos) @@ -910,8 +915,8 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt) false); qry->targetList = lappend(qry->targetList, tle); - rte->insertedCols = bms_add_member(rte->insertedCols, - attr_num - FirstLowInvalidHeapAttributeNumber); + perminfo->insertedCols = bms_add_member(perminfo->insertedCols, + attr_num - FirstLowInvalidHeapAttributeNumber); } /* @@ -938,6 +943,7 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt) /* done building the range table and jointree */ qry->rtable = pstate->p_rtable; + qry->rtepermlist = pstate->p_rtepermlist; qry->jointree = makeFromExpr(pstate->p_joinlist, NULL); qry->hasTargetSRFs = pstate->p_hasTargetSRFs; @@ -1096,8 +1102,6 @@ transformOnConflictClause(ParseState *pstate, * (We'll check the actual target relation, instead.) */ exclRte->relkind = RELKIND_COMPOSITE_TYPE; - exclRte->requiredPerms = 0; - /* other permissions fields in exclRte are already empty */ /* Create EXCLUDED rel's targetlist for use by EXPLAIN */ exclRelTlist = BuildOnConflictExcludedTargetlist(targetrel, @@ -1391,6 +1395,7 @@ transformSelectStmt(ParseState *pstate, SelectStmt *stmt) resolveTargetListUnknowns(pstate, qry->targetList); qry->rtable = pstate->p_rtable; + qry->rtepermlist = pstate->p_rtepermlist; qry->jointree = makeFromExpr(pstate->p_joinlist, qual); qry->hasSubLinks = pstate->p_hasSubLinks; @@ -1619,6 +1624,7 @@ transformValuesClause(ParseState *pstate, SelectStmt *stmt) linitial(stmt->lockingClause))->strength)))); qry->rtable = pstate->p_rtable; + qry->rtepermlist = pstate->p_rtepermlist; qry->jointree = makeFromExpr(pstate->p_joinlist, NULL); qry->hasSubLinks = pstate->p_hasSubLinks; @@ -1865,6 +1871,7 @@ transformSetOperationStmt(ParseState *pstate, SelectStmt *stmt) qry->limitOption = stmt->limitOption; qry->rtable = pstate->p_rtable; + qry->rtepermlist = pstate->p_rtepermlist; qry->jointree = makeFromExpr(pstate->p_joinlist, NULL); qry->hasSubLinks = pstate->p_hasSubLinks; @@ -2339,6 +2346,7 @@ transformReturnStmt(ParseState *pstate, ReturnStmt *stmt) if (pstate->p_resolve_unknowns) resolveTargetListUnknowns(pstate, qry->targetList); qry->rtable = pstate->p_rtable; + qry->rtepermlist = pstate->p_rtepermlist; qry->jointree = makeFromExpr(pstate->p_joinlist, NULL); qry->hasSubLinks = pstate->p_hasSubLinks; qry->hasWindowFuncs = pstate->p_hasWindowFuncs; @@ -2405,6 +2413,7 @@ transformUpdateStmt(ParseState *pstate, UpdateStmt *stmt) qry->targetList = transformUpdateTargetList(pstate, stmt->targetList); qry->rtable = pstate->p_rtable; + qry->rtepermlist = pstate->p_rtepermlist; qry->jointree = makeFromExpr(pstate->p_joinlist, qual); qry->hasTargetSRFs = pstate->p_hasTargetSRFs; @@ -2423,7 +2432,7 @@ List * transformUpdateTargetList(ParseState *pstate, List *origTlist) { List *tlist = NIL; - RangeTblEntry *target_rte; + RTEPermissionInfo *target_perminfo; ListCell *orig_tl; ListCell *tl; @@ -2435,7 +2444,7 @@ transformUpdateTargetList(ParseState *pstate, List *origTlist) pstate->p_next_resno = RelationGetNumberOfAttributes(pstate->p_target_relation) + 1; /* Prepare non-junk columns for assignment to target table */ - target_rte = pstate->p_target_nsitem->p_rte; + target_perminfo = pstate->p_target_nsitem->p_perminfo; orig_tl = list_head(origTlist); foreach(tl, tlist) @@ -2476,8 +2485,8 @@ transformUpdateTargetList(ParseState *pstate, List *origTlist) origTarget->location); /* Mark the target column as requiring update permissions */ - target_rte->updatedCols = bms_add_member(target_rte->updatedCols, - attrno - FirstLowInvalidHeapAttributeNumber); + target_perminfo->updatedCols = bms_add_member(target_perminfo->updatedCols, + attrno - FirstLowInvalidHeapAttributeNumber); orig_tl = lnext(origTlist, orig_tl); } @@ -2764,6 +2773,7 @@ transformPLAssignStmt(ParseState *pstate, PLAssignStmt *stmt) &qry->targetList); qry->rtable = pstate->p_rtable; + qry->rtepermlist = pstate->p_rtepermlist; qry->jointree = makeFromExpr(pstate->p_joinlist, qual); qry->hasSubLinks = pstate->p_hasSubLinks; @@ -3242,9 +3252,16 @@ transformLockingClause(ParseState *pstate, Query *qry, LockingClause *lc, switch (rte->rtekind) { case RTE_RELATION: - applyLockingClause(qry, i, lc->strength, lc->waitPolicy, - pushedDown); - rte->requiredPerms |= ACL_SELECT_FOR_UPDATE; + { + RTEPermissionInfo *perminfo; + + applyLockingClause(qry, i, + lc->strength, + lc->waitPolicy, + pushedDown); + perminfo = GetRTEPermissionInfo(qry->rtepermlist, rte); + perminfo->requiredPerms |= ACL_SELECT_FOR_UPDATE; + } break; case RTE_SUBQUERY: applyLockingClause(qry, i, lc->strength, lc->waitPolicy, @@ -3324,9 +3341,16 @@ transformLockingClause(ParseState *pstate, Query *qry, LockingClause *lc, switch (rte->rtekind) { case RTE_RELATION: - applyLockingClause(qry, i, lc->strength, - lc->waitPolicy, pushedDown); - rte->requiredPerms |= ACL_SELECT_FOR_UPDATE; + { + RTEPermissionInfo *perminfo; + + applyLockingClause(qry, i, + lc->strength, + lc->waitPolicy, + pushedDown); + perminfo = GetRTEPermissionInfo(qry->rtepermlist, rte); + perminfo->requiredPerms |= ACL_SELECT_FOR_UPDATE; + } break; case RTE_SUBQUERY: applyLockingClause(qry, i, lc->strength, diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c index e01c0734d1..856839f379 100644 --- a/src/backend/parser/parse_clause.c +++ b/src/backend/parser/parse_clause.c @@ -225,7 +225,7 @@ setTargetTable(ParseState *pstate, RangeVar *relation, * analysis, we will add the ACL_SELECT bit back again; see * markVarForSelectPriv and its callers. */ - nsitem->p_rte->requiredPerms = requiredPerms; + nsitem->p_perminfo->requiredPerms = requiredPerms; /* * If UPDATE/DELETE, add table to joinlist and namespace. @@ -3226,16 +3226,17 @@ transformOnConflictArbiter(ParseState *pstate, if (infer->conname) { Oid relid = RelationGetRelid(pstate->p_target_relation); - RangeTblEntry *rte = pstate->p_target_nsitem->p_rte; + RTEPermissionInfo *perminfo = pstate->p_target_nsitem->p_perminfo; Bitmapset *conattnos; conattnos = get_relation_constraint_attnos(relid, infer->conname, false, constraint); /* Make sure the rel as a whole is marked for SELECT access */ - rte->requiredPerms |= ACL_SELECT; + perminfo->requiredPerms |= ACL_SELECT; /* Mark the constrained columns as requiring SELECT access */ - rte->selectedCols = bms_add_members(rte->selectedCols, conattnos); + perminfo->selectedCols = bms_add_members(perminfo->selectedCols, + conattnos); } } diff --git a/src/backend/parser/parse_merge.c b/src/backend/parser/parse_merge.c index 62c2ff69f0..225bf7561e 100644 --- a/src/backend/parser/parse_merge.c +++ b/src/backend/parser/parse_merge.c @@ -215,6 +215,7 @@ transformMergeStmt(ParseState *pstate, MergeStmt *stmt) */ qry->targetList = NIL; qry->rtable = pstate->p_rtable; + qry->rtepermlist = pstate->p_rtepermlist; /* * Transform the join condition. This includes references to the target @@ -287,7 +288,7 @@ transformMergeStmt(ParseState *pstate, MergeStmt *stmt) { List *exprList = NIL; ListCell *lc; - RangeTblEntry *rte; + RTEPermissionInfo *perminfo; ListCell *icols; ListCell *attnos; List *icolumns; @@ -346,7 +347,7 @@ transformMergeStmt(ParseState *pstate, MergeStmt *stmt) * of expressions. Also, mark all the target columns as * needing insert permissions. */ - rte = pstate->p_target_nsitem->p_rte; + perminfo = pstate->p_target_nsitem->p_perminfo; forthree(lc, exprList, icols, icolumns, attnos, attrnos) { Expr *expr = (Expr *) lfirst(lc); @@ -360,8 +361,8 @@ transformMergeStmt(ParseState *pstate, MergeStmt *stmt) false); action->targetList = lappend(action->targetList, tle); - rte->insertedCols = - bms_add_member(rte->insertedCols, + perminfo->insertedCols = + bms_add_member(perminfo->insertedCols, attr_num - FirstLowInvalidHeapAttributeNumber); } } diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c index 4665f0b2b7..fab8083dbb 100644 --- a/src/backend/parser/parse_relation.c +++ b/src/backend/parser/parse_relation.c @@ -1037,11 +1037,15 @@ markRTEForSelectPriv(ParseState *pstate, int rtindex, AttrNumber col) if (rte->rtekind == RTE_RELATION) { + RTEPermissionInfo *perminfo; + /* Make sure the rel as a whole is marked for SELECT access */ - rte->requiredPerms |= ACL_SELECT; + perminfo = GetRTEPermissionInfo(pstate->p_rtepermlist, rte); + perminfo->requiredPerms |= ACL_SELECT; /* Must offset the attnum to fit in a bitmapset */ - rte->selectedCols = bms_add_member(rte->selectedCols, - col - FirstLowInvalidHeapAttributeNumber); + perminfo->selectedCols = + bms_add_member(perminfo->selectedCols, + col - FirstLowInvalidHeapAttributeNumber); } else if (rte->rtekind == RTE_JOIN) { @@ -1251,10 +1255,13 @@ chooseScalarFunctionAlias(Node *funcexpr, char *funcname, * * rte: the new RangeTblEntry for the rel * rtindex: its index in the rangetable list + * perminfo: permission list entry for the rel * tupdesc: the physical column information */ static ParseNamespaceItem * -buildNSItemFromTupleDesc(RangeTblEntry *rte, Index rtindex, TupleDesc tupdesc) +buildNSItemFromTupleDesc(RangeTblEntry *rte, Index rtindex, + RTEPermissionInfo *perminfo, + TupleDesc tupdesc) { ParseNamespaceItem *nsitem; ParseNamespaceColumn *nscolumns; @@ -1290,6 +1297,7 @@ buildNSItemFromTupleDesc(RangeTblEntry *rte, Index rtindex, TupleDesc tupdesc) nsitem->p_names = rte->eref; nsitem->p_rte = rte; nsitem->p_rtindex = rtindex; + nsitem->p_perminfo = perminfo; nsitem->p_nscolumns = nscolumns; /* set default visibility flags; might get changed later */ nsitem->p_rel_visible = true; @@ -1433,6 +1441,7 @@ addRangeTableEntry(ParseState *pstate, bool inFromCl) { RangeTblEntry *rte = makeNode(RangeTblEntry); + RTEPermissionInfo *perminfo; char *refname = alias ? alias->aliasname : relation->relname; LOCKMODE lockmode; Relation rel; @@ -1469,7 +1478,7 @@ addRangeTableEntry(ParseState *pstate, buildRelationAliases(rel->rd_att, alias, rte->eref); /* - * Set flags and access permissions. + * Set flags and initialize access permissions. * * The initial default on access checks is always check-for-READ-access, * which is the right thing for all except target tables. @@ -1478,12 +1487,8 @@ addRangeTableEntry(ParseState *pstate, rte->inh = inh; rte->inFromCl = inFromCl; - rte->requiredPerms = ACL_SELECT; - rte->checkAsUser = InvalidOid; /* not set-uid by default, either */ - rte->selectedCols = NULL; - rte->insertedCols = NULL; - rte->updatedCols = NULL; - rte->extraUpdatedCols = NULL; + perminfo = AddRTEPermissionInfo(&pstate->p_rtepermlist, rte); + perminfo->requiredPerms = ACL_SELECT; /* * Add completed RTE to pstate's range table list, so that we know its @@ -1497,7 +1502,7 @@ addRangeTableEntry(ParseState *pstate, * list --- caller must do that if appropriate. */ nsitem = buildNSItemFromTupleDesc(rte, list_length(pstate->p_rtable), - rel->rd_att); + perminfo, rel->rd_att); /* * Drop the rel refcount, but keep the access lock till end of transaction @@ -1534,6 +1539,7 @@ addRangeTableEntryForRelation(ParseState *pstate, bool inFromCl) { RangeTblEntry *rte = makeNode(RangeTblEntry); + RTEPermissionInfo *perminfo; char *refname = alias ? alias->aliasname : RelationGetRelationName(rel); Assert(pstate != NULL); @@ -1557,7 +1563,7 @@ addRangeTableEntryForRelation(ParseState *pstate, buildRelationAliases(rel->rd_att, alias, rte->eref); /* - * Set flags and access permissions. + * Set flags and initialize access permissions. * * The initial default on access checks is always check-for-READ-access, * which is the right thing for all except target tables. @@ -1566,12 +1572,8 @@ addRangeTableEntryForRelation(ParseState *pstate, rte->inh = inh; rte->inFromCl = inFromCl; - rte->requiredPerms = ACL_SELECT; - rte->checkAsUser = InvalidOid; /* not set-uid by default, either */ - rte->selectedCols = NULL; - rte->insertedCols = NULL; - rte->updatedCols = NULL; - rte->extraUpdatedCols = NULL; + perminfo = AddRTEPermissionInfo(&pstate->p_rtepermlist, rte); + perminfo->requiredPerms = ACL_SELECT; /* * Add completed RTE to pstate's range table list, so that we know its @@ -1585,7 +1587,7 @@ addRangeTableEntryForRelation(ParseState *pstate, * list --- caller must do that if appropriate. */ return buildNSItemFromTupleDesc(rte, list_length(pstate->p_rtable), - rel->rd_att); + perminfo, rel->rd_att); } /* @@ -1659,21 +1661,15 @@ addRangeTableEntryForSubquery(ParseState *pstate, rte->eref = eref; /* - * Set flags and access permissions. + * Set flags. * - * Subqueries are never checked for access rights. + * Subqueries are never checked for access rights, so no need to perform + * AddRTEPermissionInfo(). */ rte->lateral = lateral; rte->inh = false; /* never true for subqueries */ rte->inFromCl = inFromCl; - rte->requiredPerms = 0; - rte->checkAsUser = InvalidOid; - rte->selectedCols = NULL; - rte->insertedCols = NULL; - rte->updatedCols = NULL; - rte->extraUpdatedCols = NULL; - /* * Add completed RTE to pstate's range table list, so that we know its * index. But we don't add it to the join list --- caller must do that if @@ -1990,20 +1986,13 @@ addRangeTableEntryForFunction(ParseState *pstate, /* * Set flags and access permissions. * - * Functions are never checked for access rights (at least, not by the RTE - * permissions mechanism). + * Functions are never checked for access rights (at least, not by + * ExecCheckPermissions()), so no need to perform AddRelPermissionsInfo(). */ rte->lateral = lateral; rte->inh = false; /* never true for functions */ rte->inFromCl = inFromCl; - rte->requiredPerms = 0; - rte->checkAsUser = InvalidOid; - rte->selectedCols = NULL; - rte->insertedCols = NULL; - rte->updatedCols = NULL; - rte->extraUpdatedCols = NULL; - /* * Add completed RTE to pstate's range table list, so that we know its * index. But we don't add it to the join list --- caller must do that if @@ -2015,7 +2004,7 @@ addRangeTableEntryForFunction(ParseState *pstate, * Build a ParseNamespaceItem, but don't add it to the pstate's namespace * list --- caller must do that if appropriate. */ - return buildNSItemFromTupleDesc(rte, list_length(pstate->p_rtable), + return buildNSItemFromTupleDesc(rte, list_length(pstate->p_rtable), NULL, tupdesc); } @@ -2082,20 +2071,13 @@ addRangeTableEntryForTableFunc(ParseState *pstate, /* * Set flags and access permissions. * - * Tablefuncs are never checked for access rights (at least, not by the - * RTE permissions mechanism). + * Tablefuncs are never checked for access rights (at least, not by + * ExecCheckPermissions()), so no need to perform AddRelPermissionsInfo(). */ rte->lateral = lateral; rte->inh = false; /* never true for tablefunc RTEs */ rte->inFromCl = inFromCl; - rte->requiredPerms = 0; - rte->checkAsUser = InvalidOid; - rte->selectedCols = NULL; - rte->insertedCols = NULL; - rte->updatedCols = NULL; - rte->extraUpdatedCols = NULL; - /* * Add completed RTE to pstate's range table list, so that we know its * index. But we don't add it to the join list --- caller must do that if @@ -2170,19 +2152,13 @@ addRangeTableEntryForValues(ParseState *pstate, /* * Set flags and access permissions. * - * Subqueries are never checked for access rights. + * Subqueries are never checked for access rights, so no need to perform + * AddRTEPermissionInfo(). */ rte->lateral = lateral; rte->inh = false; /* never true for values RTEs */ rte->inFromCl = inFromCl; - rte->requiredPerms = 0; - rte->checkAsUser = InvalidOid; - rte->selectedCols = NULL; - rte->insertedCols = NULL; - rte->updatedCols = NULL; - rte->extraUpdatedCols = NULL; - /* * Add completed RTE to pstate's range table list, so that we know its * index. But we don't add it to the join list --- caller must do that if @@ -2267,19 +2243,13 @@ addRangeTableEntryForJoin(ParseState *pstate, /* * Set flags and access permissions. * - * Joins are never checked for access rights. + * Joins are never checked for access rights, so no need to perform + * AddRTEPermissionInfo(). */ rte->lateral = false; rte->inh = false; /* never true for joins */ rte->inFromCl = inFromCl; - rte->requiredPerms = 0; - rte->checkAsUser = InvalidOid; - rte->selectedCols = NULL; - rte->insertedCols = NULL; - rte->updatedCols = NULL; - rte->extraUpdatedCols = NULL; - /* * Add completed RTE to pstate's range table list, so that we know its * index. But we don't add it to the join list --- caller must do that if @@ -2294,6 +2264,7 @@ addRangeTableEntryForJoin(ParseState *pstate, nsitem = (ParseNamespaceItem *) palloc(sizeof(ParseNamespaceItem)); nsitem->p_names = rte->eref; nsitem->p_rte = rte; + nsitem->p_perminfo = NULL; nsitem->p_rtindex = list_length(pstate->p_rtable); nsitem->p_nscolumns = nscolumns; /* set default visibility flags; might get changed later */ @@ -2417,19 +2388,13 @@ addRangeTableEntryForCTE(ParseState *pstate, /* * Set flags and access permissions. * - * Subqueries are never checked for access rights. + * Subqueries are never checked for access rights, so no need to perform + * AddRTEPermissionInfo(). */ rte->lateral = false; rte->inh = false; /* never true for subqueries */ rte->inFromCl = inFromCl; - rte->requiredPerms = 0; - rte->checkAsUser = InvalidOid; - rte->selectedCols = NULL; - rte->insertedCols = NULL; - rte->updatedCols = NULL; - rte->extraUpdatedCols = NULL; - /* * Add completed RTE to pstate's range table list, so that we know its * index. But we don't add it to the join list --- caller must do that if @@ -2543,16 +2508,13 @@ addRangeTableEntryForENR(ParseState *pstate, /* * Set flags and access permissions. * - * ENRs are never checked for access rights. + * ENRs are never checked for access rights, so no need to perform + * AddRTEPermissionInfo(). */ rte->lateral = false; rte->inh = false; /* never true for ENRs */ rte->inFromCl = inFromCl; - rte->requiredPerms = 0; - rte->checkAsUser = InvalidOid; - rte->selectedCols = NULL; - /* * Add completed RTE to pstate's range table list, so that we know its * index. But we don't add it to the join list --- caller must do that if @@ -2564,7 +2526,7 @@ addRangeTableEntryForENR(ParseState *pstate, * Build a ParseNamespaceItem, but don't add it to the pstate's namespace * list --- caller must do that if appropriate. */ - return buildNSItemFromTupleDesc(rte, list_length(pstate->p_rtable), + return buildNSItemFromTupleDesc(rte, list_length(pstate->p_rtable), NULL, tupdesc); } @@ -3189,6 +3151,7 @@ expandNSItemAttrs(ParseState *pstate, ParseNamespaceItem *nsitem, int sublevels_up, bool require_col_privs, int location) { RangeTblEntry *rte = nsitem->p_rte; + RTEPermissionInfo *perminfo = nsitem->p_perminfo; List *names, *vars; ListCell *name, @@ -3206,7 +3169,10 @@ expandNSItemAttrs(ParseState *pstate, ParseNamespaceItem *nsitem, * relation of UPDATE/DELETE, which cannot be under a join.) */ if (rte->rtekind == RTE_RELATION) - rte->requiredPerms |= ACL_SELECT; + { + Assert(perminfo != NULL); + perminfo->requiredPerms |= ACL_SELECT; + } forboth(name, names, var, vars) { @@ -3855,3 +3821,57 @@ isQueryUsingTempRelation_walker(Node *node, void *context) isQueryUsingTempRelation_walker, context); } + +/* + * AddRTEPermissionInfo + * Creates RTEPermissionInfo for a given RTE and adds it into the + * provided list + * + * Returns the RTEPermissionInfo and sets rte->perminfoindex. + */ +RTEPermissionInfo * +AddRTEPermissionInfo(List **rtepermlist, RangeTblEntry *rte) +{ + RTEPermissionInfo *perminfo; + + Assert(rte->rtekind == RTE_RELATION); + Assert(rte->perminfoindex == 0); + + /* Nope, so make one and add to the list. */ + perminfo = makeNode(RTEPermissionInfo); + perminfo->relid = rte->relid; + perminfo->inh = rte->inh; + /* Other information is set by fetching the node as and where needed. */ + + *rtepermlist = lappend(*rtepermlist, perminfo); + + /* Note its index (1-based!) */ + rte->perminfoindex = list_length(*rtepermlist); + + return perminfo; +} + +/* + * GetRTEPermissionInfo + * Find RTEPermissionInfo for a given relation in the provided list + * + * This is a simple list_nth() operation though it's good to have the function + * for the various sanity checks. + */ +RTEPermissionInfo * +GetRTEPermissionInfo(List *rtepermlist, RangeTblEntry *rte) +{ + RTEPermissionInfo *perminfo; + + if (rte->perminfoindex == 0 || + rte->perminfoindex > list_length(rtepermlist)) + elog(ERROR, "invalid perminfoindex %d in RTE with relid %u", + rte->perminfoindex, rte->relid); + perminfo = list_nth_node(RTEPermissionInfo, rtepermlist, + rte->perminfoindex - 1); + if (perminfo->relid != rte->relid) + elog(ERROR, "permission info at index %u (with relid=%u) does not match provided RTE (with relid=%u)", + rte->perminfoindex, perminfo->relid, rte->relid); + + return perminfo; +} diff --git a/src/backend/parser/parse_target.c b/src/backend/parser/parse_target.c index 8e0d6fd01f..56d64c8851 100644 --- a/src/backend/parser/parse_target.c +++ b/src/backend/parser/parse_target.c @@ -1132,7 +1132,7 @@ ExpandColumnRefStar(ParseState *pstate, ColumnRef *cref, * * Note: this code is a lot like transformColumnRef; it's tempting to * call that instead and then replace the resulting whole-row Var with - * a list of Vars. However, that would leave us with the RTE's + * a list of Vars. However, that would leave us with the relation's * selectedCols bitmap showing the whole row as needing select * permission, as well as the individual columns. That would be * incorrect (since columns added later shouldn't need select @@ -1367,6 +1367,7 @@ ExpandSingleTable(ParseState *pstate, ParseNamespaceItem *nsitem, else { RangeTblEntry *rte = nsitem->p_rte; + RTEPermissionInfo *perminfo = nsitem->p_perminfo; List *vars; ListCell *l; @@ -1381,7 +1382,10 @@ ExpandSingleTable(ParseState *pstate, ParseNamespaceItem *nsitem, * target relation of UPDATE/DELETE, which cannot be under a join.) */ if (rte->rtekind == RTE_RELATION) - rte->requiredPerms |= ACL_SELECT; + { + Assert(perminfo != NULL); + perminfo->requiredPerms |= ACL_SELECT; + } /* Require read access to each column */ foreach(l, vars) @@ -1414,11 +1418,11 @@ ExpandRowReference(ParseState *pstate, Node *expr, /* * If the rowtype expression is a whole-row Var, we can expand the fields * as simple Vars. Note: if the RTE is a relation, this case leaves us - * with the RTE's selectedCols bitmap showing the whole row as needing - * select permission, as well as the individual columns. However, we can - * only get here for weird notations like (table.*).*, so it's not worth - * trying to clean up --- arguably, the permissions marking is correct - * anyway for such cases. + * with its RTEPermissionInfo's selectedCols bitmap showing the whole row + * as needing select permission, as well as the individual columns. + * However, we can only get here for weird notations like (table.*).*, so + * it's not worth trying to clean up --- arguably, the permissions marking + * is correct anyway for such cases. */ if (IsA(expr, Var) && ((Var *) expr)->varattno == InvalidAttrNumber) diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c index 36791d8817..efff8c03b3 100644 --- a/src/backend/parser/parse_utilcmd.c +++ b/src/backend/parser/parse_utilcmd.c @@ -3023,9 +3023,6 @@ transformRuleStmt(RuleStmt *stmt, const char *queryString, AccessShareLock, makeAlias("new", NIL), false, false); - /* Must override addRangeTableEntry's default access-check flags */ - oldnsitem->p_rte->requiredPerms = 0; - newnsitem->p_rte->requiredPerms = 0; /* * They must be in the namespace too for lookup purposes, but only add the @@ -3081,6 +3078,7 @@ transformRuleStmt(RuleStmt *stmt, const char *queryString, nothing_qry->commandType = CMD_NOTHING; nothing_qry->rtable = pstate->p_rtable; + nothing_qry->rtepermlist = pstate->p_rtepermlist; nothing_qry->jointree = makeFromExpr(NIL, NULL); /* no join wanted */ *actions = list_make1(nothing_qry); @@ -3123,8 +3121,6 @@ transformRuleStmt(RuleStmt *stmt, const char *queryString, AccessShareLock, makeAlias("new", NIL), false, false); - oldnsitem->p_rte->requiredPerms = 0; - newnsitem->p_rte->requiredPerms = 0; addNSItemToQuery(sub_pstate, oldnsitem, false, true, false); addNSItemToQuery(sub_pstate, newnsitem, false, true, false); diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c index e48a3f589a..568344cc23 100644 --- a/src/backend/replication/logical/worker.c +++ b/src/backend/replication/logical/worker.c @@ -156,6 +156,7 @@ #include "miscadmin.h" #include "nodes/makefuncs.h" #include "optimizer/optimizer.h" +#include "parser/parse_relation.h" #include "pgstat.h" #include "postmaster/bgworker.h" #include "postmaster/interrupt.h" @@ -516,6 +517,8 @@ create_edata_for_relation(LogicalRepRelMapEntry *rel) rte->rellockmode = AccessShareLock; ExecInitRangeTable(estate, list_make1(rte)); + AddRTEPermissionInfo(&estate->es_rtepermlist, rte); + edata->targetRelInfo = resultRelInfo = makeNode(ResultRelInfo); /* @@ -1813,6 +1816,7 @@ apply_handle_update(StringInfo s) bool has_oldtup; TupleTableSlot *remoteslot; RangeTblEntry *target_rte; + RTEPermissionInfo *target_perminfo; MemoryContext oldctx; /* @@ -1861,6 +1865,7 @@ apply_handle_update(StringInfo s) * on the subscriber, since we are not touching those. */ target_rte = list_nth(estate->es_range_table, 0); + target_perminfo = list_nth(estate->es_rtepermlist, 0); for (int i = 0; i < remoteslot->tts_tupleDescriptor->natts; i++) { Form_pg_attribute att = TupleDescAttr(remoteslot->tts_tupleDescriptor, i); @@ -1870,14 +1875,14 @@ apply_handle_update(StringInfo s) { Assert(remoteattnum < newtup.ncols); if (newtup.colstatus[remoteattnum] != LOGICALREP_COLUMN_UNCHANGED) - target_rte->updatedCols = - bms_add_member(target_rte->updatedCols, + target_perminfo->updatedCols = + bms_add_member(target_perminfo->updatedCols, i + 1 - FirstLowInvalidHeapAttributeNumber); } } /* Also populate extraUpdatedCols, in case we have generated columns */ - fill_extraUpdatedCols(target_rte, rel->localrel); + fill_extraUpdatedCols(target_rte, target_perminfo, rel->localrel); /* Build the search tuple. */ oldctx = MemoryContextSwitchTo(GetPerTupleMemoryContext(estate)); diff --git a/src/backend/rewrite/rewriteDefine.c b/src/backend/rewrite/rewriteDefine.c index db45d8a08b..3b2649f7a0 100644 --- a/src/backend/rewrite/rewriteDefine.c +++ b/src/backend/rewrite/rewriteDefine.c @@ -797,14 +797,14 @@ checkRuleResultList(List *targetList, TupleDesc resultDesc, bool isSelect, /* * setRuleCheckAsUser * Recursively scan a query or expression tree and set the checkAsUser - * field to the given userid in all rtable entries. + * field to the given userid in all RTEPermissionInfos of the query. * * Note: for a view (ON SELECT rule), the checkAsUser field of the OLD - * RTE entry will be overridden when the view rule is expanded, and the - * checkAsUser field of the NEW entry is irrelevant because that entry's - * requiredPerms bits will always be zero. However, for other types of rules - * it's important to set these fields to match the rule owner. So we just set - * them always. + * RTE entry's RTEPermissionInfo will be overridden when the view rule is + * expanded, and the checkAsUser for the NEW RTE entry's RTEPermissionInfo is + * irrelevant because its requiredPerms bits will always be zero. However, for + * other types of rules it's important to set these fields to match the rule + * owner. So we just set them always. */ void setRuleCheckAsUser(Node *node, Oid userid) @@ -831,18 +831,21 @@ setRuleCheckAsUser_Query(Query *qry, Oid userid) { ListCell *l; - /* Set all the RTEs in this query node */ + /* Set in all RTEPermissionInfos for this query. */ + foreach(l, qry->rtepermlist) + { + RTEPermissionInfo *perminfo = lfirst_node(RTEPermissionInfo, l); + + perminfo->checkAsUser = userid; + } + + /* Now recurse to any subquery RTEs */ foreach(l, qry->rtable) { RangeTblEntry *rte = (RangeTblEntry *) lfirst(l); if (rte->rtekind == RTE_SUBQUERY) - { - /* Recurse into subquery in FROM */ setRuleCheckAsUser_Query(rte->subquery, userid); - } - else - rte->checkAsUser = userid; } /* Recurse into subquery-in-WITH */ diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c index fb0c687bd8..ecf585e0fd 100644 --- a/src/backend/rewrite/rewriteHandler.c +++ b/src/backend/rewrite/rewriteHandler.c @@ -353,6 +353,7 @@ rewriteRuleAction(Query *parsetree, Query *sub_action; Query **sub_action_ptr; acquireLocksOnSubLinks_context context; + List *action_rtepermlist; context.for_execute = true; @@ -395,32 +396,35 @@ rewriteRuleAction(Query *parsetree, * Generate expanded rtable consisting of main parsetree's rtable plus * rule action's rtable; this becomes the complete rtable for the rule * action. Some of the entries may be unused after we finish rewriting, - * but we leave them all in place for two reasons: + * but we leave them all in place to avoid having to adjust the query's + * varnos. RT entries that are not referenced in the completed jointree + * will be ignored by the planner, so they do not affect query semantics. * - * We'd have a much harder job to adjust the query's varnos if we - * selectively removed RT entries. + * Also merge RTEPermissionInfo lists to ensure that all permissions are + * checked correctly. * * If the rule is INSTEAD, then the original query won't be executed at - * all, and so its rtable must be preserved so that the executor will do - * the correct permissions checks on it. + * all, and so its rtepermlist must be preserved so that the executor will + * do the correct permissions checks on the relations referenced in it. + * This allows us to check that the caller has, say, insert-permission on + * a view, when the view is not semantically referenced at all in the + * resulting query. * - * RT entries that are not referenced in the completed jointree will be - * ignored by the planner, so they do not affect query semantics. But any - * permissions checks specified in them will be applied during executor - * startup (see ExecCheckRTEPerms()). This allows us to check that the - * caller has, say, insert-permission on a view, when the view is not - * semantically referenced at all in the resulting query. + * When a rule is not INSTEAD, the permissions checks done using the + * copied entries will be redundant with those done during execution of + * the original query, but we don't bother to treat that case differently. * - * When a rule is not INSTEAD, the permissions checks done on its copied - * RT entries will be redundant with those done during execution of the - * original query, but we don't bother to treat that case differently. - * - * NOTE: because planner will destructively alter rtable, we must ensure - * that rule action's rtable is separate and shares no substructure with - * the main rtable. Hence do a deep copy here. + * NOTE: because planner will destructively alter rtable and rtepermlist, + * we must ensure that rule action's lists are separate and shares no + * substructure with the main query's lists. Hence do a deep copy here + * for both. */ - sub_action->rtable = list_concat(copyObject(parsetree->rtable), - sub_action->rtable); + action_rtepermlist = sub_action->rtepermlist; + sub_action->rtepermlist = copyObject(parsetree->rtepermlist); + sub_action->rtable = CombineRangeTables(copyObject(parsetree->rtable), + sub_action->rtable, + action_rtepermlist, + &sub_action->rtepermlist); /* * There could have been some SubLinks in parsetree's rtable, in which @@ -1624,10 +1628,13 @@ rewriteValuesRTEToNulls(Query *parsetree, RangeTblEntry *rte) /* * Record in target_rte->extraUpdatedCols the indexes of any generated columns - * that depend on any columns mentioned in target_rte->updatedCols. + * columns that depend on any columns mentioned in + * target_perminfo->updatedCols. */ void -fill_extraUpdatedCols(RangeTblEntry *target_rte, Relation target_relation) +fill_extraUpdatedCols(RangeTblEntry *target_rte, + RTEPermissionInfo *target_perminfo, + Relation target_relation) { TupleDesc tupdesc = RelationGetDescr(target_relation); TupleConstr *constr = tupdesc->constr; @@ -1650,7 +1657,7 @@ fill_extraUpdatedCols(RangeTblEntry *target_rte, Relation target_relation) expr = stringToNode(defval->adbin); pull_varattnos(expr, 1, &attrs_used); - if (bms_overlap(target_rte->updatedCols, attrs_used)) + if (bms_overlap(target_perminfo->updatedCols, attrs_used)) target_rte->extraUpdatedCols = bms_add_member(target_rte->extraUpdatedCols, defval->adnum - FirstLowInvalidHeapAttributeNumber); @@ -1743,6 +1750,8 @@ ApplyRetrieveRule(Query *parsetree, Query *rule_action; RangeTblEntry *rte, *subrte; + RTEPermissionInfo *perminfo, + *sub_perminfo; RowMarkClause *rc; if (list_length(rule->actions) != 1) @@ -1783,18 +1792,6 @@ ApplyRetrieveRule(Query *parsetree, parsetree->rtable = lappend(parsetree->rtable, newrte); parsetree->resultRelation = list_length(parsetree->rtable); - /* - * There's no need to do permissions checks twice, so wipe out the - * permissions info for the original RTE (we prefer to keep the - * bits set on the result RTE). - */ - rte->requiredPerms = 0; - rte->checkAsUser = InvalidOid; - rte->selectedCols = NULL; - rte->insertedCols = NULL; - rte->updatedCols = NULL; - rte->extraUpdatedCols = NULL; - /* * For the most part, Vars referencing the view should remain as * they are, meaning that they implicitly represent OLD values. @@ -1858,12 +1855,6 @@ ApplyRetrieveRule(Query *parsetree, /* * Recursively expand any view references inside the view. - * - * Note: this must happen after markQueryForLocking. That way, any UPDATE - * permission bits needed for sub-views are initially applied to their - * RTE_RELATION RTEs by markQueryForLocking, and then transferred to their - * OLD rangetable entries by the action below (in a recursive call of this - * routine). */ rule_action = fireRIRrules(rule_action, activeRIRs); @@ -1872,6 +1863,7 @@ ApplyRetrieveRule(Query *parsetree, * original RTE to a subquery RTE. */ rte = rt_fetch(rt_index, parsetree->rtable); + perminfo = GetRTEPermissionInfo(parsetree->rtepermlist, rte); rte->rtekind = RTE_SUBQUERY; rte->subquery = rule_action; @@ -1881,6 +1873,7 @@ ApplyRetrieveRule(Query *parsetree, rte->relkind = 0; rte->rellockmode = 0; rte->tablesample = NULL; + rte->perminfoindex = 0; /* no permission checking for this RTE */ rte->inh = false; /* must not be set for a subquery */ /* @@ -1889,19 +1882,12 @@ ApplyRetrieveRule(Query *parsetree, */ subrte = rt_fetch(PRS2_OLD_VARNO, rule_action->rtable); Assert(subrte->relid == relation->rd_id); - subrte->requiredPerms = rte->requiredPerms; - subrte->checkAsUser = rte->checkAsUser; - subrte->selectedCols = rte->selectedCols; - subrte->insertedCols = rte->insertedCols; - subrte->updatedCols = rte->updatedCols; - subrte->extraUpdatedCols = rte->extraUpdatedCols; - - rte->requiredPerms = 0; /* no permission check on subquery itself */ - rte->checkAsUser = InvalidOid; - rte->selectedCols = NULL; - rte->insertedCols = NULL; - rte->updatedCols = NULL; - rte->extraUpdatedCols = NULL; + sub_perminfo = GetRTEPermissionInfo(rule_action->rtepermlist, subrte); + sub_perminfo->requiredPerms = perminfo->requiredPerms; + sub_perminfo->checkAsUser = perminfo->checkAsUser; + sub_perminfo->selectedCols = perminfo->selectedCols; + sub_perminfo->insertedCols = perminfo->insertedCols; + sub_perminfo->updatedCols = perminfo->updatedCols; return parsetree; } @@ -1931,8 +1917,12 @@ markQueryForLocking(Query *qry, Node *jtnode, if (rte->rtekind == RTE_RELATION) { + RTEPermissionInfo *perminfo; + applyLockingClause(qry, rti, strength, waitPolicy, pushedDown); - rte->requiredPerms |= ACL_SELECT_FOR_UPDATE; + + perminfo = GetRTEPermissionInfo(qry->rtepermlist, rte); + perminfo->requiredPerms |= ACL_SELECT_FOR_UPDATE; } else if (rte->rtekind == RTE_SUBQUERY) { @@ -3073,6 +3063,9 @@ rewriteTargetView(Query *parsetree, Relation view) RangeTblEntry *base_rte; RangeTblEntry *view_rte; RangeTblEntry *new_rte; + RTEPermissionInfo *base_perminfo; + RTEPermissionInfo *view_perminfo; + RTEPermissionInfo *new_perminfo; Relation base_rel; List *view_targetlist; ListCell *lc; @@ -3209,6 +3202,7 @@ rewriteTargetView(Query *parsetree, Relation view) base_rt_index = rtr->rtindex; base_rte = rt_fetch(base_rt_index, viewquery->rtable); Assert(base_rte->rtekind == RTE_RELATION); + base_perminfo = GetRTEPermissionInfo(viewquery->rtepermlist, base_rte); /* * Up to now, the base relation hasn't been touched at all in our query. @@ -3280,57 +3274,68 @@ rewriteTargetView(Query *parsetree, Relation view) 0); /* - * If the view has "security_invoker" set, mark the new target RTE for the - * permissions checks that we want to enforce against the query caller. - * Otherwise we want to enforce them against the view owner. + * If the view has "security_invoker" set, mark the new target relation + * for the permissions checks that we want to enforce against the query + * caller. Otherwise we want to enforce them against the view owner. * * At the relation level, require the same INSERT/UPDATE/DELETE * permissions that the query caller needs against the view. We drop the - * ACL_SELECT bit that is presumably in new_rte->requiredPerms initially. + * ACL_SELECT bit that is presumably in new_perminfo->requiredPerms + * initially. * - * Note: the original view RTE remains in the query's rangetable list. - * Although it will be unused in the query plan, we need it there so that - * the executor still performs appropriate permissions checks for the - * query caller's use of the view. + * Note: the original view's RTEPermissionInfo remains in the query's + * rtepermlist so that the executor still performs appropriate permissions + * checks for the query caller's use of the view. */ - if (RelationHasSecurityInvoker(view)) - new_rte->checkAsUser = InvalidOid; - else - new_rte->checkAsUser = view->rd_rel->relowner; + view_perminfo = GetRTEPermissionInfo(parsetree->rtepermlist, view_rte); - new_rte->requiredPerms = view_rte->requiredPerms; + /* + * Disregard the perminfo in viewquery->rtepermlist that the base_rte + * would currently be pointing at, because we'd like it to point now + * to a new one that will be filled below. Must set perminfoindex to + * 0 to not trip over the Assert in AddRTEPermissionInfo(). + */ + new_rte->perminfoindex = 0; + new_perminfo = AddRTEPermissionInfo(&parsetree->rtepermlist, new_rte); + if (RelationHasSecurityInvoker(view)) + new_perminfo->checkAsUser = InvalidOid; + else + new_perminfo->checkAsUser = view->rd_rel->relowner; + new_perminfo->requiredPerms = view_perminfo->requiredPerms; /* * Now for the per-column permissions bits. * - * Initially, new_rte contains selectedCols permission check bits for all - * base-rel columns referenced by the view, but since the view is a SELECT - * query its insertedCols/updatedCols is empty. We set insertedCols and - * updatedCols to include all the columns the outer query is trying to - * modify, adjusting the column numbers as needed. But we leave - * selectedCols as-is, so the view owner must have read permission for all - * columns used in the view definition, even if some of them are not read - * by the outer query. We could try to limit selectedCols to only columns - * used in the transformed query, but that does not correspond to what - * happens in ordinary SELECT usage of a view: all referenced columns must - * have read permission, even if optimization finds that some of them can - * be discarded during query transformation. The flattening we're doing - * here is an optional optimization, too. (If you are unpersuaded and - * want to change this, note that applying adjust_view_column_set to - * view_rte->selectedCols is clearly *not* the right answer, since that - * neglects base-rel columns used in the view's WHERE quals.) + * Initially, new_perminfo (base_perminfo) contains selectedCols permission + * check bits for all base-rel columns referenced by the view, but since + * the view is a SELECT query its insertedCols/updatedCols is empty. We + * set insertedCols and updatedCols to include all the columns the outer + * query is trying to modify, adjusting the column numbers as needed. But + * we leave selectedCols as-is, so the view owner must have read permission + * for all columns used in the view definition, even if some of them are + * not read by the outer query. We could try to limit selectedCols to only + * columns used in the transformed query, but that does not correspond to + * what happens in ordinary SELECT usage of a view: all referenced columns + * must have read permission, even if optimization finds that some of them + * can be discarded during query transformation. The flattening we're + * doing here is an optional optimization, too. (If you are unpersuaded + * and want to change this, note that applying adjust_view_column_set to + * view_perminfo->selectedCols is clearly *not* the right answer, since + * that neglects base-rel columns used in the view's WHERE quals.) * * This step needs the modified view targetlist, so we have to do things * in this order. */ - Assert(bms_is_empty(new_rte->insertedCols) && - bms_is_empty(new_rte->updatedCols)); + Assert(bms_is_empty(new_perminfo->insertedCols) && + bms_is_empty(new_perminfo->updatedCols)); - new_rte->insertedCols = adjust_view_column_set(view_rte->insertedCols, - view_targetlist); + new_perminfo->selectedCols = base_perminfo->selectedCols; - new_rte->updatedCols = adjust_view_column_set(view_rte->updatedCols, - view_targetlist); + new_perminfo->insertedCols = + adjust_view_column_set(view_perminfo->insertedCols, view_targetlist); + + new_perminfo->updatedCols = + adjust_view_column_set(view_perminfo->updatedCols, view_targetlist); /* * Move any security barrier quals from the view RTE onto the new target @@ -3434,7 +3439,7 @@ rewriteTargetView(Query *parsetree, Relation view) * from the view, hence we need a new column alias list). This should * match transformOnConflictClause. In particular, note that the * relkind is set to composite to signal that we're not dealing with - * an actual relation, and no permissions checks are wanted. + * an actual relation. */ old_exclRelIndex = parsetree->onConflict->exclRelIndex; @@ -3445,8 +3450,8 @@ rewriteTargetView(Query *parsetree, Relation view) false, false); new_exclRte = new_exclNSItem->p_rte; new_exclRte->relkind = RELKIND_COMPOSITE_TYPE; - new_exclRte->requiredPerms = 0; - /* other permissions fields in new_exclRte are already empty */ + /* Ignore the RTEPermissionInfo that would've been added. */ + new_exclRte->perminfoindex = 0; parsetree->rtable = lappend(parsetree->rtable, new_exclRte); new_exclRelIndex = parsetree->onConflict->exclRelIndex = @@ -3720,6 +3725,7 @@ RewriteQuery(Query *parsetree, List *rewrite_events) { int result_relation; RangeTblEntry *rt_entry; + RTEPermissionInfo *rt_perminfo; Relation rt_entry_relation; List *locks; List *product_queries; @@ -3731,6 +3737,7 @@ RewriteQuery(Query *parsetree, List *rewrite_events) Assert(result_relation != 0); rt_entry = rt_fetch(result_relation, parsetree->rtable); Assert(rt_entry->rtekind == RTE_RELATION); + rt_perminfo = GetRTEPermissionInfo(parsetree->rtepermlist, rt_entry); /* * We can use NoLock here since either the parser or @@ -3817,7 +3824,7 @@ RewriteQuery(Query *parsetree, List *rewrite_events) NULL, 0, NULL); /* Also populate extraUpdatedCols (for generated columns) */ - fill_extraUpdatedCols(rt_entry, rt_entry_relation); + fill_extraUpdatedCols(rt_entry, rt_perminfo, rt_entry_relation); } else if (event == CMD_MERGE) { diff --git a/src/backend/rewrite/rewriteManip.c b/src/backend/rewrite/rewriteManip.c index 101c39553a..3577c7f335 100644 --- a/src/backend/rewrite/rewriteManip.c +++ b/src/backend/rewrite/rewriteManip.c @@ -1531,3 +1531,37 @@ ReplaceVarsFromTargetList(Node *node, (void *) &context, outer_hasSubLinks); } + +/* + * CombineRangeTables + * Adds the RTEs of 'rtable2' into 'rtable1' + * + * This also adds the RTEPermissionInfos of 'rtepermlist2' (belonging to the + * RTEs in 'rtable2') into *rtepermlist1 and also updates perminfoindex of the + * RTEs in 'rtable2' to now point to the perminfos' indexes in *rtepermlist1. + * + * Note that this changes both 'rtable1' and 'rtepermlist1' destructively, so + * the caller should have better passed safe-to-modify copies. + */ +List * +CombineRangeTables(List *rtable1, List *rtable2, + List *rtepermlist2, List **rtepermlist1) +{ + ListCell *l; + int offset = list_length(*rtepermlist1); + + if (offset > 0) + { + foreach(l, rtable2) + { + RangeTblEntry *rte = lfirst_node(RangeTblEntry, l); + + if (rte->perminfoindex > 0) + rte->perminfoindex += offset; + } + } + + *rtepermlist1 = list_concat(*rtepermlist1, rtepermlist2); + + return list_concat(rtable1, rtable2); +} diff --git a/src/backend/rewrite/rowsecurity.c b/src/backend/rewrite/rowsecurity.c index f49cfb6cc6..3a6487b4e5 100644 --- a/src/backend/rewrite/rowsecurity.c +++ b/src/backend/rewrite/rowsecurity.c @@ -47,6 +47,7 @@ #include "nodes/pg_list.h" #include "nodes/plannodes.h" #include "parser/parsetree.h" +#include "parser/parse_relation.h" #include "rewrite/rewriteDefine.h" #include "rewrite/rewriteHandler.h" #include "rewrite/rewriteManip.h" @@ -115,6 +116,7 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index, CmdType commandType; List *permissive_policies; List *restrictive_policies; + RTEPermissionInfo *perminfo; /* Defaults for the return values */ *securityQuals = NIL; @@ -122,16 +124,21 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index, *hasRowSecurity = false; *hasSubLinks = false; + Assert(rte->rtekind == RTE_RELATION); + /* If this is not a normal relation, just return immediately */ if (rte->relkind != RELKIND_RELATION && rte->relkind != RELKIND_PARTITIONED_TABLE) return; + perminfo = GetRTEPermissionInfo(root->rtepermlist, rte); + /* Switch to checkAsUser if it's set */ - user_id = OidIsValid(rte->checkAsUser) ? rte->checkAsUser : GetUserId(); + user_id = OidIsValid(perminfo->checkAsUser) ? + perminfo->checkAsUser : GetUserId(); /* Determine the state of RLS for this, pass checkAsUser explicitly */ - rls_status = check_enable_rls(rte->relid, rte->checkAsUser, false); + rls_status = check_enable_rls(rte->relid, perminfo->checkAsUser, false); /* If there is no RLS on this table at all, nothing to do */ if (rls_status == RLS_NONE) @@ -196,7 +203,7 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index, * which the user does not have access to via the UPDATE USING policies, * similar to how we require normal UPDATE rights for these queries. */ - if (commandType == CMD_SELECT && rte->requiredPerms & ACL_UPDATE) + if (commandType == CMD_SELECT && perminfo->requiredPerms & ACL_UPDATE) { List *update_permissive_policies; List *update_restrictive_policies; @@ -243,7 +250,7 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index, */ if ((commandType == CMD_UPDATE || commandType == CMD_DELETE || commandType == CMD_MERGE) && - rte->requiredPerms & ACL_SELECT) + perminfo->requiredPerms & ACL_SELECT) { List *select_permissive_policies; List *select_restrictive_policies; @@ -286,7 +293,7 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index, * raised if a policy is violated; otherwise, we might end up silently * dropping rows to be added. */ - if (rte->requiredPerms & ACL_SELECT) + if (perminfo->requiredPerms & ACL_SELECT) { List *select_permissive_policies = NIL; List *select_restrictive_policies = NIL; @@ -342,7 +349,7 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index, * for this relation, also as WCO policies, again, to avoid * silently dropping data. See above. */ - if (rte->requiredPerms & ACL_SELECT) + if (perminfo->requiredPerms & ACL_SELECT) { get_policies_for_relation(rel, CMD_SELECT, user_id, &conflict_select_permissive_policies, @@ -371,7 +378,7 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index, * path of an INSERT .. ON CONFLICT DO UPDATE, if SELECT rights * are required for this relation. */ - if (rte->requiredPerms & ACL_SELECT) + if (perminfo->requiredPerms & ACL_SELECT) add_with_check_options(rel, rt_index, WCO_RLS_UPDATE_CHECK, conflict_select_permissive_policies, @@ -474,8 +481,8 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index, * Copy checkAsUser to the row security quals and WithCheckOption checks, * in case they contain any subqueries referring to other relations. */ - setRuleCheckAsUser((Node *) *securityQuals, rte->checkAsUser); - setRuleCheckAsUser((Node *) *withCheckOptions, rte->checkAsUser); + setRuleCheckAsUser((Node *) *securityQuals, perminfo->checkAsUser); + setRuleCheckAsUser((Node *) *withCheckOptions, perminfo->checkAsUser); /* * Mark this query as having row security, so plancache can invalidate it diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c index dc07157037..29f29d664b 100644 --- a/src/backend/utils/adt/ri_triggers.c +++ b/src/backend/utils/adt/ri_triggers.c @@ -1375,6 +1375,8 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel) char fkattname[MAX_QUOTED_NAME_LEN + 3]; RangeTblEntry *pkrte; RangeTblEntry *fkrte; + RTEPermissionInfo *pk_perminfo; + RTEPermissionInfo *fk_perminfo; const char *sep; const char *fk_only; const char *pk_only; @@ -1397,27 +1399,34 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel) pkrte->relid = RelationGetRelid(pk_rel); pkrte->relkind = pk_rel->rd_rel->relkind; pkrte->rellockmode = AccessShareLock; - pkrte->requiredPerms = ACL_SELECT; + + 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; - fkrte->requiredPerms = ACL_SELECT; + + fk_perminfo = makeNode(RTEPermissionInfo); + fk_perminfo->relid = RelationGetRelid(fk_rel); + fk_perminfo->requiredPerms = ACL_SELECT; for (int i = 0; i < riinfo->nkeys; i++) { int attno; attno = riinfo->pk_attnums[i] - FirstLowInvalidHeapAttributeNumber; - pkrte->selectedCols = bms_add_member(pkrte->selectedCols, attno); + pk_perminfo->selectedCols = bms_add_member(pk_perminfo->selectedCols, attno); attno = riinfo->fk_attnums[i] - FirstLowInvalidHeapAttributeNumber; - fkrte->selectedCols = bms_add_member(fkrte->selectedCols, attno); + fk_perminfo->selectedCols = bms_add_member(fk_perminfo->selectedCols, attno); } - if (!ExecCheckRTPerms(list_make2(fkrte, pkrte), false)) + if (!ExecCheckPermissions(list_make2(fkrte, pkrte), + list_make2(fk_perminfo, pk_perminfo), false)) return false; /* diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index bd6cd4e47b..c3feacb4f8 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -847,8 +847,8 @@ RelationBuildRuleLock(Relation relation) /* * Scan through the rule's actions and set the checkAsUser field on - * all rtable entries. We have to look at the qual as well, in case it - * contains sublinks. + * all RTEPermissionInfos. We have to look at the qual as well, in + * case it contains sublinks. * * The reason for doing this when the rule is loaded, rather than when * it is stored, is that otherwise ALTER TABLE OWNER would have to diff --git a/src/include/commands/copyfrom_internal.h b/src/include/commands/copyfrom_internal.h index 8d9cc5accd..3d2de0ae1b 100644 --- a/src/include/commands/copyfrom_internal.h +++ b/src/include/commands/copyfrom_internal.h @@ -97,7 +97,8 @@ typedef struct CopyFromStateData int *defmap; /* array of default att numbers */ ExprState **defexprs; /* array of default att expressions */ bool volatile_defexprs; /* is any of defexprs volatile? */ - List *range_table; + List *range_table; /* single element list of RangeTblEntry */ + List *rtepermlist; /* single element list of RTEPermissionInfo */ ExprState *qualexpr; TransitionCaptureState *transition_capture; diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index 5c02a1521f..6f5a7038e1 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -80,9 +80,9 @@ extern PGDLLIMPORT ExecutorFinish_hook_type ExecutorFinish_hook; typedef void (*ExecutorEnd_hook_type) (QueryDesc *queryDesc); extern PGDLLIMPORT ExecutorEnd_hook_type ExecutorEnd_hook; -/* Hook for plugins to get control in ExecCheckRTPerms() */ -typedef bool (*ExecutorCheckPerms_hook_type) (List *, bool); -extern PGDLLIMPORT ExecutorCheckPerms_hook_type ExecutorCheckPerms_hook; +/* Hook for plugins to get control in ExecCheckPermissions() */ +typedef bool (*ExecutorCheckPermissions_hook_type) (List *, List *, bool); +extern PGDLLIMPORT ExecutorCheckPermissions_hook_type ExecutorCheckPermissions_hook; /* @@ -196,7 +196,8 @@ extern void standard_ExecutorFinish(QueryDesc *queryDesc); extern void ExecutorEnd(QueryDesc *queryDesc); extern void standard_ExecutorEnd(QueryDesc *queryDesc); extern void ExecutorRewind(QueryDesc *queryDesc); -extern bool ExecCheckRTPerms(List *rangeTable, bool ereport_on_violation); +extern bool ExecCheckPermissions(List *rangeTable, + List *rtepermlist, bool ereport_on_violation); extern void CheckValidResultRel(ResultRelInfo *resultRelInfo, CmdType operation); extern void InitResultRelInfo(ResultRelInfo *resultRelInfo, Relation resultRelationDesc, @@ -576,6 +577,7 @@ exec_rt_fetch(Index rti, EState *estate) } extern Relation ExecGetRangeTableRelation(EState *estate, Index rti); +extern RTEPermissionInfo *ExecGetRTEPermissionInfo(RangeTblEntry *rte, EState *estate); extern void ExecInitResultRelation(EState *estate, ResultRelInfo *resultRelInfo, Index rti); @@ -601,8 +603,9 @@ extern TupleTableSlot *ExecGetTriggerNewSlot(EState *estate, ResultRelInfo *relI extern TupleTableSlot *ExecGetReturningSlot(EState *estate, ResultRelInfo *relInfo); extern TupleConversionMap *ExecGetChildToRootMap(ResultRelInfo *resultRelInfo); extern AttrMap *ExecGetRootToChildMap(ResultRelInfo *resultRelInfo, - EState *estate); + EState *estate); +extern Oid ExecGetResultRelCheckAsUser(ResultRelInfo *relInfo, EState *estate); extern Bitmapset *ExecGetInsertedCols(ResultRelInfo *relinfo, EState *estate); extern Bitmapset *ExecGetUpdatedCols(ResultRelInfo *relinfo, EState *estate); extern Bitmapset *ExecGetExtraUpdatedCols(ResultRelInfo *relinfo, EState *estate); diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 313840fe32..0e6c940ed5 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -568,7 +568,7 @@ typedef struct ResultRelInfo * describe a given child table's columns; see ExecGetInsertedCols() et * al. Like ri_ChildToRootMap, computed only if needed. */ - AttrMap *ri_RootToChildMap; + AttrMap *ri_RootToChildMap; bool ri_RootToChildMapValid; /* for use by copyfrom.c when performing multi-inserts */ @@ -621,6 +621,7 @@ typedef struct EState * pointers, or NULL if not yet opened */ struct ExecRowMark **es_rowmarks; /* Array of per-range-table-entry * ExecRowMarks, or NULL if none */ + List *es_rtepermlist; /* List of RTEPermissionInfo */ PlannedStmt *es_plannedstmt; /* link to top of plan tree */ const char *es_sourceText; /* Source text from QueryDesc */ diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 6112cd85c8..c8a0d6cacc 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -154,6 +154,8 @@ typedef struct Query List *cteList; /* WITH list (of CommonTableExpr's) */ List *rtable; /* list of range table entries */ + List *rtepermlist; /* list of RTEPermissionInfo nodes for the + * rtable entries having perminfoindex > 0 */ FromExpr *jointree; /* table join tree (FROM and WHERE clauses); * also USING clause for MERGE */ @@ -968,37 +970,6 @@ typedef struct PartitionCmd * control visibility. But it is needed by ruleutils.c to determine * whether RTEs should be shown in decompiled queries. * - * requiredPerms and checkAsUser specify run-time access permissions - * checks to be performed at query startup. The user must have *all* - * of the permissions that are OR'd together in requiredPerms (zero - * indicates no permissions checking). If checkAsUser is not zero, - * then do the permissions checks using the access rights of that user, - * not the current effective user ID. (This allows rules to act as - * setuid gateways.) Permissions checks only apply to RELATION RTEs. - * - * For SELECT/INSERT/UPDATE permissions, if the user doesn't have - * table-wide permissions then it is sufficient to have the permissions - * on all columns identified in selectedCols (for SELECT) and/or - * insertedCols and/or updatedCols (INSERT with ON CONFLICT DO UPDATE may - * have all 3). selectedCols, insertedCols and updatedCols are bitmapsets, - * which cannot have negative integer members, so we subtract - * FirstLowInvalidHeapAttributeNumber from column numbers before storing - * them in these fields. A whole-row Var reference is represented by - * setting the bit for InvalidAttrNumber. - * - * updatedCols is also used in some other places, for example, to determine - * which triggers to fire and in FDWs to know which changed columns they - * need to ship off. - * - * Generated columns that are caused to be updated by an update to a base - * column are listed in extraUpdatedCols. This is not considered for - * permission checking, but it is useful in those places that want to know - * the full set of columns being updated as opposed to only the ones the - * user explicitly mentioned in the query. (There is currently no need for - * an extraInsertedCols, but it could exist.) Note that extraUpdatedCols - * is populated during query rewrite, NOT in the parser, since generated - * columns could be added after a rule has been parsed and stored. - * * securityQuals is a list of security barrier quals (boolean expressions), * to be tested in the listed order before returning a row from the * relation. It is always NIL in parser output. Entries are added by the @@ -1054,11 +1025,16 @@ typedef struct RangeTblEntry * current query; this happens if a DO ALSO rule simply scans the original * target table. We leave such RTEs with their original lockmode so as to * avoid getting an additional, lesser lock. + * + * perminfoindex is 1-based index of the RTEPermissionInfo belonging to + * this RTE in the containing struct's list of same; 0 if permissions need + * not be checked for this RTE. */ Oid relid; /* OID of the relation */ char relkind; /* relation kind (see pg_class.relkind) */ int rellockmode; /* lock level that query requires on the rel */ struct TableSampleClause *tablesample; /* sampling info, or NULL */ + Index perminfoindex; /* * Fields valid for a subquery RTE (else NULL): @@ -1178,14 +1154,64 @@ typedef struct RangeTblEntry bool lateral; /* subquery, function, or values is LATERAL? */ bool inh; /* inheritance requested? */ bool inFromCl; /* present in FROM clause? */ + Bitmapset *extraUpdatedCols; /* generated columns being updated */ + List *securityQuals; /* security barrier quals to apply, if any */ +} RangeTblEntry; + +/* + * RTEPermissionInfo + * Per-relation information for permission checking. Added to the Query + * node by the parser when adding the corresponding RTE to the query + * range table and subsequently editorialized on by the rewriter if + * needed after rule expansion. + * + * Only the relations directly mentioned in the query are checked for + * accesss permissions by the core executor, so only their RTEPermissionInfos + * are present in the Query. However, extensions may want to check inheritance + * children too, depending on the value of rte->inh, so it's copied in 'inh' + * for their perusal. + * + * requiredPerms and checkAsUser specify run-time access permissions checks + * to be performed at query startup. The user must have *all* of the + * permissions that are OR'd together in requiredPerms (never 0!). If + * checkAsUser is not zero, then do the permissions checks using the access + * rights of that user, not the current effective user ID. (This allows rules + * to act as setuid gateways.) + * + * For SELECT/INSERT/UPDATE permissions, if the user doesn't have table-wide + * permissions then it is sufficient to have the permissions on all columns + * identified in selectedCols (for SELECT) and/or insertedCols and/or + * updatedCols (INSERT with ON CONFLICT DO UPDATE may have all 3). + * selectedCols, insertedCols and updatedCols are bitmapsets, which cannot have + * negative integer members, so we subtract FirstLowInvalidHeapAttributeNumber + * from column numbers before storing them in these fields. A whole-row Var + * reference is represented by setting the bit for InvalidAttrNumber. + * + * updatedCols is also used in some other places, for example, to determine + * which triggers to fire and in FDWs to know which changed columns they need + * to ship off. + * + * Generated columns that are caused to be updated by an update to a base + * column are listed in extraUpdatedCols. This is not considered for + * permission checking, but it is useful in those places that want to know the + * full set of columns being updated as opposed to only the ones the user + * explicitly mentioned in the query. (There is currently no need for an + * extraInsertedCols, but it could exist.) Note that extraUpdatedCols is + * populated during query rewrite, NOT in the parser, since generated columns + * could be added after a rule has been parsed and stored. + */ +typedef struct RTEPermissionInfo +{ + NodeTag type; + + Oid relid; /* relation OID */ + bool inh; /* separately check inheritance children? */ AclMode requiredPerms; /* bitmask of required access permissions */ Oid checkAsUser; /* if valid, check access as this role */ Bitmapset *selectedCols; /* columns needing SELECT permission */ Bitmapset *insertedCols; /* columns needing INSERT permission */ Bitmapset *updatedCols; /* columns needing UPDATE permission */ - Bitmapset *extraUpdatedCols; /* generated columns being updated */ - List *securityQuals; /* security barrier quals to apply, if any */ -} RangeTblEntry; +} RTEPermissionInfo; /* * RangeTblFunction - diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h index ef95429a0d..27618918b4 100644 --- a/src/include/nodes/pathnodes.h +++ b/src/include/nodes/pathnodes.h @@ -113,6 +113,9 @@ typedef struct PlannerGlobal /* "flat" rangetable for executor */ List *finalrtable; + /* "flat" list of RTEPermissionInfos */ + List *finalrtepermlist; + /* "flat" list of PlanRowMarks */ List *finalrowmarks; diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h index 61cae463fb..f5e3025e27 100644 --- a/src/include/nodes/plannodes.h +++ b/src/include/nodes/plannodes.h @@ -72,6 +72,9 @@ typedef struct PlannedStmt List *rtable; /* list of RangeTblEntry nodes */ + List *permInfos; /* list of RTEPermissionInfo nodes for rtable + * entries needing one */ + /* rtable indexes of target relations for INSERT/UPDATE/DELETE/MERGE */ List *resultRelations; /* integer list of RT indexes, or NIL */ diff --git a/src/include/optimizer/inherit.h b/src/include/optimizer/inherit.h index adcb1d7372..8ebd42b757 100644 --- a/src/include/optimizer/inherit.h +++ b/src/include/optimizer/inherit.h @@ -20,6 +20,8 @@ extern void expand_inherited_rtentry(PlannerInfo *root, RelOptInfo *rel, RangeTblEntry *rte, Index rti); +extern Bitmapset *get_rel_all_updated_cols(PlannerInfo *root, RelOptInfo *rel); + extern bool apply_child_basequals(PlannerInfo *root, RelOptInfo *parentrel, RelOptInfo *childrel, RangeTblEntry *childRTE, AppendRelInfo *appinfo); diff --git a/src/include/parser/parse_node.h b/src/include/parser/parse_node.h index 962ebf65de..7aebed5863 100644 --- a/src/include/parser/parse_node.h +++ b/src/include/parser/parse_node.h @@ -111,6 +111,9 @@ typedef Node *(*CoerceParamHook) (ParseState *pstate, Param *param, * Note that neither relname nor refname of these entries are necessarily * unique; searching the rtable by name is a bad idea. * + * p_rtepermlist: list of RTEPermissionInfo containing an entry corresponding + * to each RTE_RELATION entry in p_rtable. + * * p_joinexprs: list of JoinExpr nodes associated with p_rtable entries. * This is one-for-one with p_rtable, but contains NULLs for non-join * RTEs, and may be shorter than p_rtable if the last RTE(s) aren't joins. @@ -181,6 +184,8 @@ struct ParseState ParseState *parentParseState; /* stack link */ const char *p_sourcetext; /* source text, or NULL if not available */ List *p_rtable; /* range table so far */ + List *p_rtepermlist; /* list of RTEPermissionInfo nodes for each + * RTE_RELATION entry in rtable */ List *p_joinexprs; /* JoinExprs for RTE_JOIN p_rtable entries */ List *p_joinlist; /* join items so far (will become FromExpr * node's fromlist) */ @@ -234,7 +239,8 @@ struct ParseState * join's first N columns, the net effect is just that we expose only those * join columns via this nsitem.) * - * p_rte and p_rtindex link to the underlying rangetable entry. + * p_rte and p_rtindex link to the underlying rangetable entry, and + * p_perminfo to the entry in rtepermlist. * * The p_nscolumns array contains info showing how to construct Vars * referencing the names appearing in the p_names->colnames list. @@ -271,6 +277,7 @@ struct ParseNamespaceItem Alias *p_names; /* Table and column names */ RangeTblEntry *p_rte; /* The relation's rangetable entry */ int p_rtindex; /* The relation's index in the rangetable */ + RTEPermissionInfo *p_perminfo; /* The relation's rtepermlist entry */ /* array of same length as p_names->colnames: */ ParseNamespaceColumn *p_nscolumns; /* per-column data */ bool p_rel_visible; /* Relation name is visible? */ diff --git a/src/include/parser/parse_relation.h b/src/include/parser/parse_relation.h index 484db165db..3cf475513b 100644 --- a/src/include/parser/parse_relation.h +++ b/src/include/parser/parse_relation.h @@ -120,5 +120,9 @@ extern const NameData *attnumAttName(Relation rd, int attid); extern Oid attnumTypeId(Relation rd, int attid); extern Oid attnumCollationId(Relation rd, int attid); extern bool isQueryUsingTempRelation(Query *query); +extern RTEPermissionInfo *AddRTEPermissionInfo(List **rtepermlist, + RangeTblEntry *rte); +extern RTEPermissionInfo *GetRTEPermissionInfo(List *rtepermlist, + RangeTblEntry *rte); #endif /* PARSE_RELATION_H */ diff --git a/src/include/rewrite/rewriteHandler.h b/src/include/rewrite/rewriteHandler.h index 90ecf109af..05c3680cd6 100644 --- a/src/include/rewrite/rewriteHandler.h +++ b/src/include/rewrite/rewriteHandler.h @@ -25,6 +25,7 @@ extern void AcquireRewriteLocks(Query *parsetree, extern Node *build_column_default(Relation rel, int attrno); extern void fill_extraUpdatedCols(RangeTblEntry *target_rte, + RTEPermissionInfo *target_perminfo, Relation target_relation); extern Query *get_view_query(Relation view); diff --git a/src/include/rewrite/rewriteManip.h b/src/include/rewrite/rewriteManip.h index f001ca41bb..2f56f7a6ac 100644 --- a/src/include/rewrite/rewriteManip.h +++ b/src/include/rewrite/rewriteManip.h @@ -83,5 +83,8 @@ extern Node *ReplaceVarsFromTargetList(Node *node, ReplaceVarsNoMatchOption nomatch_option, int nomatch_varno, bool *outer_hasSubLinks); +extern List *CombineRangeTables(List *rtable1, List *rtable2, + List *rtepermlist2, + List **rtepermlist1); #endif /* REWRITEMANIP_H */ diff --git a/src/test/modules/test_oat_hooks/test_oat_hooks.c b/src/test/modules/test_oat_hooks/test_oat_hooks.c index 4b4e259cd2..bfa9263233 100644 --- a/src/test/modules/test_oat_hooks/test_oat_hooks.c +++ b/src/test/modules/test_oat_hooks/test_oat_hooks.c @@ -46,7 +46,7 @@ static bool REGRESS_suset_variable2 = false; /* Saved hook values */ static object_access_hook_type next_object_access_hook = NULL; static object_access_hook_type_str next_object_access_hook_str = NULL; -static ExecutorCheckPerms_hook_type next_exec_check_perms_hook = NULL; +static ExecutorCheckPermissions_hook_type next_exec_check_perms_hook = NULL; static ProcessUtility_hook_type next_ProcessUtility_hook = NULL; /* Test Object Access Type Hook hooks */ @@ -55,7 +55,7 @@ static void REGRESS_object_access_hook_str(ObjectAccessType access, int subId, void *arg); static void REGRESS_object_access_hook(ObjectAccessType access, Oid classId, Oid objectId, int subId, void *arg); -static bool REGRESS_exec_check_perms(List *rangeTabls, bool do_abort); +static bool REGRESS_exec_check_perms(List *rangeTabls, List *rtepermlist, bool do_abort); static void REGRESS_utility_command(PlannedStmt *pstmt, const char *queryString, bool readOnlyTree, ProcessUtilityContext context, @@ -219,8 +219,8 @@ _PG_init(void) object_access_hook_str = REGRESS_object_access_hook_str; /* DML permission check */ - next_exec_check_perms_hook = ExecutorCheckPerms_hook; - ExecutorCheckPerms_hook = REGRESS_exec_check_perms; + next_exec_check_perms_hook = ExecutorCheckPermissions_hook; + ExecutorCheckPermissions_hook = REGRESS_exec_check_perms; /* ProcessUtility hook */ next_ProcessUtility_hook = ProcessUtility_hook; @@ -345,7 +345,7 @@ REGRESS_object_access_hook(ObjectAccessType access, Oid classId, Oid objectId, i } static bool -REGRESS_exec_check_perms(List *rangeTabls, bool do_abort) +REGRESS_exec_check_perms(List *rangeTabls, List *rtepermlist, bool do_abort) { bool am_super = superuser_arg(GetUserId()); bool allow = true; @@ -361,7 +361,7 @@ REGRESS_exec_check_perms(List *rangeTabls, bool do_abort) /* Forward to next hook in the chain */ if (next_exec_check_perms_hook && - !(*next_exec_check_perms_hook) (rangeTabls, do_abort)) + !(*next_exec_check_perms_hook) (rangeTabls, rtepermlist, do_abort)) allow = false; if (allow) diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out index 37c1c86473..e1cdaf9eb6 100644 --- a/src/test/regress/expected/rules.out +++ b/src/test/regress/expected/rules.out @@ -3584,6 +3584,18 @@ CREATE RULE rule1 AS ON INSERT TO ruletest_v1 SET SESSION AUTHORIZATION regress_rule_user1; INSERT INTO ruletest_v1 VALUES (1); RESET SESSION AUTHORIZATION; +-- Test that main query's relation's permissions are checked before +-- the rule action's relation's. +CREATE TABLE ruletest_t3 (x int); +CREATE RULE rule2 AS ON UPDATE TO ruletest_t1 + DO INSTEAD INSERT INTO ruletest_t2 VALUES (OLD.*); +REVOKE ALL ON ruletest_t2 FROM regress_rule_user1; +REVOKE ALL ON ruletest_t3 FROM regress_rule_user1; +ALTER TABLE ruletest_t1 OWNER TO regress_rule_user1; +SET SESSION AUTHORIZATION regress_rule_user1; +UPDATE ruletest_t1 t1 SET x = 0 FROM ruletest_t3 t3 WHERE t1.x = t3.x; +ERROR: permission denied for table ruletest_t3 +RESET SESSION AUTHORIZATION; SELECT * FROM ruletest_t1; x --- @@ -3596,6 +3608,8 @@ SELECT * FROM ruletest_t2; (1 row) DROP VIEW ruletest_v1; +DROP RULE rule2 ON ruletest_t1; +DROP TABLE ruletest_t3; DROP TABLE ruletest_t2; DROP TABLE ruletest_t1; DROP USER regress_rule_user1; diff --git a/src/test/regress/sql/rules.sql b/src/test/regress/sql/rules.sql index bfb5f3b0bb..2f7cb8482a 100644 --- a/src/test/regress/sql/rules.sql +++ b/src/test/regress/sql/rules.sql @@ -1309,11 +1309,26 @@ CREATE RULE rule1 AS ON INSERT TO ruletest_v1 SET SESSION AUTHORIZATION regress_rule_user1; INSERT INTO ruletest_v1 VALUES (1); +RESET SESSION AUTHORIZATION; + +-- Test that main query's relation's permissions are checked before +-- the rule action's relation's. +CREATE TABLE ruletest_t3 (x int); +CREATE RULE rule2 AS ON UPDATE TO ruletest_t1 + DO INSTEAD INSERT INTO ruletest_t2 VALUES (OLD.*); +REVOKE ALL ON ruletest_t2 FROM regress_rule_user1; +REVOKE ALL ON ruletest_t3 FROM regress_rule_user1; +ALTER TABLE ruletest_t1 OWNER TO regress_rule_user1; +SET SESSION AUTHORIZATION regress_rule_user1; +UPDATE ruletest_t1 t1 SET x = 0 FROM ruletest_t3 t3 WHERE t1.x = t3.x; + RESET SESSION AUTHORIZATION; SELECT * FROM ruletest_t1; SELECT * FROM ruletest_t2; DROP VIEW ruletest_v1; +DROP RULE rule2 ON ruletest_t1; +DROP TABLE ruletest_t3; DROP TABLE ruletest_t2; DROP TABLE ruletest_t1; diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 2f5802195d..1d0134db8d 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -2188,6 +2188,7 @@ RI_ConstraintInfo RI_QueryHashEntry RI_QueryKey RTEKind +RTEPermissionInfo RWConflict RWConflictPoolHeader Range @@ -3265,6 +3266,7 @@ fix_scan_expr_context fix_upper_expr_context fix_windowagg_cond_context flatten_join_alias_vars_context +flatten_rtes_walker_context float4 float4KEY float8 -- 2.30.2