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.10...@lab.ntt.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers