Re: Foreign join pushdown vs EvalPlanQual

I briefly browsed the patch apart from my preference towards the approach.

It has at least two oversight.

*** 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.

*** 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.
As you introduced a few days before, if ForeignScan has parametalized
remote join, EPQ slot contains invalid tuples based on old outer tuple.

Thanks,
--
NEC Business Creation Division / PG-Strom Project
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

> -----Original Message-----
> From: Etsuro Fujita [mailto:fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp]
> Sent: Friday, October 16, 2015 6:01 PM
> To: Robert Haas
> Cc: Kyotaro HORIGUCHI; Kaigai Kouhei(海外 浩平); pgsql-hackers(at)postgresql(dot)org;
> Shigeru Hanada
> Subject: Re: [HACKERS] Foreign join pushdown vs EvalPlanQual
>
> On 2015/10/14 17:31, Etsuro Fujita wrote:
> > As KaiGai-san also pointed out before, I think we should address this in
> > each of the following cases:
> >
> > 1) remote qual (scanrelid>0)
> > 2) remote join (scanrelid==0)
>
> As for #2, I updated the patch, which uses a local join execution plan
> for an EvalPlanQual rechech, according to the comment from Robert [1].
> Attached is an updated version of the patch. This is a WIP patch, but
> it would be appreciated if I could get feedback earlier.
>
> For tests, apply the patches:
>
> foreign-recheck-for-foreign-join-1.patch
> usermapping_matching.patch [2]
> add_GetUserMappingById.patch [2]
> foreign_join_v16_efujita.patch [3]
>
> Since that as I said upthread, what I'd like to discuss is changes to
> the PG core, I didn't do anything about the postgres_fdw patches.
>
> Best regards,
> Etsuro Fujita
>
> [1]
> http://www.postgresql.org/message-id/CA+TgmoaAzs0dR23R7PTBseQfwOtuVCPNBqDHxe
> Bo9Gi+dMxj8w(at)mail(dot)gmail(dot)com
> [2]
> http://www.postgresql.org/message-id/CAEZqfEe9KGy=1_waGh2rgZPg0o4pqgD+iauYaj
> 8wTze+CYJUHg(at)mail(dot)gmail(dot)com
> [3] http://www.postgresql.org/message-id/55CB2D45.7040100@lab.ntt.co.jp