On Thu, Jan 21, 2016 at 2:07 AM, Robert Haas <robertmh...@gmail.com> wrote:

> Well, we kind of need to get it right, not just be guessing.
> It looks to me as though GetConnection() only uses the user ID as a
> cache lookup key.  What it's trying to do is ensure that if user A and
> user B need different user mapping options, we don't use the same
> connection for both.  So, actually, passing InvalidOid seems like it's
> not really a problem here.  It's arguably more correct than what we've
> been doing up until now, since it means we cache the connection under
> user OID whose options we used, rather than the user OID that asked
> about the options.
In that case, do we need GetUserMappingById changes in that patch and not
pull it out. If we are keeping those changes, I need some clarification
about your comment

Even if that were no issue, it doesn't seem to add anything.  The only
> caller of the new function is  postgresBeginForeignScan(), and that
> function already has a way of getting the user mapping.  The new way
> doesn't seem to be better or faster, so why bother changing it?

In pg_fdw_join_v2.patch, postgresBeginForeignScan() obtained user mapping
using GetUserMappingById() instead of the earlier way of fetching it by
userid and serverid. So even that change will remain, right?

Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

Reply via email to