Re: PgFDW connection invalidation by ALTER SERVER/ALTER USER MAPPING

Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com> writes:
> On Fri, Jul 21, 2017 at 10:55 AM, Kyotaro HORIGUCHI
> <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>> The attached patch differs only in this point.

> +1. The patch looks good to me.

Pushed with a couple additional changes: we'd all missed that the header
comment for GetConnection was obsoleted by this change, and the arguments
for GetSysCacheHashValue really need to be coerced to Datum. (I think
OID to Datum is the same as what the compiler would do anyway, but best
not to assume that.)

Back-patching was more exciting than I could wish. It seems that
before 9.6, we did not have struct UserMapping storing the OID of the
pg_user_mapping row it had been made from. I changed GetConnection to
re-look-up that row and get the OID. But that's ugly, and there's a
race condition: if user mappings are being added or deleted meanwhile,
we might locate a per-user mapping when we're really using a PUBLIC
mapping or vice versa, causing the ConnCacheEntry to be labeled with
the wrong hash value so that it might not get invalidated properly later.
Still, it's significantly better than it was, and that corner case seems
unlikely to get hit in practice --- for one thing, you'd have to then
revert the mapping addition/deletion before the ConnCacheEntry would be
found and used again. I don't want to take the risk of modifying struct
UserMapping in stable branches, so it's hard to see a way to make that
completely bulletproof before 9.6.

regards, tom lane