Re: Foreign join pushdown vs EvalPlanQual

On 2015/10/16 19:03, Kouhei Kaigai wrote:
> *** 48,59 **** ExecScanFetch(ScanState *node,
> +         /*
> +          * Execute recheck plan and get the next tuple if foreign join.
> +          */
> +         if (scanrelid == 0)
> +         {
> +             (*recheckMtd) (node, slot);
> +             return slot;
> +         }
>
> Ensure the slot is empty if recheckMtd returned false, as base relation
> case doing so.

Fixed.

> *** 347,352 **** ExecScanReScan(ScanState *node)
>        {
>            Index        scanrelid = ((Scan *) node->ps.plan)->scanrelid;
>
> +         if (scanrelid == 0)
> +             return;                /* nothing to do */
> +
>            Assert(scanrelid > 0);
>
>            estate->es_epqScanDone[scanrelid - 1] = false;
>
> Why nothing to do?
> Base relations managed by ForeignScan are tracked in fs_relids bitmap.

I think the estate->es_epqScanDone flag should be initialized when we do
ExecScanReSacn for each of the component ForeignScanState nodes in the
local join execution plan state tree.

> As you introduced a few days before, if ForeignScan has parametalized
> remote join, EPQ slot contains invalid tuples based on old outer tuple.

Maybe my explanation was not enough, but I haven't said such a thing.
The problem in that case is that just returning the previously-returned
foeign-join tuple would produce an incorrect result if an outer tuple to
be joined has changed due to a concurrent transaction, as explained
upthread.  (I think that the EPQ slots would contain valid tuples.)

Attached is an updated version of the patch.

Other changes:
* remove unnecessary memory-context handling for the foreign-join case
in ForeignRecheck
* revise code a bit and add a bit more comments

Thanks for the comments!

Best regards,
Etsuro Fujita