Re: Foreign join pushdown vs EvalPlanQual - Mailing list pgsql-hackers

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@ak.jp.nec.com>


> -----Original Message-----
> From: Etsuro Fujita [mailto:fujita.etsuro@lab.ntt.co.jp]
> Sent: Friday, October 16, 2015 6:01 PM
> To: Robert Haas
> Cc: Kyotaro HORIGUCHI; Kaigai Kouhei(海外 浩平); pgsql-hackers@postgresql.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@mail.gmail.com
> [2]
> http://www.postgresql.org/message-id/CAEZqfEe9KGy=1_waGh2rgZPg0o4pqgD+iauYaj
> 8wTze+CYJUHg@mail.gmail.com
> [3] http://www.postgresql.org/message-id/55CB2D45.7040100@lab.ntt.co.jp