Re: postgres_fdw: Use COPY to speed up batch inserts

Matheus Alcantara <matheusssilv97@gmail.com>

From: "Matheus Alcantara" <matheusssilv97@gmail.com>
To: "Dewei Dai" <daidewei1970@163.com>, "Matheus Alcantara" <matheusssilv97@gmail.com>, "Masahiko Sawada" <sawada.mshk@gmail.com>
Cc: "Andrew Dunstan" <andrew@dunslane.net>, "jian he" <jian.universality@gmail.com>, "Tomas Vondra" <tomas@vondra.me>, "pgsql-hackers" <pgsql-hackers@postgresql.org>
Date: 2026-01-05T13:43:52Z
Lists: pgsql-hackers

Attachments

Hi, thank you for reviewing this patch!

On Sat Jan 3, 2026 at 9:45 AM -03, Dewei Dai wrote:
> 1 - in function `execute_foreign_modify_using_copy`
>   The `res` object obtained from the first call to `pgfdw_get_result`
>   is not freed, maybe you can use `PQclear` to release it
>
Fixed.

> 2 - in function `execute_foreign_modify_using_copy`
>   After using `copy_data`,it appears it can be released by
>     calling `destroyStringInfo`.
>
I'm wondering if we should call destroyStringInfo(&copy_data) or
pfree(copy_data.data). Other functions use the pfree version, so I
decided to use the same.

(I actually tried to use destroyStringInfo() but the postgres_fdw tests
kept running for longer than usual, so I think that using the pfree is
correct)

> 3 - in function `convert_slot_to_copy_text`
>   The value returned by the OutputFunctionCall function can be
>     freed by calling pfree
>   
We have other calls to OutputFunctionCall() on postgres_fdw.c and I'm
not seeing a subsequent call to pfree. IIUC the returned valued will be
allocated on the current memory context which will be free at the end of
query execution, so I don't think that a pfree here is necessary, or I'm
missing something?

> 4 - in function `execute_foreign_modify_using_copy`
> ```Send initial COPY data if the buffer reach the limit to avoid large
> ```
> Typo: reach ->  reaches 
>
Fixed

--
Matheus Alcantara
EDB: https://www.enterprisedb.com