Re: Optimization for updating foreign tables in Postgres FDW - Mailing list pgsql-hackers

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.etsuro@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.GC16422@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.

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