I've been working on this again for the past week. I think I now finally understand the problem I am trying to solve with this feature, and the concurrency implications. I cannot see a way to solve this without writing some custom locking code, unless I go back to using advisory locks, and the limitations (max_locks_per_transaction) and poor performance that imposes. I really thought I could get away with implementing this feature without getting too low level, but that is absolutely not the case now that I've hit the issues that I have when testing. It also humbles me that every prior trigger/table based approach to this I've seen or implemented in the past absolutely has concurrency bugs I wasn't aware of at the time.
I thought I was getting close and had fixed all reported correctness issues, but during another review pass I found multiple other concurrency situations that the locking system I used in the v2 patch simply couldn't solve. The last problem I couldn't get around is that row locks only protect rows that already exist in the materialized view, so there was nothing to lock for keys that are just about to be inserted. This lets a concurrent refresh with an older snapshot apply stale data after a newer one had already committed. I said earlier in this thread that the outcome there was still correct because the last writer wins on that key, and that was wrong. If I solve for that while keeping the existing locking code, I kill performance by duplicating locking in the vast majority of cases that don't need it (not under concurrency on the same key). I could avoid the custom locking by serializing all partial refreshes of a materialized view, like Dharin suggested, but for this feature to actually be useful the locking to allow concurrency needs to be there. I am splitting it into two patches so it is easier to review: the first adds the WHERE clause with all partial refreshes of a materialized view serialized (readers are still not blocked), and the second adds the custom locking on top so refreshes of different keys can run at the same time. The first is correct on its own, but I don't see it as useful without the second. I am still working on the v3 patchset, and it has changed since my last message, so the locking I described there is not what will be in the patch. Adam
