Re: postgres_fdw bug in 9.6

Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>

From: Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>
To: Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
Cc: Jeff Janes <jeff.janes@gmail.com>, Robert Haas <robertmhaas@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2017-01-20T03:19:10Z
Lists: pgsql-hackers
On 2017/01/18 20:34, Ashutosh Bapat wrote:
> On Wed, Jan 18, 2017 at 8:18 AM, Etsuro Fujita
> <fujita.etsuro@lab.ntt.co.jp> wrote:
>> Done.  Attached is the new version.  I also adjusted the code a bit further.

> With this patch there are multiple cases, where CreateLocalJoinPath()
> would return NULL and hence postgres_fdw would not push a join down
> for example
>                 /*
>                  * (1) if either cheapest-total path is parameterized by the
>                  * other rel, we can't generate a hashjoin/mergejoin path, and
>                  * (2) proposed hashjoin/mergejoin path is still parameterized
>                  * (ie, the required_outer set calculated by
>                  * calc_non_nestloop_required_outer isn't NULL), it's not what
>                  * we want; which means that both the cheapest-total paths
>                  * should be unparameterized.
>                  */
>                 if (outer_path->param_info || inner_path->param_info)
>                     return NULL;
> or
>                 /*
>                  * If the cheapest-total outer path is parameterized by the
>                  * inner rel, we can't generate a nestloop path.  (There's no
>                  * use looking for alternative outer paths, since it should
>                  * already be the least-parameterized available path.)
>                  */
>                 if (PATH_PARAM_BY_REL(outer_path, innerrel))
>                     return NULL;
>                 /* If proposed path is still parameterized, don't return it. */
>                 required_outer = calc_nestloop_required_outer(outer_path,
>                                                               inner_path);
>                 if (required_outer)
>                 {
>                     bms_free(required_outer);
>                     return NULL;
>                 }
>
> Am I right?
>
> The earlier version of this function GetExistingLocalJoinPath() used
> to return a local path in those cases and hence postgres_fdw was able
> to push down corresponding queries. So, we are introducing a
> performance regression.

Really?  My understanding is: postgres_fdw will never have those cases 
because it can always get the cheapest-total paths that are unparameterized.

Best regards,
Etsuro Fujita




Commits

  1. Fix test case for 'outer pathkeys do not match mergeclauses' fix.

  2. postgres_fdw: Avoid 'outer pathkeys do not match mergeclauses' error.

  3. postgres_fdw: Consider foreign joining and foreign sorting together.

  4. Allow foreign and custom joins to handle EvalPlanQual rechecks.

  5. Allow FDWs to push down quals without breaking EvalPlanQual rechecks.