On Thu, Jul 19, 2018 at 2:05 PM, Etsuro Fujita <fujita.ets...@lab.ntt.co.jp> wrote: > (2018/04/18 19:34), Ashutosh Bapat wrote: >> >> Hi, >> While working on a fix related to non-direct DML [1], I noticed that >> postgresExecForeignInsert(), postgresExecForeignUpdate() and >> postgresExecForeignDelete() functions are almost identical except that >> postgresExecForeignInsert() doesn't require ctid. The fix that I was >> working is applicable to Delete and Update but can be useful for >> Insert as well. I had to add the same code to two places at least and >> might have missed fixing one of them. Why don't we have a single >> function which prepares the statement, extract parameters, executes >> the prepared statement and checks for the results, returned rows etc? >> It's been a while that these functions are there and haven't produced >> code which is a lot different for each of these cases. Here's a patch >> to extract that code into a separate function and use it in all the >> three hook implementations. >> >> [1] >> https://www.postgresql.org/message-id/CAFjFpRfcgwsHRmpvoOK-GUQi-n8MgAS+OxcQo=abdn1coyw...@mail.gmail.com > > > +1 for the general idea. (Actually, I also thought the same thing before.) > But since this is definitely a matter of PG12, ISTM that it's wise to work > on this after addressing the issue in [1]. My concern is: if we do this > refactoring now, we might need two patches for fixing the issue in case of > backpatching as the fix might need to change those executor functions.
The only thing in [1] that would conflict with this patch is the 0002 (and possibly 0001) patch in [2]. We haven't yet decided anything about whether those patches can be back-patched or not. I think it's going to take much longer time for the actual solution to arise. But we don't have to wait for it to commit this patch as well as 0001 and 0002 patches in [2] because a. the larger solution is not likely to be back-patchable b. it's going to take much longer time. We don't have any estimate about how much longer it's going to take. We don't want this small piece to get stuck for that larger piece worst get stuck indefinitely. So, I will vote for getting it committed ASAP, if we agree that it's good change. If the solution to [1] happens to require this refactoring we we can back-patch that since back-patching it won't hurt. [1] https://www.postgresql.org/message-id/CAFjFpRfcgwsHRmpvoOK-GUQi-n8MgAS+OxcQo=abdn1coyw...@mail.gmail.com [2] https://www.postgresql.org/message-id/CAFjFpRfK69ptCTNChBBk%2BLYMXFzJ92SW6NmG4HLn_1y7xFk%3Dkw%40mail.gmail.com -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company
