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: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Dmitry Dolgov <9erthalion6(at)gmail(dot)com>, Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: de-deduplicate code in DML execution hooks in postgres_fdw
Date: 2019-01-16 11:30:41
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


(2019/01/16 15:54), Michael Paquier wrote:
> On Wed, Jan 16, 2019 at 02:59:15PM +0900, Etsuro Fujita wrote:
>> (2019/01/07 20:26), Etsuro Fujita wrote:
>>> 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.
>> If there are no objections, I'll commit that version of the patch.
> I think that you could use PgFdwModifyState for the second argument of
> execute_foreign_modify instead of ResultRelInfo.

Yeah, that is another option, but my favorite would be to use
ResultRelInfo, as in the original patch by Ashutosh, because that makes
it possible to write postgresExecForeignInsert,
postgresExecForeignUpdate, and postgresExecForeignDelete as a single line.

> Except of that nit,
> it looks fine to me, thanks for taking care of it.

Great! Thanks for the review!

Best regards,
Etsuro Fujita

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Alexander Kuzmenkov 2019-01-16 11:39:53 Redundant filter in index scan with a bool column
Previous Message Etsuro Fujita 2019-01-16 11:21:32 Re: Query with high planning time at version 11.1 compared versions 10.5 and 11.0