Re: Implementing Incremental View Maintenance - Mailing list pgsql-hackers

On Fri, 29 Nov 2019 15:34:52 +0900
Amit Langote <amitlangote09@gmail.com> wrote:

> Thanks a lot for working on this.  It's a great (and big!) feature and
> I can see that a lot of work has been put into writing this patch.  I
> started looking at the patch (v8), but as it's quite big:
> 
>  34 files changed, 5444 insertions(+), 69 deletions(-)

Thank you for your reviewing the patch! Yes, this is a big patch
athough 

> I'm having a bit of trouble reading through, which I suspect others
> may be too.  Perhaps, it can be easier for you, as authors, to know
> everything that's being changed (added, removed, existing code
> rewritten), but certainly not for a reviewer, so I think it would be a
> good idea to try to think dividing this into parts.  I still don't

I agree with you. We also think the need to split the patch and we are
considering the way.
 
> have my head wrapped around the topic of materialized view
> maintenance, but roughly it looks to me like there are really *two*
> features that are being added:
> 
> 1. Add a new method to refresh an MV incrementally; IIUC, there's
> already one method that's used by REFRESH MATERIALIZED VIEW
> CONCURRENTLY, correct?

No, REFRESH MATERIALIZED VIEW CONCURRENTLY is not the way to refresh
materialized views. This just acquires weaker locks on views to not
prevent SELECT, so this calculate the content of the view completely
from scratch. There is no method to incrementally refresh materialized
views in the current PostgreSQL.

Also, we didn't implement incremental refresh on REFRESH command in
this patch. This supports only automatically refresh using triggers.
However, we used the code for REFRESH in our IVM implementation, so
I think splitting the patch according to this point of view can make
sense.

> 2. Make the refresh automatic (using triggers on the component tables)
> 
> Maybe, there are even:
> 
> 0. Infrastructure additions

Yes, we have a bit modification on the infrastructure, for example,
trigger.c.

> As you can tell, having the patch broken down like this would allow us
> to focus on the finer aspects of each of the problem being solved and
> solution being adopted, for example:
> 
> * It would be easier for someone having an expert opinion on how to
> implement incremental refresh to have to only look at the patch for
> (1).  If the new method handles more query types than currently, which
> obviously means more code is needed, which in turn entails possibility
> of bugs, despite the best efforts.  It would be better to get more
> eyeballs at this portion of the patch and having it isolated seems
> like a good way to attract more eyeballs.
> 
> * Someone well versed in trigger infrastructure can help fine tune the
> patch for (2)
> 
> and so on.
> 
> So, please consider giving some thought to this.

Agreed. Although I am not sure we will do it as above way, we will
consider to split the patch, anyway. Thanks. 

Regards,
Yugo Nagata

-- 
Yugo Nagata <nagata@sraoss.co.jp>