public inbox for [email protected]  

help / color / mirror / Atom feed

From: Sutou Kouhei <[email protected]>
To: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Subject: Re: Make COPY format extendable: Extract COPY TO format implementations
Date: Fri, 26 Jun 2026 12:21:50 +0900 (JST)
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAD21AoBLbmxT6xMHkDjfvBZEJUo1VbCd-vWhwX9WXbpMgaGW_A@mail.gmail.com>
References: <CAD21AoCnA7vayZAOmwVqTSOyWfyBhyxH7mBb4UzjskF-eZ+_Jg@mail.gmail.com>
	<[email protected]>
	<CAD21AoBLbmxT6xMHkDjfvBZEJUo1VbCd-vWhwX9WXbpMgaGW_A@mail.gmail.com>

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 <[email protected]> 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

view thread (125+ messages)

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Make COPY format extendable: Extract COPY TO format implementations
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox