Hi,
I attached the updated patch-set (v26).
> On Wed, 16 Feb 2022 22:34:18 +0800
> huyajun <hu_yajun@qq.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.patchand 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
insteadof query_immv like CreateIndexOnIMMV? It seems only planner may change query, but it shouldn't affect us
findingthe 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@sraoss.co.jp>
