Re: [HACKERS] PgFDW connection invalidation by ALTER SERVER/ALTER USER MAPPING

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