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

Hi,

Thanks for restarting this.

In <CAD21AoCnA7vayZAOmwVqTSOyWfyBhyxH7mBb4UzjskF-eZ+_Jg(at)mail(dot)gmail(dot)com>
"Re: Make COPY format extendable: Extract COPY TO format implementations" on Mon, 22 Jun 2026 18:06:07 -0700,
Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:

> After more thought, I'd like to keep the custom-format changes to the
> bare minimum and not disturb the existing built-in format processing.

+1

> Updated patches attached:
>
> - 0001 moves CopyFromStateData and CopyToStateData to a new
> copy_state.h, so extensions can implement their routines without
> including the *_internal.h headers. It also drops file_fdw.c's
> dependency on copyfrom_internal.h.

+1

> - 0002 introduces the registration API and the opaque per-format
> pointer in both structs.

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

> +bool
> +GetCopyCustomFormatRoutines(const char *name, const CopyToRoutine **to,
> + const CopyFromRoutine **from, ProcessOneOptionFn * option_fn)

How about returning CopyCustomFormatEntry instead? The
function name is "Get...Routines" but it also returns
ProcessOneOptionFn. "Get...Routines" is a bit strange.

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

> @@ -102,4 +103,40 @@ typedef struct CopyFromRoutine
> ...
> +typedef bool (*ProcessOneOptionFn) (CopyFormatOptions *opts, bool is_from,
> + DefElem *option);

How about adding "Copy" keyword to the type name such as
"ProcessOneCopyOptionFn" because this is only for COPY format?

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

> @@ -58,7 +58,16 @@ typedef enum CopyFormat
> ...
> +#define CopyFormatBuiltins(format) ((format) != COPY_FORMAT_CUSTOM)

How about renaming this to CopyFormatIsBuiltin() or
something? "...Builtins" is a bit strange because this
returns a boolean.

> - 0003 adds a callback to validate the COPY options as a whole, called
> after all options are processed.

> --- a/src/include/commands/copyapi.h
> +++ b/src/include/commands/copyapi.h
> @@ -120,6 +120,15 @@ typedef struct CopyFromRoutine
> ...
> +typedef void (*ValidateOptionsFn) (CopyFormatOptions *opts, bool is_from);

How about adding "Copy" keyword like "ValidateCopyOptionsFn"?

> - 0004 adds the regression tests.

> --- /dev/null
> +++ b/src/test/modules/test_copy_custom_format/test_copy_custom_format.c

> @@ -0,0 +1,169 @@
> ...
> +TestCopyProcessOneOption(CopyFormatOptions *opts, bool is_from, DefElem *option)
> +{
> + TestCopyOptions *t = (TestCopyOptions *) opts->format_private_opts;
> +
> + if (t == NULL)
> + {
> + t = palloc0_object(TestCopyOptions);
> + opts->format_private_opts = (void *) t;
> + }

This is not a blocker but we may want to add
InitializeCopyOptions callback for this.

Thanks,
--
kou