Re: Foreign join pushdown vs EvalPlanQual

> On Thu, Nov 26, 2015 at 12:04 AM, Kouhei Kaigai <kaigai(at)ak(dot)jp(dot)nec(dot)com> wrote:
> > The attached patch adds: Path *fdw_outerpath field to ForeignPath node.
> > FDW driver can set arbitrary but one path-node here.
> > After that, this path-node shall be transformed to plan-node by
> > createplan.c, then passed to FDW driver using GetForeignPlan callback.
> > We expect FDW driver set this plan-node on lefttree (a.k.a outerPlan).
> > The Plan->outerPlan is a common field, so patch size become relatively
> > small. FDW driver can initialize this plan at BeginForeignScan, then
> > execute this sub-plan-tree on demand.
> >
> > Remaining portions are as previous version. ExecScanFetch is revised
> > to call recheckMtd always when scanrelid==0, then FDW driver can get
> > control using RecheckForeignScan callback.
> > It allows FDW driver to handle (1) EPQ recheck on underlying scan nodes,
> > (2) reconstruction of joined tuple, and (3) EPQ recheck on join clauses,
> > by its preferable implementation - including execution of an alternative
> > sub-plan.
> >
> >> There seems to be no changes to make_foreignscan. Is that OK?
> >>
> > create_foreignscan_path(), not only make_foreignscan().
> >
> > 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.
>
Thanks for the revising. (I could not be online for a few days, sorry.)

> Also, I made nodeForeignScan.c do some of the outer plan
> handling automatically,
>
It's OK for me. We may omit initialization/shutdown of sub-plan when
it is not actually needed, even if FDW driver set up. However, it is
very tiny advantage.

> and I fixed the compile breaks in
> contrib/file_fdw and contrib/postgres_fdw.
>
Sorry, I didn't fix up contrib side.

> Comments/review/testing are very welcome.
>
One small point:

@@ -3755,7 +3762,6 @@ make_foreignscan(List *qptlist,
/* cost will be filled in by create_foreignscan_plan */
plan->targetlist = qptlist;
plan->qual = qpqual;
- plan->lefttree = NULL;
plan->righttree = NULL;
node->scan.scanrelid = scanrelid;
/* fs_server will be filled in by create_foreignscan_plan */

Although it is harmless, I prefer this line is kept because caller
of make_foreignscan() expects a ForeignScan node with empty lefttree,
even if it is filled up later.

Best regards,
--
NEC Business Creation Division / PG-Strom Project
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>