Re: Re: [COMMITTERS] pgsql: Only try to push down foreign joins if the user mapping OIDs mat

On Wed, Mar 16, 2016 at 10:22 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> > On Wed, Mar 16, 2016 at 4:10 AM, Ashutosh Bapat <
> > ashutosh(dot)bapat(at)enterprisedb(dot)com> wrote:
> >> In 9.5, postgres_fdw allowed to prepare statements involving foreign
> >> tables without an associated user mapping as long as planning did not
> >> require the user mapping. Remember, planner would require user mapping
> in
> >> case use_remote_estimate is ON for that foreign table. The user mapping
> >> would be certainly required at the time of execution. So executing such
> a
> >> prepared statement would throw error. If somebody created a user mapping
> >> between prepare and execute, execute would not throw an error. A join
> can
> >> be pushed down only when user mappings associated with the joining
> >> relations are known and they match. But given the behavior of 9.5 we
> should
> >> let the prepare succeed, even if the join can not be pushed down
> because of
> >> unknown user mapping. Before this fix, the test was not letting the
> prepare
> >> succeed, which is not compliant with 9.5.
>
> > If a query against a single table with no user mapping is legal, I don't
> > see why pushing down a join between two tables neither of which has a
> user
> > mapping shouldn't also be legal.
>
> The key point here is that Ashutosh is arguing on the basis of the
> behavior of postgres_fdw, which is not representative of all FDWs.
> The core code has no business assuming that all FDWs require user
> mappings; file_fdw is a counterexample.
>
>
Here's what the patch is doing.

The "core" code FDW is given chance to add paths for a join between foreign
relations if they are from the same (valid) server and have same user
mappings, even if the user mapping is invalid. This is code in
build_join_rel(). So, from core code's perspective it's perfectly fine to
push a join down when both sides do not have valid user mapping associated
with them. So, file_fdw is capable of implementing join pushdown even
without having user mapping.

postgres_fdw code however is different. Rest of the discussion only
pertains to postgres_fdw. The comment in postgres_fdw.sql testcase, to
which Robert objected, is relevant only for postgres_fdw.

postgres_fdw requires user mapping to execute the query. For base foreign
tables, it's fine not to have a user mapping at the time of planning as
long as planner doesn't need to query the foreign server. But it certainly
requires a user mapping at the time of execution, or else it throws error
at the time of execution. So, Robert's statement "If a query against a
single table with no user mapping is legal" is not entirely correct in the
context of postgres_fdw; postgresGetForeignRelSize(), which is called
during planning, can throw error if it doesn't find a valid user mapping.
If a join between two postgres_fdw foreign relations without valid user
mappings is decided to be pushed down at the time of planning, it will
require a user mapping at the time of execution. This user mapping is
derived from the joining relations recursively. If all of them have same
user mapping (even if invalid) it's fine. If it's invalid, we will throw
error at the time of execution. But if they acquire different user
mappings, postgres_fdw can not fire the join query. It can not use any of
the user mappings since that will compromise security. So, we have landed
with a plan which can not be executed. To be on the safer side,
postgres_fdw does not push a join down if joining sides do not have a valid
user mapping. A base foreign relation with postgres_fdw will require a user
mapping, for all serious uses. So, it's not likely that we will end up with
joins between postgres_fdw foreign relations with invalid user mapping.

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