Horton

Re: Make COPY format extendable: Extract COPY TO format implementations

Sutou Kouhei <kou@clear-code.com>

View thread View on postgr.es
From: Sutou Kouhei <kou@clear-code.com>
To: sawada.mshk@gmail.com
Cc: tomas@vondra.me, andres@anarazel.de, michael@paquier.xyz, david.g.johnston@gmail.com, tgl@sss.pgh.pa.us, zhjwpku@gmail.com, pgsql-hackers@postgresql.org
Date: 2026-06-26T03:21:50Z
Lists: pgsql-hackers 
Hi,

In <CAD21AoBLbmxT6xMHkDjfvBZEJUo1VbCd-vWhwX9WXbpMgaGW_A@mail.gmail.com>
  "Re: Make COPY format extendable: Extract COPY TO format implementations" on Tue, 23 Jun 2026 18:15:10 -0700,
  Masahiko Sawada <sawada.mshk@gmail.com> wrote:

> Thank you for reviewing the patches! I've attached updated patches.

+1

I have only a few minor comments:

0002:

> --- /dev/null
> +++ b/src/backend/commands/copyapi.c

> +void
> +RegisterCopyCustomFormat(const char *name, const CopyToRoutine *to,
> +						 const CopyFromRoutine *from, ProcessOneCopyOptionFn option_fn)

How about using "const CopyCustomFormatEntry *" instead of
"to", "from" and "option_fn"? If we use
CopyCustomFormatEntry here, we don't need change the
signature of this function when we add more items.

> +const CopyCustomFormatEntry *
> +GetCopyCustomFormatRoutines(const char *name)

How about renaming this to "...FormatEntry" from
"...FormatRoutines"?

> --- a/src/include/commands/copy.h
> +++ b/src/include/commands/copy.h

> +#define CopyFormatIsBuiltins(format) ((format) != COPY_FORMAT_CUSTOM)

How about removing the last "s" ("...IsBuiltin") because
this processes only one format?

> +	const struct CopyCustomFormatEntry *custom_format_ent;

It may be better that we don't abbreviate "_entry" to "_ent"
here for readability. It seems that we use this abbreviation
only in a few places:

$ git grep '_ent;' src/
src/backend/replication/logical/reorderbuffer.c:		ReorderBufferTupleCidEnt *new_ent;
src/backend/utils/cache/catcache.c:	CatCInProgress in_progress_ent;
src/backend/utils/cache/catcache.c:	catcache_in_progress_stack = &in_progress_ent;
src/backend/utils/cache/catcache.c:			CatCInProgress in_progress_ent;
src/backend/utils/cache/catcache.c:			catcache_in_progress_stack = &in_progress_ent;


> I'll verify that the new API works well with an experimental custom
> copy format extension.

I think that we need to provide more APIs to read/write data
like we did in v40-0003 to implement a custom copy format
extension:
https://www.postgresql.org/message-id/flat/20250425.214534.1841428689427124725.kou%40clear-code.com

At least https://github.com/kou/pg-copy-arrow needs them.


Thanks,
-- 
kou

Commits

  1. Refactor Copy{From|To}GetRoutine() to use pass-by-reference argument.

  2. Refactor COPY FROM to use format callback functions.

  3. Refactor COPY TO to use format callback functions.

  4. Another try to fix BF failure introduced in commit ddd5f4f54a.

  5. Revert "Refactor CopyReadAttributes{CSV,Text}() to use a callback in COPY FROM"

  6. Improve COPY TO performance when server and client encodings match

  7. Simplify signature of CopyAttributeOutCSV() in copyto.c

  8. Revert "Refactor CopyAttributeOut{CSV,Text}() to use a callback in COPY TO"

  9. Refactor CopyAttributeOut{CSV,Text}() to use a callback in COPY TO

  10. Refactor CopyReadAttributes{CSV,Text}() to use a callback in COPY FROM

  11. Add progress reporting of skipped tuples during COPY FROM.

  12. pgbench: Add \syncpipeline

  13. meson: Make gzip and tar optional

  14. Export the external file reader used in COPY FROM as APIs.