On Fri, Dec 11, 2015 at 3:41 AM, Robert Haas <robertmh...@gmail.com> wrote:
> On Wed, Dec 2, 2015 at 6:45 AM, Ashutosh Bapat > <ashutosh.ba...@enterprisedb.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 <robertmh...@gmail.com>: > On Sat, May 16, 2015 at 9:04 AM, Shigeru Hanada > <shigeru.han...@gmail.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
