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

On Wed, Jan 20, 2016 at 8:50 AM, Ashutosh Bapat
<ashutosh(dot)bapat(at)enterprisedb(dot)com> wrote:
>> Third, I removed GetUserMappingById(). As mentioned in the email to
>> which I replied earlier, that doesn't actually produce the same result
>> as the way we're doing it now, and might leave the user ID invalid.
>
> The comment you quoted was my comment :). I never got a reply from
> Hanada-san on that comment. A bit of investigation revealed this: A pushed
> down foreign join which involves N foreign tables, might have different
> effective userid for each of them.
> userid = OidIsValid(rte->checkAsUser) ? rte->checkAsUser : GetUserId()
> In such a case, AFAIU, the join will be pushed down only if none of those
> have user mapping and there is public user mapping. Is that right?

Yes, I think that is right.

> In such a
> case, which userid should be stored in UserMapping structure?It might look
> like setting GetUserId() as userid in UserMapping is wise, but not really.
> All the foreign tables might have different effective userids, each
> different from GetUserId() and what GetUserId() would return might have a
> user mapping different from the effective userids. What userid should
> UserMapping structure have in that case? I thought, that's why Hanada-san
> used invalid userid there, so left as it is.

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.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company