From fdfeacac480fe09a38083c01c4f665799872605d Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy Date: Thu, 12 Nov 2020 16:02:37 +0530 Subject: [PATCH v3] 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. --- .../sgml/ref/create_materialized_view.sgml | 4 +- doc/src/sgml/ref/create_table_as.sgml | 4 +- src/backend/commands/createas.c | 87 +++++++++++-------- src/test/regress/expected/select_into.out | 59 ++++++++++++- src/test/regress/sql/select_into.sql | 27 ++++++ 5 files changed, 143 insertions(+), 38 deletions(-) diff --git a/doc/src/sgml/ref/create_materialized_view.sgml b/doc/src/sgml/ref/create_materialized_view.sgml index 5ba851b687..b8ef54f45b 100644 --- a/doc/src/sgml/ref/create_materialized_view.sgml +++ b/doc/src/sgml/ref/create_materialized_view.sgml @@ -38,7 +38,9 @@ CREATE MATERIALIZED VIEW [ IF NOT EXISTS ] table_name CREATE MATERIALIZED VIEW defines a materialized view of a query. The query is executed and used to populate the view at the time the command is issued (unless WITH NO DATA is used) and may be - refreshed later using REFRESH MATERIALIZED VIEW. + refreshed later using REFRESH MATERIALIZED VIEW. Insert + permission is checked on the materialized view before populating the data + (unless WITH NO DATA is specified). diff --git a/doc/src/sgml/ref/create_table_as.sgml b/doc/src/sgml/ref/create_table_as.sgml index bcbd73b227..416a2c0f94 100644 --- a/doc/src/sgml/ref/create_table_as.sgml +++ b/doc/src/sgml/ref/create_table_as.sgml @@ -41,7 +41,9 @@ CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } | UNLOGGED ] TABLE [ IF NOT EXI The table columns have the names and data types associated with the output columns of the SELECT (except that you can override the column - names by giving an explicit list of new column names). + names by giving an explicit list of new column names). Insert permission is + checked on the created table before filling the data (unless + WITH NO DATA is specified). 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..415c281019 100644 --- a/src/test/regress/expected/select_into.out +++ b/src/test/regress/expected/select_into.out @@ -31,6 +31,55 @@ 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.tbl_nodata1 (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.tbl_nodata1 + SELECT oid FROM pg_class WHERE relname like '%c%'; -- Error +ERROR: permission denied for table tbl_nodata1 +CREATE TABLE selinto_schema.tbl_nodata2 (a) AS + SELECT oid FROM pg_class WHERE relname like '%c%' WITH NO DATA; -- OK +PREPARE nodata_sel AS + SELECT oid FROM pg_class WHERE relname like '%c%'; +EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF) + CREATE TABLE selinto_schema.tbl_nodata3 (a) AS + EXECUTE nodata_sel WITH NO DATA; -- OK + QUERY PLAN +--------------------------------------- + Seq Scan on pg_class (never executed) + Filter: (relname ~~ '%c%'::text) +(2 rows) + +CREATE TABLE selinto_schema.tbl_nodata4 (a) AS + EXECUTE nodata_sel WITH NO DATA; -- OK +EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF) + CREATE TABLE selinto_schema.tbl_nodata5 (a) AS + EXECUTE nodata_sel WITH DATA; -- Error +ERROR: permission denied for table tbl_nodata5 +EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF) + CREATE MATERIALIZED VIEW selinto_schema.mv_nodata1 (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) + +CREATE MATERIALIZED VIEW selinto_schema.mv_nodata2 (a) AS + SELECT oid FROM pg_class WHERE relname like '%c%' WITH NO DATA; -- OK +EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF) + CREATE MATERIALIZED VIEW selinto_schema.mv_nodata3 (a) AS + SELECT oid FROM pg_class WHERE relname like '%c%' WITH DATA; -- Error +ERROR: permission denied for materialized view mv_nodata3 +CREATE MATERIALIZED VIEW selinto_schema.mv_nodata4 (a) AS + SELECT oid FROM pg_class WHERE relname like '%c%' WITH DATA; -- Error +ERROR: permission denied for materialized view mv_nodata4 RESET SESSION AUTHORIZATION; ALTER DEFAULT PRIVILEGES FOR ROLE regress_selinto_user GRANT INSERT ON TABLES TO regress_selinto_user; @@ -45,8 +94,14 @@ 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 9 other objects +DETAIL: drop cascades to table selinto_schema.tbl_nodata1 +drop cascades to table selinto_schema.tbl_nodata2 +drop cascades to table selinto_schema.tbl_nodata3 +drop cascades to table selinto_schema.tbl_nodata4 +drop cascades to materialized view selinto_schema.mv_nodata1 +drop cascades to materialized view selinto_schema.mv_nodata2 +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..8eb2fda40c 100644 --- a/src/test/regress/sql/select_into.sql +++ b/src/test/regress/sql/select_into.sql @@ -34,6 +34,33 @@ 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.tbl_nodata1 (a) AS + SELECT oid FROM pg_class WHERE relname like '%c%' WITH NO DATA; -- OK +INSERT INTO selinto_schema.tbl_nodata1 + SELECT oid FROM pg_class WHERE relname like '%c%'; -- Error +CREATE TABLE selinto_schema.tbl_nodata2 (a) AS + SELECT oid FROM pg_class WHERE relname like '%c%' WITH NO DATA; -- OK +PREPARE nodata_sel AS + SELECT oid FROM pg_class WHERE relname like '%c%'; +EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF) + CREATE TABLE selinto_schema.tbl_nodata3 (a) AS + EXECUTE nodata_sel WITH NO DATA; -- OK +CREATE TABLE selinto_schema.tbl_nodata4 (a) AS + EXECUTE nodata_sel WITH NO DATA; -- OK +EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF) + CREATE TABLE selinto_schema.tbl_nodata5 (a) AS + EXECUTE nodata_sel WITH DATA; -- Error +EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF) + CREATE MATERIALIZED VIEW selinto_schema.mv_nodata1 (a) AS + SELECT oid FROM pg_class WHERE relname like '%c%' WITH NO DATA; -- OK +CREATE MATERIALIZED VIEW selinto_schema.mv_nodata2 (a) AS + SELECT oid FROM pg_class WHERE relname like '%c%' WITH NO DATA; -- OK +EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF) + CREATE MATERIALIZED VIEW selinto_schema.mv_nodata3 (a) AS + SELECT oid FROM pg_class WHERE relname like '%c%' WITH DATA; -- Error +CREATE MATERIALIZED VIEW selinto_schema.mv_nodata4 (a) AS + SELECT oid FROM pg_class WHERE relname like '%c%' WITH DATA; -- Error RESET SESSION AUTHORIZATION; ALTER DEFAULT PRIVILEGES FOR ROLE regress_selinto_user -- 2.25.1