From 31a42dd6ca2fe23cdfe46c5e0cdc21ebd9d0639a Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy Date: Fri, 13 Nov 2020 12:43:05 +0530 Subject: [PATCH v4] 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 | 7 ++ doc/src/sgml/ref/create_table_as.sgml | 7 ++ src/backend/commands/createas.c | 87 +++++++++++-------- src/test/regress/expected/matview.out | 33 +++++++ src/test/regress/expected/select_into.out | 39 ++++++++- src/test/regress/sql/matview.sql | 26 ++++++ src/test/regress/sql/select_into.sql | 17 ++++ 7 files changed, 180 insertions(+), 36 deletions(-) diff --git a/doc/src/sgml/ref/create_materialized_view.sgml b/doc/src/sgml/ref/create_materialized_view.sgml index 5ba851b687..e4ea049eff 100644 --- a/doc/src/sgml/ref/create_materialized_view.sgml +++ b/doc/src/sgml/ref/create_materialized_view.sgml @@ -48,6 +48,13 @@ CREATE MATERIALIZED VIEW [ IF NOT EXISTS ] table_name A materialized view has many of the same properties as a table, but there is no support for temporary materialized views. + + + CREATE MATERIALIZED VIEW requires + CREATE privilege on the schema used for the materialized + view. If using WITH DATA, the default, + INSERT privilege is also required. + diff --git a/doc/src/sgml/ref/create_table_as.sgml b/doc/src/sgml/ref/create_table_as.sgml index bcbd73b227..2cac4e3ec0 100644 --- a/doc/src/sgml/ref/create_table_as.sgml +++ b/doc/src/sgml/ref/create_table_as.sgml @@ -53,6 +53,13 @@ CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } | UNLOGGED ] TABLE [ IF NOT EXI defining SELECT statement whenever it is queried. + + + CREATE TABLE AS requires CREATE + privilege on the schema used for the table. If using + WITH DATA, the default, INSERT + privilege is also required. + diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c index d53ec952d0..13bfd7711d 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 privilege 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 privilege 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/matview.out b/src/test/regress/expected/matview.out index d0121a7b0b..f0ecff8a10 100644 --- a/src/test/regress/expected/matview.out +++ b/src/test/regress/expected/matview.out @@ -589,3 +589,36 @@ SELECT * FROM mvtest2; ERROR: materialized view "mvtest2" has not been populated HINT: Use the REFRESH MATERIALIZED VIEW command. ROLLBACK; +-- Verfiy INSERT privilege, if owner is not allowed to insert. +CREATE SCHEMA matview_schema; +CREATE USER matview_user; +ALTER DEFAULT PRIVILEGES FOR ROLE matview_user + REVOKE INSERT ON TABLES FROM matview_user; +GRANT ALL ON SCHEMA matview_schema TO public; +SET SESSION AUTHORIZATION matview_user; +EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF) + CREATE MATERIALIZED VIEW matview_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 matview_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 matview_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 matview_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 matview_user + GRANT INSERT ON TABLES TO matview_user; +DROP SCHEMA matview_schema CASCADE; +NOTICE: drop cascades to 2 other objects +DETAIL: drop cascades to materialized view matview_schema.mv_nodata1 +drop cascades to materialized view matview_schema.mv_nodata2 +DROP USER matview_user; diff --git a/src/test/regress/expected/select_into.out b/src/test/regress/expected/select_into.out index f373fae679..7d85386891 100644 --- a/src/test/regress/expected/select_into.out +++ b/src/test/regress/expected/select_into.out @@ -31,6 +31,37 @@ 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 RESET SESSION AUTHORIZATION; ALTER DEFAULT PRIVILEGES FOR ROLE regress_selinto_user GRANT INSERT ON TABLES TO regress_selinto_user; @@ -45,8 +76,12 @@ 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 7 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 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/matview.sql b/src/test/regress/sql/matview.sql index d96175aa26..bed09967a1 100644 --- a/src/test/regress/sql/matview.sql +++ b/src/test/regress/sql/matview.sql @@ -236,3 +236,29 @@ SELECT mvtest_func(); SELECT * FROM mvtest1; SELECT * FROM mvtest2; ROLLBACK; + +-- Verfiy INSERT privilege, if owner is not allowed to insert. +CREATE SCHEMA matview_schema; +CREATE USER matview_user; +ALTER DEFAULT PRIVILEGES FOR ROLE matview_user + REVOKE INSERT ON TABLES FROM matview_user; +GRANT ALL ON SCHEMA matview_schema TO public; + +SET SESSION AUTHORIZATION matview_user; +EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF) + CREATE MATERIALIZED VIEW matview_schema.mv_nodata1 (a) AS + SELECT oid FROM pg_class WHERE relname like '%c%' WITH NO DATA; -- OK +CREATE MATERIALIZED VIEW matview_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 matview_schema.mv_nodata3 (a) AS + SELECT oid FROM pg_class WHERE relname like '%c%' WITH DATA; -- Error +CREATE MATERIALIZED VIEW matview_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 matview_user + GRANT INSERT ON TABLES TO matview_user; + +DROP SCHEMA matview_schema CASCADE; +DROP USER matview_user; diff --git a/src/test/regress/sql/select_into.sql b/src/test/regress/sql/select_into.sql index a708fef0ea..05a620b12d 100644 --- a/src/test/regress/sql/select_into.sql +++ b/src/test/regress/sql/select_into.sql @@ -34,6 +34,23 @@ 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 RESET SESSION AUTHORIZATION; ALTER DEFAULT PRIVILEGES FOR ROLE regress_selinto_user -- 2.25.1