On Fri, Sep 2, 2016 at 5:00 PM, Etsuro Fujita <fujita.ets...@lab.ntt.co.jp> wrote:
> Hi Robert, > > Thanks for the comments! > > On 2016/09/02 11:55, Robert Haas wrote: > >> On Mon, Aug 1, 2016 at 5:44 PM, Etsuro Fujita >> <fujita.ets...@lab.ntt.co.jp> wrote: >> >>> I noticed that the following note about direct modification via >>> GetForeignUpperPaths in fdwhandler.sgml is a bit confusing. We have >>> another >>> approach using PlanDirectModify, so that should be reflected in the note >>> as >>> well. Please find attached a patch. >>> >>> <function>PlanForeignModify</> and the other callbacks described in >>> <xref linkend="fdw-callbacks-update"> are designed around the >>> assumption >>> that the foreign relation will be scanned in the usual way and then >>> individual row updates will be driven by a local >>> <literal>ModifyTable</> >>> plan node. This approach is necessary for the general case where an >>> update requires reading local tables as well as foreign tables. >>> However, if the operation could be executed entirely by the foreign >>> server, the FDW could generate a path representing that and insert >>> it >>> into the <literal>UPPERREL_FINAL</> upper relation, where it would >>> compete against the <literal>ModifyTable</> approach. >>> >> > I suppose this is factually correct, but I don't think it's very >> illuminating. I think that if we're going to document both >> approaches, there should be some discussion of the pros and cons of >> PlanDirectModify vs. PlanForeignModify. >> > > PlanDirectModify vs. GetForeignUpperPaths for an UPPERREL_FINAL upper > relation? > > Of course either should be >> better than an iterative ModifyTable, but how should the FDW author >> decide between the two of them? >> > > That would apply to row locking. We have two approaches for that too: > GetForeignRowMarkType and GetForeignUpperPaths, which is documented in the > same paragraph following the above documentation: > > This approach > could also be used to implement remote <literal>SELECT FOR UPDATE</>, > rather than using the row locking callbacks described in > <xref linkend="fdw-callbacks-row-locking">. Keep in mind that a path > > The point of the patch is just to let the FDW author know that there is > another approach for implementing direct modification (ie, > PlanDirectModify) just as for implementing row locking. > > Considering the primary object of this patch is just to let the FDW author know that there is another approach for implementing direct modification, I like the idea of modifying the document. I agree that the documentation about how the FDW author should decide > between the two would be helpful, but I'd like to leave that for future > work. > I performed basic test with patch, a) patch get applied cleanly on latest source, b) able to build documentation cleanly. Marking this as ready for committer. > > > -- > 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
