On Mon, Dec 21, 2015 at 6:20 PM, Etsuro Fujita <fujita.ets...@lab.ntt.co.jp> wrote:
> On 2015/11/26 18:00, Etsuro Fujita wrote: > >> On 2015/11/25 20:36, Thom Brown wrote: >> >>> On 13 May 2015 at 04:10, Etsuro Fujita <fujita.ets...@lab.ntt.co.jp> >>> wrote: >>> >>>> On 2015/05/13 0:55, Stephen Frost wrote: >>>> >>>>> While the EXPLAIN output changed, the structure hasn't really changed >>>>> from what was discussed previously and there's not been any real >>>>> involvment from the core code in what's happening here. >>>>> >>>>> Clearly, the documentation around how to use the FDW API hasn't changed >>>>> at all and there's been no additions to it for handling bulk work. >>>>> Everything here continues to be done inside of postgres_fdw, which >>>>> essentially ignores the prescribed "Update/Delete one tuple" interface >>>>> for ExecForeignUpdate/ExecForeignDelete. >>>>> >>>>> I've spent the better part of the past two days trying to reason my way >>>>> around that while reviewing this patch and I haven't come out the other >>>>> side any happier with this approach than I was back in >>>>> 20140911153049.gc16...@tamriel.snowman.net. >>>>> >>>>> There are other things that don't look right to me, such as what's >>>>> going >>>>> on at the bottom of push_update_down(), but I don't think there's much >>>>> point going into it until we figure out what the core FDW API here >>>>> should look like. It might not be all that far from what we have now, >>>>> but I don't think we can just ignore the existing, documented, API. >>>>> >>>> > OK, I'll try to introduce the core FDW API for this (and make changes >>>> to the >>>> core code) to address your previous comments. >>>> >>> > I'm a bit behind in reading up on this, so maybe it's been covered >>> since, but is there a discussion of this API on another thread, or a >>> newer patch available? >>> >> > To address Stephen's comments, I'd like to propose the following FDW APIs: > > bool > PlanDMLPushdown (PlannerInfo *root, > ModifyTable *plan, > Index resultRelation, > int subplan_index); > > This is called in make_modifytable, before calling PlanForeignModify. This > checks to see whether a given UPDATE/DELETE .. RETURNING .. is > pushdown-safe and if so, performs planning actions needed for the DML > pushdown. The idea is to modify a ForeignScan subplan accordingly as in > the previous patch. If the DML is pushdown-safe, this returns true, and we > don't call PlanForeignModify anymore. (Else returns false and call > PlanForeignModify as before.) When the DML is pushdown-safe, we hook the > following FDW APIs located in nodeForeignscan.c, instead of > BeginForeignModify, ExecForeignUpdate/ExecForeignDelete and > EndForeignModify: > > void > BeginDMLPushdown (ForeignScanState *node, > int eflags); > > This initializes the DML pushdown, like BeginForeignScan. > > TupleTableSlot * > IterateDMLPushdown (ForeignScanState *node); > > This fetches one returning result from the foreign server, like > IterateForeignScan, if having a RETURNING clause. If not, just return an > empty slot. (I'm thinking that it's required that the FDW replaces the > targetlist of the ForeignScan subplan to the RETURNING clause during > PlanDMLPushdown, if having the clause, so that we do nothing at > ModifyTable.) > > void > EndDMLPushdown (ForeignScanState *node); > > This finishes the DML pushdown, like EndForeignScan. > +1. I like idea of separate FDW API for the DML Pushdown. Was thinking can't we can re-use the IterateForeignScan(ForeignScanState *node) rather then introducing IterateDMLPushdown(ForeignScanState *node) new API ? > I'm attaching a WIP patch, which only includes changes to the core. I'm > now working on the postgres_fdw patch to demonstrate that these APIs work > well, but I'd be happy if I could get any feedback earlier. > > Best regards, > Etsuro Fujita > > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers > > -- Rushabh Lathia
