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
