From: | Michael Paquier <michael(at)paquier(dot)xyz> |
---|---|
To: | Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp> |
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 06:54:54 |
Message-ID: | 20190116065454.GA2550@paquier.xyz |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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. Except of that nit,
it looks fine to me, thanks for taking care of it.
--
Michael
From | Date | Subject | |
---|---|---|---|
Next Message | Amit Langote | 2019-01-16 07:41:49 | Re: using expression syntax for partition bounds |
Previous Message | Tsunakawa, Takayuki | 2019-01-16 06:41:58 | RE: Protect syscache from bloating with negative cache entries |