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

Author: anarazel

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;

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,13 +243,16 @@ 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__UPDATE)
-				column_perms |= SEPG_DB_COLUMN__UPDATE;
 			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 (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,23 +338,27 @@ 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
 			 * up them.
 			 */
 			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;
 		}

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);
 		}

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

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);
 	}