Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> writes:
> This is the revased and revised version of the previous patch.
A few more comments:
* Per the spec for CacheRegisterSyscacheCallback, a zero hash value means
to flush all associated state. This isn't handling that case correctly.
Even when you are given a specific hash value, I think exiting after
finding one match is incorrect: there could be multiple connections
to the same server with different user mappings, or vice versa.
* I'm not really sure that it's worth the trouble to pay attention
to the hash value at all. Very few other syscache callbacks do,
and the pg_server/pg_user_mapping catalogs don't seem likely to
have higher than average traffic.
* Personally I'd be inclined to register the callbacks at the same
time the hashtables are created, which is a pattern we use elsewhere
... in fact, postgres_fdw itself does it that way for the transaction
related callbacks, so it seems a bit weird that you are going in a
different direction for these callbacks. That way avoids the need to
depend on a _PG_init function and means that the callbacks don't have to
worry about the hashtables not being valid. Also, it seems a bit
pointless to have separate layers of postgresMappingSysCallback and
InvalidateConnectionForMapping functions.
* How about some regression test cases? You couldn't really exercise
cross-session invalidation easily, but I don't think you need to.
regards, tom lane