Re: Odd behavior in foreign table modification (Was: Re: Optimization for updating foreign tables in Postgres FDW)

On 2016/01/21 5:06, Robert Haas wrote:
> On Wed, Jan 20, 2016 at 4:50 AM, Etsuro Fujita
> <fujita.etsuro@lab.ntt.co.jp> wrote:
>> My concern about that is that would make the code in deparseTargetList()
>> complicated.
>>
>> Essentially, I think your propossal needs a two-pass algorithm for
>> deparseTargetList; (1) create an integer List of the columns being retrieved
>> from the given attrs_used (getRetrievedAttrs()), and (2) print those columns
>> (printRetrievedAttrs()).  How about sharing those two functions between
>> deparseTargetList and deparseReturningList?:

> I don't see why we'd need that.  I adjusted the code in postgres_fdw
> along the lines I had in mind and am attaching the result.  It doesn't
> look complicated to me, and it passes the regression test you wrote.

Thanks for the patch!  From the patch, I correctly understand what you 
proposed.  Good idea!

> By the way, I'm not too sure I understand the need for the core
> changes that are part of this patch, and I think that point merits
> some discussion.  Whenever you change core like this, you're changing
> the contract between the FDW and core; it's not just postgres_fdw that
> needs updating, but every FDW.  So we'd better be pretty sure we need
> these changes and they are adequately justified before we think about
> putting them into the tree.  Are these core changes really needed
> here, or can we fix this whole issue in postgres_fdw and leave the
> core code alone?

Well, if we think it is the FDW's responsibility to insert a valid value 
for tableoid in the returned slot during ExecForeignInsert, 
ExecForeignUpdate or ExecForeignDelete, we don't need those core 
changes.  However, I think it would be better that that's done by 
ModifyTable in the same way as ForeignScan does in ForeignNext, IMO. 
That eliminates the need for postgres_fdw or any other FDW to do that 
business in the callback routines.

Best regards,
Etsuro Fujita