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
