Kyotaro HORIGUCHI <horiguchi.kyot...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers