On 2016/01/21 5:06, Robert Haas wrote:
On Wed, Jan 20, 2016 at 4:50 AM, Etsuro Fujita
<fujita.ets...@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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
