Re: Implementing Incremental View Maintenance

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.

Regards,
Yugo Nagata

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