On 2015/09/11 6:02, Robert Haas wrote:
> On Thu, Sep 3, 2015 at 6:25 AM, Etsuro Fujita
> <fujita.etsuro@lab.ntt.co.jp> wrote:
>> I gave it another thought; the following changes to ExecInitNode would make
>> the patch much simpler, ie, we would no longer need to call the new code in
>> ExecInitForeignScan, ExecForeignScan, ExecEndForeignScan, and
>> ExecReScanForeignScan. I think that would resolve the name problem also.
>>
>> *** a/src/backend/executor/execProcnode.c
>> --- b/src/backend/executor/execProcnode.c
>> ***************
>> *** 247,254 **** ExecInitNode(Plan *node, EState *estate, int eflags)
>> break;
>>
>> case T_ForeignScan:
>> ! result = (PlanState *) ExecInitForeignScan((ForeignScan *) node,
>> ! estate, eflags);
>> break;
>>
>> case T_CustomScan:
>> --- 247,269 ----
>> break;
>>
>> case T_ForeignScan:
>> ! {
>> ! Index scanrelid = ((ForeignScan *)
>> node)->scan.scanrelid;
>> !
>> ! if (estate->es_epqTuple != NULL && scanrelid == 0)
>> ! {
>> ! /*
>> ! * We are in foreign join inside an EvalPlanQual
>> recheck.
>> ! * Initialize local join execution plan, instead.
>> ! */
>> ! Plan *subplan = ((ForeignScan *)
>> node)->fs_subplan;
>> !
>> ! result = ExecInitNode(subplan, estate, eflags);
>> ! }
>> ! else
>> ! result = (PlanState *) ExecInitForeignScan((ForeignScan
>> *) node,
>> ! estate,
>> eflags);
>> ! }
>> break;
>
> I don't think that's a good idea. The Plan tree and the PlanState
> tree should be mirror images of each other; breaking that equivalence
> will cause confusion, at least.
IIRC, Horiguchi-san also pointed that out. Honestly, I also think that
that is weird, but IIUC, I think it can't hurt. What I was concerned
about was EXPLAIN, but EXPLAIN doesn't handle an EvalPlanQual PlanState
tree at least currently.
Best regards,
Etsuro Fujita