Re: Foreign join pushdown vs EvalPlanQual

Hello, thank you for taking time for this.

At Tue, 1 Dec 2015 14:56:54 -0500, Robert Haas <robertmhaas(at)gmail(dot)com> wrote in <CA+TgmoY+1Cq0bjXBP+coeKtkOMbpUMVQsfL2fJQY+ws7Nu=wgg(at)mail(dot)gmail(dot)com>
> On Thu, Nov 26, 2015 at 12:04 AM, Kouhei Kaigai <kaigai(at)ak(dot)jp(dot)nec(dot)com> wrote:
> > This patch is not tested by actual FDW extensions, so it is helpful
> > to enhance postgres_fdw to run the alternative sub-plan on EPQ recheck.
>
> I have done some editing and some small revisions on this patch.
> Here's what I came up with. The revisions are mostly cosmetic, but I
> revised it a bit so that the signature of GetForeignPlan need not
> change. Also, I made nodeForeignScan.c do some of the outer plan
> handling automatically, and I fixed the compile breaks in
> contrib/file_fdw and contrib/postgres_fdw.
>
> Comments/review/testing are very welcome.

Applied on HEAD with no error. Regtests of core, postgres_fdw and
file_fdw finished with no error. (I haven't done any further testing)

nodeScan.c:

The comments in nodeScan.c looks way clearer. Thank you for rewriting.

nodeForeignscan.c:

Is this a mistake?

> @@ -205,6 +218,11 @@ ExecInitForeignScan(ForeignScan *node, EState *estate, int eflags)
> scanstate->fdwroutine = fdwroutine;
> scanstate->fdw_state = NULL;
>
> + /* Initialize any outer plan. */
-> + if (outerPlanState(scanstate))
+> + if (outerPlanState(node))
> + outerPlanState(scanstate) =

createplan.c, planmain.h:

I agree with reverting the signature of GetForeignPlan.

fdwapi.h:

The reverting of the additional parameter of ForeignScan leaves
only change of indentation of the last parameter.

fdwhandler.sgml:

This is easy to understand to me. Thank you.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center