Hi! Thanks for reply!
On Mon, Dec 24, 2018 at 2:20 PM David Fetter <da...@fetter.org> wrote: > You've got the right mailing list, a description of what you want, and > a PoC patch. You also got the patch in during the time between > Commitfests. You're doing great! Great! One thing I am unclear about is how it is determined if this is a viable feature to be eventually included. You gave me some suggestions to improve in my patch (adding tests and so on). Does this mean that the patch should be fully done before a decision is made? Also, the workflow is that I improve things, and resubmit a patch to the mailing list, for now? > > - Currently only insert and remove operations are done on the > > materialized view. This is because the current logic just removes > > changed rows and inserts new rows. > > What other operations might you want to support? Update. So if a row is changing, instead of doing a remove and insert, what currently is being done, I would prefer an update. Then UPDATE trigger operation would happen as well. Maybe the INSERT query could be changed to INSERT ... ON CONFLICT UPDATE query (not sure if this one does UPDATE trigger operation on conflict), and REMOVE changed to remove just rows which were really removed, but not only updated. > As far as you can tell, is this just an efficiency optimization, or > might it go to correctness of the behavior? It is just an optimization. Or maybe even just a surprise. Maybe a documentation addition could help here. In my use case I would loop over OLD and NEW REFERENCING TABLE so if they are empty, nothing would be done. But it is just surprising that DELETE trigger is called even when no rows are being deleted in the materialized view. > I'm not sure I understand the problem being described here. Do you see > these as useful to separate for some reason? So rows which are just updated currently get first DELETE trigger called and then INSERT. The issue is that if I am observing this behavior from outside, it makes it unclear when I see DELETE if this means really that a row has been deleted or it just means that later on an INSERT would happen. Now I have to wait for an eventual INSERT to determine that. But how long should I wait? It makes consuming these notifications tricky. If I just blindly respond to those notifications, this could introduce other problems. For example, if I have a reactive web application it could mean a visible flicker to the user. Instead of updating rendered row, I would first delete it and then later on re-insert it. > > Non-concurrent refresh does not trigger any trigger. But it seems > > all data to do so is there (previous table, new table), at least for > > the statement-level trigger. Row-level triggers could also be > > simulated probably (with TRUNCATE and INSERT triggers). > > Would it make more sense to fill in the missing implementations of NEW > and OLD for per-row triggers instead of adding another hack? You lost me here. But I agree, we should implement this fully, without hacks. I just do not know how exactly. Are you saying that we should support only row-level triggers, or that we should support both statement-level and row-level triggers, but just make sure we implement this properly? I think that my suggestion of using TRUNCATE and INSERT triggers is reasonable in the case of full refresh. This is what happens. If we would want to have DELETE/UPDATE/INSERT triggers, we would have to compute the diff like concurrent version has to do, which would defeat the difference between the two. But yes, all INSERT trigger calls should have NEW provided. So per-statement trigger would have TRUNCATE and INSERT called. And per-row trigger would have TRUNCATE and per-row INSERTs called. Mitar -- http://mitar.tnode.com/ https://twitter.com/mitar_m