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
