Re: EXPLAIN/EXPLAIN ANALYZE REFRESH MATERIALIZED VIEW

On Fri, Jan 8, 2021 at 1:50 PM japin <japinli(at)hotmail(dot)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