On 2015/12/08 3:06, Tom Lane wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> I think the core system likely needs visibility into where paths and
>> plans are present in node trees, and putting them somewhere inside
>> fdw_private would be going in the opposite direction.
> Absolutely. You don't really want FDWs having to take responsibility
> for setrefs.c processing of their node trees, for example. This is why
> e.g. ForeignScan has both fdw_exprs and fdw_private.
>
> I'm not too concerned about whether we have to adjust FDW-related APIs
> as we go along. It's been clear from the beginning that we'd have to
> do that, and we are nowhere near a point where we should promise that
> we're done doing so.
OK, I'd vote for Robert's idea, then. I'd like to discuss the next
thing about his patch. As I mentioned in [1], the following change in
the patch will break the EXPLAIN output.
@@ -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))
+ outerPlanState(scanstate) =
+ ExecInitNode(outerPlan(node), estate, eflags);
+
As pointed out by Horiguchi-san, that's not correct, though; we should
initialize the outer plan if outerPlan(node) != NULL, not
outerPlanState(scanstate) != NULL. Attached is an updated version of
his patch. I'm also attaching an updated version of the postgres_fdw
join pushdown patch. You can find the breaking examples by doing the
regression tests in the postgres_fdw patch. Please apply the patches in
the following order:
epq-recheck-v6-efujita (attached)
usermapping_matching.patch in [2]
add_GetUserMappingById.patch in [2]
foreign_join_v16_efujita2.patch (attached)
As I proposed upthread, I think we could fix that by handling the outer
plan as in the patch [3]; a) the core initializes the outer plan and
stores it into somewhere in the ForeignScanState node, not the lefttree
of the ForeignScanState node, during ExecInitForeignScan, and b) when
the RecheckForeignScan routine gets called, the FDW extracts the plan
from the given ForeignScanState node and executes it. What do you think
about that?
Best regards,
Etsuro Fujita
[1] http://www.postgresql.org/message-id/565EA539.1080703@lab.ntt.co.jp
[2]
http://www.postgresql.org/message-id/CAEZqfEe9KGy=1_waGh2rgZPg0o4pqgD+iauYaj8wTze+CYJUHg@mail.gmail.com
[3] http://www.postgresql.org/message-id/5624D583.10202@lab.ntt.co.jp
