Re: Confusing docs about GetForeignUpperPaths in fdwhandler.sgml

On Fri, Sep 2, 2016 at 5:00 PM, Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)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(dot)etsuro(at)lab(dot)ntt(dot)co(dot)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(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>

--
Rushabh Lathia