Re: Foreign join pushdown vs EvalPlanQual - Mailing list pgsql-hackers

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