Re: Implementing Incremental View Maintenance

From: Yugo NAGATA <nagata(at)sraoss(dot)co(dot)jp>
To: Yugo NAGATA <nagata(at)sraoss(dot)co(dot)jp>
Cc: huyajun <hu_yajun(at)qq(dot)com>, Zhihong Yu <zyu(at)yugabyte(dot)com>, Julien Rouhaud <rjuju123(at)gmail(dot)com>, "r(dot)takahashi_2(at)fujitsu(dot)com" <r(dot)takahashi_2(at)fujitsu(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Andrew Dunstan <andrew(at)dunslane(dot)net>, Andy Fan <zhihui(dot)fan1213(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Tatsuo Ishii <ishii(at)sraoss(dot)co(dot)jp>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
Subject: Re: Implementing Incremental View Maintenance
Date: 2022-03-14 10:12:17
Message-ID: 20220314191217.86df25dbd663151405122f22@sraoss.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

I attached the updated patch-set (v26).

> On Wed, 16 Feb 2022 22:34:18 +0800
> huyajun <hu_yajun(at)qq(dot)com> wrote:
>
> > Hi, Nagata-san
> > I am very interested in IMMV and read your patch but have some comments in v25-0007-Add-Incremental-View-Maintenance-support.patch and want to discuss with you.
>
> Thank you for your review!
>
> >
> > + /* For IMMV, we need to rewrite matview query */
> > + query = rewriteQueryForIMMV(query, into->colNames);
> > + query_immv = copyObject(query);
> >
> > /* Create triggers on incremental maintainable materialized view */
> > + Assert(query_immv != NULL);
> > + CreateIvmTriggersOnBaseTables(query_immv, matviewOid, true);
> > 1. Do we need copy query?Is it okay that CreateIvmTriggersOnBaseTables directly use (Query *) into->viewQuery instead of query_immv like CreateIndexOnIMMV? It seems only planner may change query, but it shouldn't affect us finding the correct base table in CreateIvmTriggersOnBaseTables .
>
> The copy to query_immv was necessary for supporting sub-queries in the view
> definition. However, we excluded the fueature from the current patch to reduce
> the patch size, so it would be unnecessary. I'll fix it.
>
> >
> > +void
> > +CreateIndexOnIMMV(Query *query, Relation matviewRel)
> > +{
> > + Query *qry = (Query *) copyObject(query);
> > 2. Also, is it okay to not copy query in CreateIndexOnIMMV? It seems we only read query in CreateIndexOnIMMV.
>
> This was also necessary for supporting CTEs, but unnecessary in the current
> patch, so I'll fix it, too.

I removed unnecessary copies of Query in according with the suggestions
from huyajun, and fix wrong codes in a "switch" statement pointed out
by Zhihong Yu.

In addition, I made the following fixes:
- Fix psql tab-completion code according with master branch
- Fix auto-index-creation that didn't work well in REFRESH command
- Add documentation description about the automatic index creation

Regards,
Yugo Nagata

--
Yugo NAGATA <nagata(at)sraoss(dot)co(dot)jp>

Attachment Content-Type Size
v26-0010-Add-documentations-about-Incremental-View-Mainte.patch text/x-diff 25.2 KB
v26-0009-Add-regression-tests-for-Incremental-View-Mainte.patch text/x-diff 46.1 KB
v26-0008-Add-aggregates-support-in-IVM.patch text/x-diff 55.2 KB
v26-0007-Add-Incremental-View-Maintenance-support.patch text/x-diff 91.0 KB
v26-0006-Add-Incremental-View-Maintenance-support-to-psql.patch text/x-diff 4.8 KB
v26-0005-Add-Incremental-View-Maintenance-support-to-pg_d.patch text/x-diff 4.1 KB
v26-0004-Allow-to-prolong-life-span-of-transition-tables-.patch text/x-diff 5.6 KB
v26-0003-Add-new-deptype-option-m-in-pg_depend-system-cat.patch text/x-diff 1.7 KB
v26-0002-Add-relisivm-column-to-pg_class-system-catalog.patch text/x-diff 4.7 KB
v26-0001-Add-a-syntax-to-create-Incrementally-Maintainabl.patch text/x-diff 6.7 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Bharath Rupireddy 2022-03-14 10:24:19 Re: add checkpoint stats of snapshot and mapping files of pg_logical dir
Previous Message Amit Kapila 2022-03-14 09:53:44 Re: Column Filtering in Logical Replication