Re: EXPLAIN/EXPLAIN ANALYZE REFRESH MATERIALIZED VIEW

On Fri, 08 Jan 2021 at 17:24, Bharath Rupireddy wrote:
> 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.
>

Thanks!

> Attaching v3 patches, please consider these for further review.
>

I find that both the declaration and definition of match_matview_with_new_data()
have a tab between type and variable. We can use pgindent to fix it.
What do you think?

static void
match_matview_with_new_data(RefreshMatViewStmt *stmt, Relation matviewRel,
^
Oid matviewOid, Oid OIDNewHeap, Oid relowner,
int save_sec_context, char relpersistence,
uint64 processed)
^

--
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.