From 9777849fd2b05e4f55101d7423dd18c3642e5829 Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy Date: Wed, 11 Nov 2020 19:17:32 +0530 Subject: [PATCH v2] Skip Insert Perm Check & Bulk Insert State alloc in CTAS with no data In CTAS with no data, we actually do not insert the tuples into the created table, so we can skip checking for the insert permissions. Anyways, the insert permissions will be checked when the tuples are inserted into the table. And also, do not allocate bulk insert state i.e. 16MB of data just to free it in intorel_shutdown() without having to use it. --- src/backend/commands/createas.c | 87 ++++++++++++++--------- src/test/regress/expected/select_into.out | 17 ++++- src/test/regress/sql/select_into.sql | 5 ++ 3 files changed, 73 insertions(+), 36 deletions(-) diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c index d53ec952d0..b17c5b4dfa 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 we do not actually insert the tuples, we + * just create the table. The insert permissions will be checked anyways + * while inserting tuples into the table. */ - 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; + + rte = makeNode(RangeTblEntry); + rte->rtekind = RTE_RELATION; + rte->relid = intoRelationAddr.objectId; + rte->relkind = relkind; + rte->rellockmode = RowExclusiveLock; + rte->requiredPerms = ACL_INSERT; - for (attnum = 1; attnum <= intoRelationDesc->rd_att->natts; attnum++) - rte->insertedCols = bms_add_member(rte->insertedCols, - attnum - FirstLowInvalidHeapAttributeNumber); + for (attnum = 1; attnum <= intoRelationDesc->rd_att->natts; attnum++) + rte->insertedCols = bms_add_member(rte->insertedCols, + attnum - FirstLowInvalidHeapAttributeNumber); - ExecCheckRTPerms(list_make1(rte), true); + ExecCheckRTPerms(list_make1(rte), true); + } /* * Make sure the constructed table does not have RLS enabled. @@ -550,9 +554,15 @@ intorel_startup(DestReceiver *self, int operation, TupleDesc typeinfo) */ myState->rel = intoRelationDesc; myState->reladdr = intoRelationAddr; - myState->output_cid = GetCurrentCommandId(true); myState->ti_options = TABLE_INSERT_SKIP_FSM; - myState->bistate = GetBulkInsertState(); + myState->output_cid = GetCurrentCommandId(true); + + /* + * In case if WITH NO DATA is specified, we do not allocate bulk insert + * state as we do not have tuples to insert. + */ + if (!into->skipData) + myState->bistate = GetBulkInsertState(); /* * Valid smgr_targblock implies something already wrote to the relation. @@ -569,20 +579,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); + /* Do not insert in case 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 */ @@ -597,9 +610,15 @@ intorel_shutdown(DestReceiver *self) { DR_intorel *myState = (DR_intorel *) self; - FreeBulkInsertState(myState->bistate); - - table_finish_bulk_insert(myState->rel, myState->ti_options); + /* + * In case if WITH NO DATA is specified, we do not allocate bulk insert + * state as we do not have tuples to insert. + */ + if (!myState->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); diff --git a/src/test/regress/expected/select_into.out b/src/test/regress/expected/select_into.out index f373fae679..0e10eb39cf 100644 --- a/src/test/regress/expected/select_into.out +++ b/src/test/regress/expected/select_into.out @@ -31,6 +31,18 @@ CREATE TABLE selinto_schema.tmp3 (a,b,c) AS SELECT oid,relname,relacl FROM pg_class WHERE relname like '%c%'; -- Error ERROR: permission denied for table tmp3 +EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF) + CREATE TABLE selinto_schema.tmp0 (a) AS + SELECT oid FROM pg_class WHERE relname like '%c%' WITH NO DATA; -- OK + QUERY PLAN +--------------------------------------- + Seq Scan on pg_class (never executed) + Filter: (relname ~~ '%c%'::text) +(2 rows) + +INSERT INTO selinto_schema.tmp0 + SELECT oid FROM pg_class WHERE relname like '%c%'; -- Error +ERROR: permission denied for table tmp0 RESET SESSION AUTHORIZATION; ALTER DEFAULT PRIVILEGES FOR ROLE regress_selinto_user GRANT INSERT ON TABLES TO regress_selinto_user; @@ -45,8 +57,9 @@ CREATE TABLE selinto_schema.tmp3 (a,b,c) WHERE relname like '%c%'; -- OK RESET SESSION AUTHORIZATION; DROP SCHEMA selinto_schema CASCADE; -NOTICE: drop cascades to 3 other objects -DETAIL: drop cascades to table selinto_schema.tmp1 +NOTICE: drop cascades to 4 other objects +DETAIL: drop cascades to table selinto_schema.tmp0 +drop cascades to table selinto_schema.tmp1 drop cascades to table selinto_schema.tmp2 drop cascades to table selinto_schema.tmp3 DROP USER regress_selinto_user; diff --git a/src/test/regress/sql/select_into.sql b/src/test/regress/sql/select_into.sql index a708fef0ea..c6e157a465 100644 --- a/src/test/regress/sql/select_into.sql +++ b/src/test/regress/sql/select_into.sql @@ -34,6 +34,11 @@ SELECT oid AS clsoid, relname, relnatts + 10 AS x CREATE TABLE selinto_schema.tmp3 (a,b,c) AS SELECT oid,relname,relacl FROM pg_class WHERE relname like '%c%'; -- Error +EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF) + CREATE TABLE selinto_schema.tmp0 (a) AS + SELECT oid FROM pg_class WHERE relname like '%c%' WITH NO DATA; -- OK +INSERT INTO selinto_schema.tmp0 + SELECT oid FROM pg_class WHERE relname like '%c%'; -- Error RESET SESSION AUTHORIZATION; ALTER DEFAULT PRIVILEGES FOR ROLE regress_selinto_user -- 2.25.1