On Thu, Nov 12, 2020 at 04:17:43PM +0530, Bharath Rupireddy wrote:
> On HEAD/master, the behaviour is as follows: a) for plain CTAS WITH NO
> DATA, ExecCheckRTPerms() will not be called. b) for explain analyze
> CTAS WITH NO DATA, ExecCheckRTPerms() will be called. So, we need a).
> This is what exactly this patch does i.e. ExecCheckRTPerms() will not
> be called for both cases.
> 
> Attaching V3 patch, please review it.

+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
Let's move any tests related to matviews to matviews.sql.  It does not
seem consistent to me to have those tests in a test path reserved to
CTAS, though I agree that there is some overlap and that setting up
the permissions requires a bit of duplication.

+   refreshed later using <command>REFRESH MATERIALIZED VIEW</command>. Insert
+   permission is checked on the materialized view before populating the data
+   (unless <command>WITH NO DATA</command> is specified).
Let's document that in a new paragraph, using "privilege" instead of
"permission", say (comment applies as well to the CTAS page):
CREATE MATERIALIZED VIEW requires CREATE privileges on the schema used
for the materialized view.  If using WITH DATA, the default, INSERT
privileges are also required.

+    * 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.
I would also use "privilege" here.  A nit.

    myState->reladdr = intoRelationAddr;
-   myState->output_cid = GetCurrentCommandId(true);
    myState->ti_options = TABLE_INSERT_SKIP_FSM;
-   myState->bistate = GetBulkInsertState();
+   myState->output_cid = GetCurrentCommandId(true);
The changes related to the bulk-insert state data look fine per se.
One nit: I would set bistate to NULL for the data-skip case here.
--
Michael

Attachment: signature.asc
Description: PGP signature

Reply via email to