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

On Fri, Dec 11, 2015 at 3:41 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

> On Wed, Dec 2, 2015 at 6:45 AM, Ashutosh Bapat
> <ashutosh(dot)bapat(at)enterprisedb(dot)com> wrote:
> > It's been a long time since last patch on this thread was posted. I have
> > started
> > to work on supporting join pushdown for postgres_fdw. Attached please
> find
> > three
> > patches
> > 1. pg_fdw_core.patch: changes in core related to user mapping handling,
> GUC
> > enable_foreignjoin
> > 2. pg_fdw_join.patch: postgres_fdw changes for supporting join pushdown
>
> It seems useful to break things up this way. However, I'm not sure we
> want an enable_foreignjoin GUC; in fact, I think we probably don't.
> If we want to have a way to control this, postgres_fdw can provide a
> custom GUC or FDW option for that.
>

enable_foreignjoin or its FDW counterpart would be useful for debugging
purposes just like enable_hashjoin/enable_mergejoin etc. Foreign join push
down can be viewed as a join strategy just like merge/nest loop/hash join
etc. Having enable_foreignjoin complements that picture. Users find more
usage of the same. A user running multiple FDWs and needing to disable join
pushdown across FDWs for any purpose would find enable_foreignjoin very
useful. Needing to turn on/off multiple GUCs would be cumbersome.

>
> And to be honest, I haven't really been able to understand why join
> pushdown needs changes to user mapping handling.

Current join pushdown infrastructure in core allows join to be pushed down
if both the sides of join come from the same server. Those sides may have
different user mappings and thus different user properties/access
permissions/visibility on the foreign server. If FDW chooses either of
these different user mappings to push down the join, it will get wrong
results. So, a join between two foreign relations can not be pushed down if
the user mappings on both sides do not match. This has been already
discussed in this thread. I am pasting your response to Hanada-san back in
May 2015 as hint to the discussion
--
2015-05-21 23:11 GMT+09:00 Robert Haas <robertmhaas(at)gmail(dot)com>:
> On Sat, May 16, 2015 at 9:04 AM, Shigeru Hanada
> <shigeru(dot)hanada(at)gmail(dot)com> wrote:
>> d) All relations must have the same effective user id
>> This check is done with userid stored in PgFdwRelationInfo, which is
>> valid only when underlying relations have the same effective user id.
>> Here "effective user id" means id of the user executing the query, or
>> the owner of the view when the foreign table is accessed via view.
>> Tom suggested that it is necessary to check that user mapping matches
>> for security reason, and now I think it's better than checking
>> effective user as current postgres_fdw does.
>
> So, should this be a separate patch?
--
To add to that, checking user mapping is better than checking the effective
user id for multiple reasons. Multiple local users can share same public
user mapping, which implies that they share same permissions/visibility for
objects on the foreign server. Join involving two sides with different
effective local user but same user mapping can be pushed down to the
foreign server as same objects/data is going to visible where or not we
push the join down.

> Just hypothetically
> speaking, if I put my foot down and said we're not committing any of
> that stuff, how and why would that impact our ability to have join
>
pushdown in 9.6?
>
>
In fact, the question would be what user mapping should be used by the
connection on which we are firing join query. Unless we answer that
question we won't have join pushdown in 9.6. If we push down joins without
taking into consideration the user mapping, we will have all sorts of
security/data visibility problems because of wrong user properties used for
connection.

--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company