Re: EXPLAIN/EXPLAIN ANALYZE REFRESH MATERIALIZED VIEW

On Fri, Jan 8, 2021 at 1:50 PM japin <japinli@hotmail.com> wrote:
> Thanks for updating the patch!
>
> +       /* Get the data generating query. */
> +       dataQuery = get_matview_query(stmt, &matviewRel, &matviewOid);
>
> -       /*
> -        * Check for active uses of the relation in the current transaction, such
> -        * as open scans.
> -        *
> -        * NB: We count on this to protect us against problems with refreshing the
> -        * data using TABLE_INSERT_FROZEN.
> -        */
> -       CheckTableNotInUse(matviewRel, "REFRESH MATERIALIZED VIEW");
> +       relowner = matviewRel->rd_rel->relowner;
>
> After apply the patch, there is a duplicate
>
> relowner = matviewRel->rd_rel->relowner;

Corrected that.

> +       else if(matviewInfo)
> +               dest = CreateTransientRelDestReceiver(matviewInfo->OIDNewHeap);
>
> If the `matviewInfo->OIDNewHeap` is invalid, IMO we don't need create
> DestReceiver, isn't it?  And we should add a space after `if`.

Yes, we can skip creating the dest receiver when OIDNewHeap is
invalid, this can happen for plain explain refresh mat view case.

    if (explainInfo && !explainInfo->es->analyze)
        OIDNewHeap = InvalidOid;
    else
        OIDNewHeap = get_new_heap_oid(stmt, matviewRel, matviewOid,
                                      &relpersistence);

Since we don't call ExecutorRun for plain explain, we can skip the
dest receiver creation. I modified the code as below in explain.c.

    if (into)
        dest = CreateIntoRelDestReceiver(into);
    else if (matviewInfo && OidIsValid(matviewInfo->OIDNewHeap))
        dest = CreateTransientRelDestReceiver(matviewInfo->OIDNewHeap);
    else
        dest = None_Receiver;

Thanks for taking a look at the patches.

Attaching v3 patches, please consider these for further review.

With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com