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


> -----Original Message-----
> From: Etsuro Fujita [mailto:fujita.ets...@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+dmx...@mail.gmail.com
> [2]
> http://www.postgresql.org/message-id/CAEZqfEe9KGy=1_waGh2rgZPg0o4pqgD+iauYaj
> 8wtze+cyj...@mail.gmail.com
> [3] http://www.postgresql.org/message-id/55cb2d45.7040...@lab.ntt.co.jp

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to