Thanks for the comments.
On Fri, Nov 13, 2020 at 9:19 AM Michael Paquier <michael@paquier.xyz> wrote:
>
> 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.
>
Done.
>
> "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.
>
Done.
>
> + * 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.
>
Done.
> 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.
>
It's not required to set bistate to null as we have allocated myState
with palloc0 in CreateIntoRelDestReceiver, it will anyways be null.
if (!into->skipData)
myState->bistate = GetBulkInsertState();
Attaching v4 patch. Please review it.
With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com
