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

From: Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>
To: Dmitry Dolgov <9erthalion6(at)gmail(dot)com>
Cc: Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: de-deduplicate code in DML execution hooks in postgres_fdw
Date: 2019-01-07 11:26:39
Message-ID: 5C33376F.10603@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

(2018/11/30 19:55), Etsuro Fujita wrote:
> One thing we would need to discuss more about this is the name of a new
> function added by the patch. IIRC, we have three options so far [1]:
>
> 1) perform_one_foreign_dml proposed by Ashutosh
> 2) execute_dml_single_row proposed by Michael
> 3) execute_parameterized_dml_stmt proposed by me
>
> I'll withdraw my proposal; I think #1 and #2 would describe the
> functionality better than #3. My vote would go to #2 or
> execute_dml_stmt_single_row, which moves the function much more closer
> to execute_dml_stmt. I'd like to hear the opinions of others.

On second thought I'd like to propose another option:
execute_foreign_modify because I think this would match the existing
helper functions for updating foreign tables in postgres_fdw.c better,
such as create_foreign_modify, prepare_foreign_modify and
finish_foreign_modify. I don't really think the function name should
contain "one" or "single_row". Like the names of the calling APIs (ie,
ExecForeignInsert, ExecForeignUpdate and ExecForeignDelete), I think
it's OK to omit such words from the function name. Here is an updated
version of the patch. In addition to the naming, I tweaked the function
a little bit to match other functions (mainly variable names), moved it
to the place where the helper functions are defined, fiddled with some
comments, and removed an extra include file that the original patch added.

Best regards,
Etsuro Fujita

Attachment Content-Type Size
Refactor-postgres_fdw-s-DML-execution-hook-functions-v2.patch text/x-patch 9.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Khandekar 2019-01-07 11:33:20 Re: pgsql: Remove WITH OIDS support, change oid catalog column visibility.
Previous Message Haozhou Wang 2019-01-07 10:56:09 Re: Control your disk usage in PG: Introduction to Disk Quota Extension