Hello. At Tue, 24 Apr 2018 15:49:20 -0400, Robert Haas <robertmh...@gmail.com> wrote in <ca+tgmob+ciht+9jeuixznvmucuf1zbb-bueffvmbzwjhv0y...@mail.gmail.com> > On Tue, Apr 24, 2018 at 12:21 PM, Robert Haas <robertmh...@gmail.com> wrote: > > On Mon, Apr 23, 2018 at 11:25 AM, Alvaro Herrera > > <alvhe...@alvh.no-ip.org> wrote: > >> Robert, I think this is your turf, per 3d956d9562aa. Are you looking > >> into it? > > > > Thanks for the ping. I just had a look over the proposed patch and I > > guess I don't like it very much. Temporarily modifying the range > > table in place and then changing it back before we return seems ugly > > and error-prone. I hope we can come up with a solution that doesn't > > involve needing to do that. > > I have done some refactoring to avoid that. See attached. > > I didn't really get beyond the refactoring stage with this today. > This version still seems to work, but I don't really understand the > logic in postgresBeginForeignInsert which decides whether to use the > RTE from the range table or create our own. We seem to need to do one > sometimes and the other sometimes, but I don't know why that is, > really. Nor do I understand why we need the other changes in the > patch. There's probably a good reason, but I haven't figured it out > yet.
FWIW, the refactoring drops the condition that the ForeignInsert is a part of UPDATE. The code exists just for avoiding temprary RTE generation in 2 cases out of the 3 possible cases mentioned in [1]. If it looks too complex for the gain, we can always create an RTE for deparsing as Fujita-san's first patch in this thread did. Anyway the condition for "dostuff" + "is update" might be a bit too arbitrary. [1] https://www.postgresql.org/message-id/f970d875-9711-b8cb-f270-965fa3e40...@lab.ntt.co.jp regards. -- Kyotaro Horiguchi NTT Open Source Software Center
