Re: de-deduplicate code in DML execution hooks in postgres_fdw

(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=aBDn1COywmcg@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