Re: Foreign join pushdown vs EvalPlanQual

From: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
To: robertmhaas(at)gmail(dot)com
Cc: fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp, kaigai(at)ak(dot)jp(dot)nec(dot)com, pgsql-hackers(at)postgresql(dot)org, shigeru(dot)hanada(at)gmail(dot)com
Subject: Re: Foreign join pushdown vs EvalPlanQual
Date: 2015-10-15 07:04:08
Message-ID: 20151015.160408.151716632.horiguchi.kyotaro@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hello,

I confirmed that an epqtuple of foreign parameterized scan is
correctly rejected by fdw_recheck_quals with modified outer
tuple.

I have no objection to this and have two humble comments.

In file_fdw.c, the comment for the last parameter just after the
added line seems to be better to be aligned with other comments.

In subselect.c, the added break is in the added curly-braces but
it would be better to place it after the closing brace, like the
other cases.

regards,

At Wed, 14 Oct 2015 15:21:41 -0400, Robert Haas <robertmhaas(at)gmail(dot)com> wrote in <CA+TgmoZ8REePoFv7ZjjDH-T54sQw40fnP4Mkr8hw5eizbxA4BA(at)mail(dot)gmail(dot)com>
> On Wed, Oct 14, 2015 at 4:31 AM, Etsuro Fujita
> <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> > 1) remote qual (scanrelid>0)
> > 2) remote join (scanrelid==0)
> >
> > As for #1, I noticed that there is a bug in handling the same kind of FDW
> > queries, which will be shown below. As you said, I think this should be
> > addressed by rechecking the remote quals *locally*. (I thought another fix
> > for this kind of bug before, though.) IIUC, I think this should be fixed
> > separately from #2, as this is a bug not only in 9.5, but in back branches.
> > Please find attached a patch.
>
> +1 for doing something like this. However, I don't think we can
> commit this to released branches, despite the fact that it's a bug
> fix, because breaking third-party FDWs in a minor release seems
> unfriendly. We might be able to slip it into 9.5, though, if we act
> quickly.
>
> A few review comments:
>
> - nodeForeignscan.c now needs to #include "utils/memutils.h"
> - I think it'd be safer for finalize_plan() not to try to shortcut
> processing fdw_scan_quals.
> - You forgot to update _readForeignScan.
> - The documentation needs updating.
> - I think we should use the name fdw_recheck_quals.
>
> Here's an updated patch with those changes and some improvements to
> the comments. Absent objections, I will commit it and back-patch to
> 9.5 only.

--
Kyotaro Horiguchi
NTT Open Source Software Center

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Victor Wagner 2015-10-15 07:07:40 Re: Patch: Implement failover on libpq connect level.
Previous Message Craig Ringer 2015-10-15 06:19:44 Re: Typo in replorigin_sesssion_origin (9.5+)