Re: Foreign join pushdown vs EvalPlanQual

On 2015/11/09 9:26, Kouhei Kaigai wrote:
>>> I think ForeignRecheck should first call ExecQual to test
>>> fdw_recheck_quals. If it returns false, return false. If it returns
>>> true, then give the FDW callback a chance, if one is defined. If that
>>> returns false, return false. If we haven't yet returned false,
>>> return true.

>> I think ExecQual on fdw_recheck_quals shall be called next to the
>> RecheckForeignScan callback, because econtext->ecxt_scantuple shall
>> not be reconstructed unless RecheckForeignScan callback is not called
>> if scanrelid==0.

I agree with KaiGai-san. I think we can define fdw_recheck_quals for
the foreign-join case as quals not in scan.plan.qual, the same way as
the simple foreign scan case. (In other words, the quals would be
defind as "otherclauses", ie, rinfo->is_pushed_down=true, that have been
pushed down to the remote server. For checking the fdw_recheck_quals,
however, I think we should reconstruct the join tuple first, which I
think is essential for cases where an outer join is performed remotely,
to avoid changing the semantics. BTW, in my patch [1], a secondary plan
will be created to evaluate such otherclauses after reconstructing the
join tuple.

> The attached patch is an adjusted version of the previous one.
> Even though it co-exists a new callback and fdw_recheck_quals,
> the callback is kicked first as follows.

Thanks for the patch!

> ----------------<cut here>----------------
> @@ -85,6 +86,18 @@ ForeignRecheck(ForeignScanState *node, TupleTableSlot *slot)
>
> ResetExprContext(econtext);
>
> + /*
> + * FDW driver has to recheck visibility of EPQ tuple towards
> + * the scan qualifiers once it gets pushed down.
> + * In addition, if this node represents a join sub-tree, not
> + * a scan, FDW driver is also responsible to reconstruct
> + * a joined tuple according to the primitive EPQ tuples.
> + */
> + if (fdwroutine->RecheckForeignScan)
> + {
> + if (!fdwroutine->RecheckForeignScan(node, slot))
> + return false;
> + }
> return ExecQual(node->fdw_recheck_quals, econtext, false);
> }
> ----------------<cut here>----------------
>
> If callback is invoked first, FDW driver can reconstruct a joined tuple
> with its comfortable way, then remaining checks can be done by ExecQual
> and fds_recheck_quals on the caller side.
> If callback would be located on the tail, FDW driver has no choice.

To test this change, I think we should update the postgres_fdw patch so
as to add the RecheckForeignScan.

Having said that, as I said previously, I don't see much value in adding
the callback routine, to be honest. I know KaiGai-san considers that
that would be useful for custom joins, but I don't think that that would
be useful even for foreign joins, because I think that in case of
foreign joins, the practical implementation of that routine in FDWs
would be to create a secondary plan and execute that plan by performing
ExecProcNode, as my patch does [1]. Maybe I'm missing something, though.

Best regards,
Etsuro Fujita

[1] http://www.postgresql.org/message-id/5624D583.10202@lab.ntt.co.jp