Re: Confusing docs about GetForeignUpperPaths in fdwhandler.sgml

On Fri, Sep 2, 2016 at 5:00 PM, Etsuro Fujita <fujita.etsuro@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.etsuro@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

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.

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