Re: postgres_fdw join pushdown (was Re: Custom/Foreign-Join-APIs)

On Thu, Jan 21, 2016 at 2:07 AM, Robert Haas <robertmhaas(at)gmail(dot)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