Re: Foreign join pushdown vs EvalPlanQual

From: Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>
To: Kouhei Kaigai <kaigai(at)ak(dot)jp(dot)nec(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, Shigeru Hanada <shigeru(dot)hanada(at)gmail(dot)com>
Subject: Re: Foreign join pushdown vs EvalPlanQual
Date: 2015-10-19 11:35:31
Message-ID: 5624D583.10202@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

Attachment Content-Type Size
foreign-recheck-for-foreign-join-v2.patch text/x-patch 11.5 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Etsuro Fujita 2015-10-19 11:51:50 Re: Foreign join pushdown vs EvalPlanQual
Previous Message Andres Freund 2015-10-19 10:10:44 Checkpoint throttling issues