Re: EXPLAIN/EXPLAIN ANALYZE REFRESH MATERIALIZED VIEW - Mailing list pgsql-hackers

From japin
Subject Re: EXPLAIN/EXPLAIN ANALYZE REFRESH MATERIALIZED VIEW
Date
Msg-id MEYP282MB16698107786A45784694D989B6AE0@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM
Whole thread Raw
In response to Re: EXPLAIN/EXPLAIN ANALYZE REFRESH MATERIALIZED VIEW  (Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>)
Responses Re: EXPLAIN/EXPLAIN ANALYZE REFRESH MATERIALIZED VIEW
List pgsql-hackers
On Fri, 08 Jan 2021 at 17:24, Bharath Rupireddy wrote:
> 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.
>

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.



pgsql-hackers by date:

Previous
From: Bruce Momjian
Date:
Subject: Re: data_checksums enabled by default (was: Move --data-checksums to common options in initdb --help)
Next
From: Ryan Lambert
Date:
Subject: Re: WIP: System Versioned Temporal Table