Re: [Patch] Add WHERE clause support to REFRESH MATERIALIZED VIEW

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