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@gmail.com> wrote in
<CA+TgmoZ8REePoFv7ZjjDH-T54sQw40fnP4Mkr8hw5eizbxA4BA@mail.gmail.com>
> On Wed, Oct 14, 2015 at 4:31 AM, Etsuro Fujita
> <fujita.etsuro@lab.ntt.co.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
