(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.
Best regards,
Etsuro Fujita
