Relax INSERT privilege requirement for CTAS and matviews WITH NO DATA

When specified, WITH NO DATA does not insert any data into the relation
created, so skip checking for the insert permissions.  With WITH DATA or
WITH NO DATA, it is always required for the user to have CREATE
privileges on the schema targeted for the relation.

Note that plain CREATE TABLE AS or CREATE MATERIALIZED VIEW queries have
begun to work accidentally without INSERT privilege checks as of
874fe3ae, while using EXECUTE or EXPLAIN ANALYZE would fail with the ACL
check, so this makes the behavior for all the command flavors consistent
with each other.  This is arguably a bug fix, but there have been no
complaints about the current behavior either so stable branches are not
changed.

While on it, document properly the privileges requirements for each
commands with more tests for all the scenarios possible, and avoid a
useless bulk-insert allocation when using WITH NO DATA.

Author: Bharath Rupireddy
Reviewed-by: Anastasia Lubennikova, Michael Paquier
Discussion: https://postgr.es/m/CALj2ACWc3N8j0_9nMPz9wcAUnVcdKHzFdDZJ3hVFNEbqtcyG9w@mail.gmail.com

1 files changed, 51 insertions, 33 deletions

diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c
index d53ec952d07..37649eafa88 100644
--- a/src/backend/commands/createas.c
+++ b/src/backend/commands/createas.c
@@ -436,7 +436,6 @@ intorel_startup(DestReceiver *self, int operation, TupleDesc typeinfo)
 	List	   *attrList;
 	ObjectAddress intoRelationAddr;
 	Relation	intoRelationDesc;
-	RangeTblEntry *rte;
 	ListCell   *lc;
 	int			attnum;
 
@@ -507,23 +506,28 @@ intorel_startup(DestReceiver *self, int operation, TupleDesc typeinfo)
 	intoRelationDesc = table_open(intoRelationAddr.objectId, AccessExclusiveLock);
 
 	/*
-	 * Check INSERT permission on the constructed table.
-	 *
-	 * XXX: It would arguably make sense to skip this check if into->skipData
-	 * is true.
+	 * Check INSERT permission on the constructed table.  Skip this check if
+	 * WITH NO DATA is specified as only a table gets created with no tuples
+	 * inserted, that is a case possible when using EXPLAIN ANALYZE or
+	 * EXECUTE.
 	 */
-	rte = makeNode(RangeTblEntry);
-	rte->rtekind = RTE_RELATION;
-	rte->relid = intoRelationAddr.objectId;
-	rte->relkind = relkind;
-	rte->rellockmode = RowExclusiveLock;
-	rte->requiredPerms = ACL_INSERT;
+	if (!into->skipData)
+	{
+		RangeTblEntry *rte;
 
-	for (attnum = 1; attnum <= intoRelationDesc->rd_att->natts; attnum++)
-		rte->insertedCols = bms_add_member(rte->insertedCols,
-										   attnum - FirstLowInvalidHeapAttributeNumber);
+		rte = makeNode(RangeTblEntry);
+		rte->rtekind = RTE_RELATION;
+		rte->relid = intoRelationAddr.objectId;
+		rte->relkind = relkind;
+		rte->rellockmode = RowExclusiveLock;
+		rte->requiredPerms = ACL_INSERT;
 
-	ExecCheckRTPerms(list_make1(rte), true);
+		for (attnum = 1; attnum <= intoRelationDesc->rd_att->natts; attnum++)
+			rte->insertedCols = bms_add_member(rte->insertedCols,
+											   attnum - FirstLowInvalidHeapAttributeNumber);
+
+		ExecCheckRTPerms(list_make1(rte), true);
+	}
 
 	/*
 	 * Make sure the constructed table does not have RLS enabled.
@@ -552,7 +556,15 @@ intorel_startup(DestReceiver *self, int operation, TupleDesc typeinfo)
 	myState->reladdr = intoRelationAddr;
 	myState->output_cid = GetCurrentCommandId(true);
 	myState->ti_options = TABLE_INSERT_SKIP_FSM;
-	myState->bistate = GetBulkInsertState();
+
+	/*
+	 * If WITH NO DATA is specified, there is no need to set up the state for
+	 * bulk inserts as there are no tuples to insert.
+	 */
+	if (!into->skipData)
+		myState->bistate = GetBulkInsertState();
+	else
+		myState->bistate = NULL;
 
 	/*
 	 * Valid smgr_targblock implies something already wrote to the relation.
@@ -569,20 +581,23 @@ intorel_receive(TupleTableSlot *slot, DestReceiver *self)
 {
 	DR_intorel *myState = (DR_intorel *) self;
 
-	/*
-	 * Note that the input slot might not be of the type of the target
-	 * relation. That's supported by table_tuple_insert(), but slightly less
-	 * efficient than inserting with the right slot - but the alternative
-	 * would be to copy into a slot of the right type, which would not be
-	 * cheap either. This also doesn't allow accessing per-AM data (say a
-	 * tuple's xmin), but since we don't do that here...
-	 */
-
-	table_tuple_insert(myState->rel,
-					   slot,
-					   myState->output_cid,
-					   myState->ti_options,
-					   myState->bistate);
+	/* Nothing to insert if WITH NO DATA is specified. */
+	if (!myState->into->skipData)
+	{
+		/*
+		 * Note that the input slot might not be of the type of the target
+		 * relation. That's supported by table_tuple_insert(), but slightly
+		 * less efficient than inserting with the right slot - but the
+		 * alternative would be to copy into a slot of the right type, which
+		 * would not be cheap either. This also doesn't allow accessing per-AM
+		 * data (say a tuple's xmin), but since we don't do that here...
+		 */
+		table_tuple_insert(myState->rel,
+						   slot,
+						   myState->output_cid,
+						   myState->ti_options,
+						   myState->bistate);
+	}
 
 	/* We know this is a newly created relation, so there are no indexes */
 
@@ -596,10 +611,13 @@ static void
 intorel_shutdown(DestReceiver *self)
 {
 	DR_intorel *myState = (DR_intorel *) self;
+	IntoClause *into = myState->into;
 
-	FreeBulkInsertState(myState->bistate);
-
-	table_finish_bulk_insert(myState->rel, myState->ti_options);
+	if (!into->skipData)
+	{
+		FreeBulkInsertState(myState->bistate);
+		table_finish_bulk_insert(myState->rel, myState->ti_options);
+	}
 
 	/* close rel, but keep lock until commit */
 	table_close(myState->rel, NoLock);