From 2c8f4836db058d0715bc30a30655d646287ba509 Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Fri, 8 May 2015 00:20:46 +0200 Subject: [PATCH] Represent columns requiring insert and update privileges indentently. Previously, relation range table entries used a single Bitmapset field representing which columns required either UPDATE or INSERT privileges, despite the fact that INSERT and UPDATE privileges are separately cataloged, and may be independently held. As statements so far required either insert or update privileges but never both, that was sufficient. The required permission could be inferred from the top level statement run. The upcoming INSERT ... ON CONFLICT UPDATE feature needs to independently check for both privileges in one statement though, so that is not sufficient anymore. Bumps catversion as stored rules change. Author: Peter Geoghegan Reviewed-By: Andres Freund --- contrib/postgres_fdw/postgres_fdw.c | 2 +- contrib/sepgsql/dml.c | 31 ++++-- src/backend/commands/copy.c | 2 +- src/backend/commands/createas.c | 2 +- src/backend/commands/trigger.c | 30 +++--- src/backend/executor/execMain.c | 126 ++++++++++++++-------- src/backend/nodes/copyfuncs.c | 3 +- src/backend/nodes/equalfuncs.c | 3 +- src/backend/nodes/outfuncs.c | 3 +- src/backend/nodes/readfuncs.c | 3 +- src/backend/optimizer/plan/setrefs.c | 6 +- src/backend/optimizer/prep/prepsecurity.c | 6 +- src/backend/optimizer/prep/prepunion.c | 8 +- src/backend/parser/analyze.c | 4 +- src/backend/parser/parse_relation.c | 21 ++-- src/backend/rewrite/rewriteHandler.c | 52 +++++---- src/include/catalog/catversion.h | 2 +- src/include/nodes/parsenodes.h | 14 +-- 18 files changed, 193 insertions(+), 125 deletions(-) diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c index 478e12484b..de732319d7 100644 --- a/contrib/postgres_fdw/postgres_fdw.c +++ b/contrib/postgres_fdw/postgres_fdw.c @@ -1205,7 +1205,7 @@ postgresPlanForeignModify(PlannerInfo *root, int col; col = -1; - while ((col = bms_next_member(rte->modifiedCols, col)) >= 0) + while ((col = bms_next_member(rte->updatedCols, col)) >= 0) { /* bit numbers are offset by FirstLowInvalidHeapAttributeNumber */ AttrNumber attno = col + FirstLowInvalidHeapAttributeNumber; diff --git a/contrib/sepgsql/dml.c b/contrib/sepgsql/dml.c index 36c6a37ac1..4a71753d3f 100644 --- a/contrib/sepgsql/dml.c +++ b/contrib/sepgsql/dml.c @@ -145,7 +145,8 @@ fixup_inherited_columns(Oid parentId, Oid childId, Bitmapset *columns) static bool check_relation_privileges(Oid relOid, Bitmapset *selected, - Bitmapset *modified, + Bitmapset *inserted, + Bitmapset *updated, uint32 required, bool abort_on_violation) { @@ -231,8 +232,9 @@ check_relation_privileges(Oid relOid, * Check permissions on the columns */ selected = fixup_whole_row_references(relOid, selected); - modified = fixup_whole_row_references(relOid, modified); - columns = bms_union(selected, modified); + inserted = fixup_whole_row_references(relOid, inserted); + updated = fixup_whole_row_references(relOid, updated); + columns = bms_union(selected, bms_union(inserted, updated)); while ((index = bms_first_member(columns)) >= 0) { @@ -241,12 +243,15 @@ check_relation_privileges(Oid relOid, if (bms_is_member(index, selected)) column_perms |= SEPG_DB_COLUMN__SELECT; - if (bms_is_member(index, modified)) + if (bms_is_member(index, inserted)) + { + if (required & SEPG_DB_TABLE__INSERT) + column_perms |= SEPG_DB_COLUMN__INSERT; + } + if (bms_is_member(index, updated)) { if (required & SEPG_DB_TABLE__UPDATE) column_perms |= SEPG_DB_COLUMN__UPDATE; - if (required & SEPG_DB_TABLE__INSERT) - column_perms |= SEPG_DB_COLUMN__INSERT; } if (column_perms == 0) continue; @@ -304,7 +309,7 @@ sepgsql_dml_privileges(List *rangeTabls, bool abort_on_violation) required |= SEPG_DB_TABLE__INSERT; if (rte->requiredPerms & ACL_UPDATE) { - if (!bms_is_empty(rte->modifiedCols)) + if (!bms_is_empty(rte->updatedCols)) required |= SEPG_DB_TABLE__UPDATE; else required |= SEPG_DB_TABLE__LOCK; @@ -333,7 +338,8 @@ sepgsql_dml_privileges(List *rangeTabls, bool abort_on_violation) { Oid tableOid = lfirst_oid(li); Bitmapset *selectedCols; - Bitmapset *modifiedCols; + Bitmapset *insertedCols; + Bitmapset *updatedCols; /* * child table has different attribute numbers, so we need to fix @@ -341,15 +347,18 @@ sepgsql_dml_privileges(List *rangeTabls, bool abort_on_violation) */ selectedCols = fixup_inherited_columns(rte->relid, tableOid, rte->selectedCols); - modifiedCols = fixup_inherited_columns(rte->relid, tableOid, - rte->modifiedCols); + insertedCols = fixup_inherited_columns(rte->relid, tableOid, + rte->insertedCols); + updatedCols = fixup_inherited_columns(rte->relid, tableOid, + rte->updatedCols); /* * check permissions on individual tables */ if (!check_relation_privileges(tableOid, selectedCols, - modifiedCols, + insertedCols, + updatedCols, required, abort_on_violation)) return false; } diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c index 0d3721a96d..aa8ae4b9bc 100644 --- a/src/backend/commands/copy.c +++ b/src/backend/commands/copy.c @@ -847,7 +847,7 @@ DoCopy(const CopyStmt *stmt, const char *queryString, uint64 *processed) FirstLowInvalidHeapAttributeNumber; if (is_from) - rte->modifiedCols = bms_add_member(rte->modifiedCols, attno); + rte->insertedCols = bms_add_member(rte->insertedCols, attno); else rte->selectedCols = bms_add_member(rte->selectedCols, attno); } diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c index 54b2f382ea..e8f0d793b6 100644 --- a/src/backend/commands/createas.c +++ b/src/backend/commands/createas.c @@ -433,7 +433,7 @@ intorel_startup(DestReceiver *self, int operation, TupleDesc typeinfo) rte->requiredPerms = ACL_INSERT; for (attnum = 1; attnum <= intoRelationDesc->rd_att->natts; attnum++) - rte->modifiedCols = bms_add_member(rte->modifiedCols, + rte->insertedCols = bms_add_member(rte->insertedCols, attnum - FirstLowInvalidHeapAttributeNumber); ExecCheckRTPerms(list_make1(rte), true); diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index 098893f494..222e7fce85 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -66,13 +66,13 @@ int SessionReplicationRole = SESSION_REPLICATION_ROLE_ORIGIN; static int MyTriggerDepth = 0; /* - * Note that this macro also exists in executor/execMain.c. There does not - * appear to be any good header to put it into, given the structures that - * it uses, so we let them be duplicated. Be sure to update both if one needs - * to be changed, however. + * Note that similar macros also exists in executor/execMain.c. There does not + * appear to be any good header to put it into, given the structures that it + * uses, so we let them be duplicated. Be sure to update both if one needs to + * be changed, however. */ -#define GetModifiedColumns(relinfo, estate) \ - (rt_fetch((relinfo)->ri_RangeTableIndex, (estate)->es_range_table)->modifiedCols) +#define GetUpdatedColumns(relinfo, estate) \ + (rt_fetch((relinfo)->ri_RangeTableIndex, (estate)->es_range_table)->updatedCols) /* Local function prototypes */ static void ConvertTriggerToFK(CreateTrigStmt *stmt, Oid funcoid); @@ -2347,7 +2347,7 @@ ExecBSUpdateTriggers(EState *estate, ResultRelInfo *relinfo) TriggerDesc *trigdesc; int i; TriggerData LocTriggerData; - Bitmapset *modifiedCols; + Bitmapset *updatedCols; trigdesc = relinfo->ri_TrigDesc; @@ -2356,7 +2356,7 @@ ExecBSUpdateTriggers(EState *estate, ResultRelInfo *relinfo) if (!trigdesc->trig_update_before_statement) return; - modifiedCols = GetModifiedColumns(relinfo, estate); + updatedCols = GetUpdatedColumns(relinfo, estate); LocTriggerData.type = T_TriggerData; LocTriggerData.tg_event = TRIGGER_EVENT_UPDATE | @@ -2377,7 +2377,7 @@ ExecBSUpdateTriggers(EState *estate, ResultRelInfo *relinfo) TRIGGER_TYPE_UPDATE)) continue; if (!TriggerEnabled(estate, relinfo, trigger, LocTriggerData.tg_event, - modifiedCols, NULL, NULL)) + updatedCols, NULL, NULL)) continue; LocTriggerData.tg_trigger = trigger; @@ -2402,7 +2402,7 @@ ExecASUpdateTriggers(EState *estate, ResultRelInfo *relinfo) if (trigdesc && trigdesc->trig_update_after_statement) AfterTriggerSaveEvent(estate, relinfo, TRIGGER_EVENT_UPDATE, false, NULL, NULL, NIL, - GetModifiedColumns(relinfo, estate)); + GetUpdatedColumns(relinfo, estate)); } TupleTableSlot * @@ -2420,7 +2420,7 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate, HeapTuple oldtuple; TupleTableSlot *newSlot; int i; - Bitmapset *modifiedCols; + Bitmapset *updatedCols; Bitmapset *keyCols; LockTupleMode lockmode; @@ -2429,10 +2429,10 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate, * been modified, then we can use a weaker lock, allowing for better * concurrency. */ - modifiedCols = GetModifiedColumns(relinfo, estate); + updatedCols = GetUpdatedColumns(relinfo, estate); keyCols = RelationGetIndexAttrBitmap(relinfo->ri_RelationDesc, INDEX_ATTR_BITMAP_KEY); - if (bms_overlap(keyCols, modifiedCols)) + if (bms_overlap(keyCols, updatedCols)) lockmode = LockTupleExclusive; else lockmode = LockTupleNoKeyExclusive; @@ -2486,7 +2486,7 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate, TRIGGER_TYPE_UPDATE)) continue; if (!TriggerEnabled(estate, relinfo, trigger, LocTriggerData.tg_event, - modifiedCols, trigtuple, newtuple)) + updatedCols, trigtuple, newtuple)) continue; LocTriggerData.tg_trigtuple = trigtuple; @@ -2556,7 +2556,7 @@ ExecARUpdateTriggers(EState *estate, ResultRelInfo *relinfo, AfterTriggerSaveEvent(estate, relinfo, TRIGGER_EVENT_UPDATE, true, trigtuple, newtuple, recheckIndexes, - GetModifiedColumns(relinfo, estate)); + GetUpdatedColumns(relinfo, estate)); if (trigtuple != fdw_trigtuple) heap_freetuple(trigtuple); } diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index 26793eecf4..4272d9bc15 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -82,6 +82,9 @@ static void ExecutePlan(EState *estate, PlanState *planstate, ScanDirection direction, DestReceiver *dest); static bool ExecCheckRTEPerms(RangeTblEntry *rte); +static bool ExecCheckRTEPermsModified(Oid relOid, Oid userid, + Bitmapset *modifiedCols, + AclMode requiredPerms); static void ExecCheckXactReadOnly(PlannedStmt *plannedstmt); static char *ExecBuildSlotValueDescription(Oid reloid, TupleTableSlot *slot, @@ -92,13 +95,15 @@ static void EvalPlanQualStart(EPQState *epqstate, EState *parentestate, Plan *planTree); /* - * Note that this macro also exists in commands/trigger.c. There does not - * appear to be any good header to put it into, given the structures that + * Note that GetUpdatedColumns() also exists in commands/trigger.c. There does + * not appear to be any good header to put it into, given the structures that * it uses, so we let them be duplicated. Be sure to update both if one needs * to be changed, however. */ -#define GetModifiedColumns(relinfo, estate) \ - (rt_fetch((relinfo)->ri_RangeTableIndex, (estate)->es_range_table)->modifiedCols) +#define GetInsertedColumns(relinfo, estate) \ + (rt_fetch((relinfo)->ri_RangeTableIndex, (estate)->es_range_table)->insertedCols) +#define GetUpdatedColumns(relinfo, estate) \ + (rt_fetch((relinfo)->ri_RangeTableIndex, (estate)->es_range_table)->updatedCols) /* end of local decls */ @@ -571,7 +576,6 @@ ExecCheckRTEPerms(RangeTblEntry *rte) AclMode remainingPerms; Oid relOid; Oid userid; - int col; /* * Only plain-relation RTEs need to be checked here. Function RTEs are @@ -609,6 +613,8 @@ ExecCheckRTEPerms(RangeTblEntry *rte) remainingPerms = requiredPerms & ~relPerms; if (remainingPerms != 0) { + int col = -1; + /* * If we lack any permissions that exist only as relation permissions, * we can fail straight away. @@ -637,7 +643,6 @@ ExecCheckRTEPerms(RangeTblEntry *rte) return false; } - col = -1; while ((col = bms_next_member(rte->selectedCols, col)) >= 0) { /* bit #s are offset by FirstLowInvalidHeapAttributeNumber */ @@ -660,43 +665,63 @@ ExecCheckRTEPerms(RangeTblEntry *rte) } /* - * Basically the same for the mod columns, with either INSERT or - * UPDATE privilege as specified by remainingPerms. + * Basically the same for the mod columns, for both INSERT and UPDATE + * privilege as specified by remainingPerms. */ - remainingPerms &= ~ACL_SELECT; - if (remainingPerms != 0) + if (remainingPerms & ACL_INSERT && !ExecCheckRTEPermsModified(relOid, + userid, + rte->insertedCols, + ACL_INSERT)) + return false; + + if (remainingPerms & ACL_UPDATE && !ExecCheckRTEPermsModified(relOid, + userid, + rte->updatedCols, + ACL_UPDATE)) + return false; + } + return true; +} + +/* + * ExecCheckRTEPermsModified + * Check INSERT or UPDATE access permissions for a single RTE (these + * are processed uniformly). + */ +static bool +ExecCheckRTEPermsModified(Oid relOid, Oid userid, Bitmapset *modifiedCols, + AclMode requiredPerms) +{ + int col = -1; + + /* + * When the query doesn't explicitly update any columns, allow the + * query if we have permission on any column of the rel. This is + * to handle SELECT FOR UPDATE as well as possible corner cases in + * UPDATE. + */ + if (bms_is_empty(modifiedCols)) + { + if (pg_attribute_aclcheck_all(relOid, userid, requiredPerms, + ACLMASK_ANY) != ACLCHECK_OK) + return false; + } + + while ((col = bms_next_member(modifiedCols, col)) >= 0) + { + /* bit #s are offset by FirstLowInvalidHeapAttributeNumber */ + AttrNumber attno = col + FirstLowInvalidHeapAttributeNumber; + + if (attno == InvalidAttrNumber) { - /* - * When the query doesn't explicitly change any columns, allow the - * query if we have permission on any column of the rel. This is - * to handle SELECT FOR UPDATE as well as possible corner cases in - * INSERT and UPDATE. - */ - if (bms_is_empty(rte->modifiedCols)) - { - if (pg_attribute_aclcheck_all(relOid, userid, remainingPerms, - ACLMASK_ANY) != ACLCHECK_OK) - return false; - } - - col = -1; - while ((col = bms_next_member(rte->modifiedCols, col)) >= 0) - { - /* bit #s are offset by FirstLowInvalidHeapAttributeNumber */ - AttrNumber attno = col + FirstLowInvalidHeapAttributeNumber; - - if (attno == InvalidAttrNumber) - { - /* whole-row reference can't happen here */ - elog(ERROR, "whole-row update is not implemented"); - } - else - { - if (pg_attribute_aclcheck(relOid, attno, userid, - remainingPerms) != ACLCHECK_OK) - return false; - } - } + /* whole-row reference can't happen here */ + elog(ERROR, "whole-row update is not implemented"); + } + else + { + if (pg_attribute_aclcheck(relOid, attno, userid, + requiredPerms) != ACLCHECK_OK) + return false; } } return true; @@ -1633,6 +1658,9 @@ ExecConstraints(ResultRelInfo *resultRelInfo, Relation rel = resultRelInfo->ri_RelationDesc; TupleDesc tupdesc = RelationGetDescr(rel); TupleConstr *constr = tupdesc->constr; + Bitmapset *modifiedCols; + Bitmapset *insertedCols; + Bitmapset *updatedCols; Assert(constr); @@ -1647,9 +1675,10 @@ ExecConstraints(ResultRelInfo *resultRelInfo, slot_attisnull(slot, attrChk)) { char *val_desc; - Bitmapset *modifiedCols; - modifiedCols = GetModifiedColumns(resultRelInfo, estate); + insertedCols = GetInsertedColumns(resultRelInfo, estate); + updatedCols = GetUpdatedColumns(resultRelInfo, estate); + modifiedCols = bms_union(insertedCols, updatedCols); val_desc = ExecBuildSlotValueDescription(RelationGetRelid(rel), slot, tupdesc, @@ -1673,9 +1702,10 @@ ExecConstraints(ResultRelInfo *resultRelInfo, if ((failed = ExecRelCheck(resultRelInfo, slot, estate)) != NULL) { char *val_desc; - Bitmapset *modifiedCols; - modifiedCols = GetModifiedColumns(resultRelInfo, estate); + insertedCols = GetInsertedColumns(resultRelInfo, estate); + updatedCols = GetUpdatedColumns(resultRelInfo, estate); + modifiedCols = bms_union(insertedCols, updatedCols); val_desc = ExecBuildSlotValueDescription(RelationGetRelid(rel), slot, tupdesc, @@ -1746,6 +1776,8 @@ ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo, { char *val_desc; Bitmapset *modifiedCols; + Bitmapset *insertedCols; + Bitmapset *updatedCols; switch (wco->kind) { @@ -1758,7 +1790,9 @@ ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo, * as that depends on the USING policy. */ case WCO_VIEW_CHECK: - modifiedCols = GetModifiedColumns(resultRelInfo, estate); + insertedCols = GetInsertedColumns(resultRelInfo, estate); + updatedCols = GetUpdatedColumns(resultRelInfo, estate); + modifiedCols = bms_union(insertedCols, updatedCols); val_desc = ExecBuildSlotValueDescription(RelationGetRelid(rel), slot, tupdesc, diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index 805045d15e..1b02be287c 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -2042,7 +2042,8 @@ _copyRangeTblEntry(const RangeTblEntry *from) COPY_SCALAR_FIELD(requiredPerms); COPY_SCALAR_FIELD(checkAsUser); COPY_BITMAPSET_FIELD(selectedCols); - COPY_BITMAPSET_FIELD(modifiedCols); + COPY_BITMAPSET_FIELD(insertedCols); + COPY_BITMAPSET_FIELD(updatedCols); COPY_NODE_FIELD(securityQuals); return newnode; diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index 578ead5463..1b9a83b93e 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -2352,7 +2352,8 @@ _equalRangeTblEntry(const RangeTblEntry *a, const RangeTblEntry *b) COMPARE_SCALAR_FIELD(requiredPerms); COMPARE_SCALAR_FIELD(checkAsUser); COMPARE_BITMAPSET_FIELD(selectedCols); - COMPARE_BITMAPSET_FIELD(modifiedCols); + COMPARE_BITMAPSET_FIELD(insertedCols); + COMPARE_BITMAPSET_FIELD(updatedCols); COMPARE_NODE_FIELD(securityQuals); return true; diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index f9f948e39b..d5ddd0b359 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -2475,7 +2475,8 @@ _outRangeTblEntry(StringInfo str, const RangeTblEntry *node) WRITE_UINT_FIELD(requiredPerms); WRITE_OID_FIELD(checkAsUser); WRITE_BITMAPSET_FIELD(selectedCols); - WRITE_BITMAPSET_FIELD(modifiedCols); + WRITE_BITMAPSET_FIELD(insertedCols); + WRITE_BITMAPSET_FIELD(updatedCols); WRITE_NODE_FIELD(securityQuals); } diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c index b0cd95da06..d1ced0cc4b 100644 --- a/src/backend/nodes/readfuncs.c +++ b/src/backend/nodes/readfuncs.c @@ -1256,7 +1256,8 @@ _readRangeTblEntry(void) READ_UINT_FIELD(requiredPerms); READ_OID_FIELD(checkAsUser); READ_BITMAPSET_FIELD(selectedCols); - READ_BITMAPSET_FIELD(modifiedCols); + READ_BITMAPSET_FIELD(insertedCols); + READ_BITMAPSET_FIELD(updatedCols); READ_NODE_FIELD(securityQuals); READ_DONE(); diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c index 69ed2a574e..b7d6ff1122 100644 --- a/src/backend/optimizer/plan/setrefs.c +++ b/src/backend/optimizer/plan/setrefs.c @@ -373,9 +373,9 @@ flatten_rtes_walker(Node *node, PlannerGlobal *glob) * * 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 alias and eref Alias fields, which - * are needed by EXPLAIN, and the selectedCols and modifiedCols bitmaps, - * which are needed for executor-startup permissions checking and for + * for cached plans. We keep only the alias and eref Alias fields, which are + * needed by EXPLAIN, and the selectedCols, insertedCols and updatedCols + * bitmaps, which are needed for executor-startup permissions checking and for * trigger event checking. */ static void diff --git a/src/backend/optimizer/prep/prepsecurity.c b/src/backend/optimizer/prep/prepsecurity.c index 0be44c1c2f..c4b61df300 100644 --- a/src/backend/optimizer/prep/prepsecurity.c +++ b/src/backend/optimizer/prep/prepsecurity.c @@ -125,7 +125,8 @@ expand_security_quals(PlannerInfo *root, List *tlist) rte->requiredPerms = 0; rte->checkAsUser = InvalidOid; rte->selectedCols = NULL; - rte->modifiedCols = NULL; + rte->insertedCols = NULL; + rte->updatedCols = NULL; /* * For the most part, Vars referencing the original relation @@ -224,7 +225,8 @@ expand_security_qual(PlannerInfo *root, List *tlist, int rt_index, rte->requiredPerms = 0; rte->checkAsUser = InvalidOid; rte->selectedCols = NULL; - rte->modifiedCols = NULL; + rte->insertedCols = NULL; + rte->updatedCols = NULL; /* * Now deal with any PlanRowMark on this RTE by requesting a lock diff --git a/src/backend/optimizer/prep/prepunion.c b/src/backend/optimizer/prep/prepunion.c index 51b3da21b3..5859748df8 100644 --- a/src/backend/optimizer/prep/prepunion.c +++ b/src/backend/optimizer/prep/prepunion.c @@ -1368,14 +1368,16 @@ expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti) * if this is the parent table, leave copyObject's result alone. * * Note: we need to do this even though the executor won't run any - * permissions checks on the child RTE. The modifiedCols bitmap may - * be examined for trigger-firing purposes. + * permissions checks on the child RTE. The insertedCols/updatedCols + * bitmaps may be examined for trigger-firing purposes. */ if (childOID != parentOID) { childrte->selectedCols = translate_col_privs(rte->selectedCols, appinfo->translated_vars); - childrte->modifiedCols = translate_col_privs(rte->modifiedCols, + childrte->insertedCols = translate_col_privs(rte->insertedCols, + appinfo->translated_vars); + childrte->updatedCols = translate_col_privs(rte->updatedCols, appinfo->translated_vars); } diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c index 4a5a520539..2d320d100b 100644 --- a/src/backend/parser/analyze.c +++ b/src/backend/parser/analyze.c @@ -733,7 +733,7 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt) false); qry->targetList = lappend(qry->targetList, tle); - rte->modifiedCols = bms_add_member(rte->modifiedCols, + rte->insertedCols = bms_add_member(rte->insertedCols, attr_num - FirstLowInvalidHeapAttributeNumber); icols = lnext(icols); @@ -2002,7 +2002,7 @@ transformUpdateStmt(ParseState *pstate, UpdateStmt *stmt) origTarget->location); /* Mark the target column as requiring update permissions */ - target_rte->modifiedCols = bms_add_member(target_rte->modifiedCols, + target_rte->updatedCols = bms_add_member(target_rte->updatedCols, attrno - FirstLowInvalidHeapAttributeNumber); origTargetList = lnext(origTargetList); diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c index ca560ccee1..562c2f54f9 100644 --- a/src/backend/parser/parse_relation.c +++ b/src/backend/parser/parse_relation.c @@ -1218,7 +1218,8 @@ addRangeTableEntry(ParseState *pstate, rte->requiredPerms = ACL_SELECT; rte->checkAsUser = InvalidOid; /* not set-uid by default, either */ rte->selectedCols = NULL; - rte->modifiedCols = NULL; + rte->insertedCols = NULL; + rte->updatedCols = NULL; /* * Add completed RTE to pstate's range table list, but not to join list @@ -1272,7 +1273,8 @@ addRangeTableEntryForRelation(ParseState *pstate, rte->requiredPerms = ACL_SELECT; rte->checkAsUser = InvalidOid; /* not set-uid by default, either */ rte->selectedCols = NULL; - rte->modifiedCols = NULL; + rte->insertedCols = NULL; + rte->updatedCols = NULL; /* * Add completed RTE to pstate's range table list, but not to join list @@ -1351,7 +1353,8 @@ addRangeTableEntryForSubquery(ParseState *pstate, rte->requiredPerms = 0; rte->checkAsUser = InvalidOid; rte->selectedCols = NULL; - rte->modifiedCols = NULL; + rte->insertedCols = NULL; + rte->updatedCols = NULL; /* * Add completed RTE to pstate's range table list, but not to join list @@ -1606,7 +1609,8 @@ addRangeTableEntryForFunction(ParseState *pstate, rte->requiredPerms = 0; rte->checkAsUser = InvalidOid; rte->selectedCols = NULL; - rte->modifiedCols = NULL; + rte->insertedCols = NULL; + rte->updatedCols = NULL; /* * Add completed RTE to pstate's range table list, but not to join list @@ -1679,7 +1683,8 @@ addRangeTableEntryForValues(ParseState *pstate, rte->requiredPerms = 0; rte->checkAsUser = InvalidOid; rte->selectedCols = NULL; - rte->modifiedCols = NULL; + rte->insertedCols = NULL; + rte->updatedCols = NULL; /* * Add completed RTE to pstate's range table list, but not to join list @@ -1748,7 +1753,8 @@ addRangeTableEntryForJoin(ParseState *pstate, rte->requiredPerms = 0; rte->checkAsUser = InvalidOid; rte->selectedCols = NULL; - rte->modifiedCols = NULL; + rte->insertedCols = NULL; + rte->updatedCols = NULL; /* * Add completed RTE to pstate's range table list, but not to join list @@ -1849,7 +1855,8 @@ addRangeTableEntryForCTE(ParseState *pstate, rte->requiredPerms = 0; rte->checkAsUser = InvalidOid; rte->selectedCols = NULL; - rte->modifiedCols = NULL; + rte->insertedCols = NULL; + rte->updatedCols = NULL; /* * Add completed RTE to pstate's range table list, but not to join list diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c index 60c60caf39..0fc47cb786 100644 --- a/src/backend/rewrite/rewriteHandler.c +++ b/src/backend/rewrite/rewriteHandler.c @@ -1403,7 +1403,8 @@ ApplyRetrieveRule(Query *parsetree, rte->requiredPerms = 0; rte->checkAsUser = InvalidOid; rte->selectedCols = NULL; - rte->modifiedCols = NULL; + rte->insertedCols = NULL; + rte->updatedCols = NULL; /* * For the most part, Vars referencing the view should remain as @@ -1466,12 +1467,14 @@ ApplyRetrieveRule(Query *parsetree, subrte->requiredPerms = rte->requiredPerms; subrte->checkAsUser = rte->checkAsUser; subrte->selectedCols = rte->selectedCols; - subrte->modifiedCols = rte->modifiedCols; + subrte->insertedCols = rte->insertedCols; + subrte->updatedCols = rte->updatedCols; rte->requiredPerms = 0; /* no permission check on subquery itself */ rte->checkAsUser = InvalidOid; rte->selectedCols = NULL; - rte->modifiedCols = NULL; + rte->insertedCols = NULL; + rte->updatedCols = NULL; /* * If FOR [KEY] UPDATE/SHARE of view, mark all the contained tables as @@ -2621,9 +2624,9 @@ rewriteTargetView(Query *parsetree, Relation view) /* * For INSERT/UPDATE the modified columns must all be updatable. Note that * we get the modified columns from the query's targetlist, not from the - * result RTE's modifiedCols set, since rewriteTargetListIU may have added - * additional targetlist entries for view defaults, and these must also be - * updatable. + * result RTE's insertedCols and/or updatedCols set, since + * rewriteTargetListIU may have added additional targetlist entries for + * view defaults, and these must also be updatable. */ if (parsetree->commandType != CMD_DELETE) { @@ -2760,28 +2763,33 @@ rewriteTargetView(Query *parsetree, Relation view) * * 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 modifiedCols is empty. We set modifiedCols 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.) + * 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.) * * This step needs the modified view targetlist, so we have to do things * in this order. */ - Assert(bms_is_empty(new_rte->modifiedCols)); - new_rte->modifiedCols = adjust_view_column_set(view_rte->modifiedCols, + Assert(bms_is_empty(new_rte->insertedCols) && + bms_is_empty(new_rte->updatedCols)); + + new_rte->insertedCols = adjust_view_column_set(view_rte->insertedCols, view_targetlist); + new_rte->updatedCols = adjust_view_column_set(view_rte->updatedCols, + view_targetlist); + /* * Move any security barrier quals from the view RTE onto the new target * RTE. Any such quals should now apply to the new target RTE and will diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h index 2444e543a7..3d2c5b2a29 100644 --- a/src/include/catalog/catversion.h +++ b/src/include/catalog/catversion.h @@ -53,6 +53,6 @@ */ /* yyyymmddN */ -#define CATALOG_VERSION_NO 201505071 +#define CATALOG_VERSION_NO 201505081 #endif diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 852eb4fbdc..7d15ef2847 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -735,11 +735,12 @@ typedef struct XmlSerialize * 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 - * modifiedCols (for INSERT/UPDATE; we can tell which from the query type). - * selectedCols and modifiedCols 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. + * insertedCols and/or updatedCols (INSERT with ON CONFLICT 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. *-------------------- */ typedef enum RTEKind @@ -834,7 +835,8 @@ typedef struct RangeTblEntry AclMode requiredPerms; /* bitmask of required access permissions */ Oid checkAsUser; /* if valid, check access as this role */ Bitmapset *selectedCols; /* columns needing SELECT permission */ - Bitmapset *modifiedCols; /* columns needing INSERT/UPDATE permission */ + Bitmapset *insertedCols; /* columns needing INSERT permission */ + Bitmapset *updatedCols; /* columns needing UPDATE permission */ List *securityQuals; /* any security barrier quals to apply */ } RangeTblEntry;