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

On Fri, Jul 20, 2018 at 2:27 PM, Etsuro Fujita
<fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> (2018/07/20 13:49), Michael Paquier wrote:
>>
>> On Thu, Jul 19, 2018 at 05:35:11PM +0900, Etsuro Fujita wrote:
>>>
>>> +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.
>>
>>
>> FWIW, I would think that if some cleanup of the code is obvious, we
>> should make it without waiting for the other issues to settle down
>> because there is no way to know when those are done,
>
>
> I would agree to that if we were late in the development cycle for PG12.
>
>> and this patch
>> could be forgotten.
>
>
> I won't forget this patch. :)
>
>> Looking at the proposed patch, moving the new routine closer to
>> execute_dml_stmt and renaming it execute_dml_single_row would be nicer.
>
>
> Or execute_parameterized_dml_stmt?

I have placed it closer to its caller, I think that's better than
moving it closer to execute_dml_stmt, unless there is any other strong
advantage.

I used perform instead of execute since the later is usually
associated with local operation. I added "foreign" in the name of the
function to indicate that it's executed on foreign server. I am happy
with "remote" as well. I don't think "one" and "single" make any
difference. I don't like "parameterized" since that gets too tied to
the method we are using rather than what's actually being done. In
short I don't find any of the suggestions to be significantly better
or worse than the name I have chosen. Said that, I am not wedded to
any of those. A committer is free to choose anything s/he likes.

--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company