Re: Foreign join pushdown vs EvalPlanQual

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