Re: Foreign join pushdown vs EvalPlanQual

On Wed, Oct 14, 2015 at 4:31 AM, Etsuro Fujita
<fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> Agreed.
>
> As KaiGai-san also pointed out before, I think we should address this in
> each of the following cases:
>
> 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.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company