Hi, Thanks for restarting this.
In <cad21aocna7vayzaomwvqtsoywfybhyxh7mbb4uzjskf-ez+...@mail.gmail.com> "Re: Make COPY format extendable: Extract COPY TO format implementations" on Mon, 22 Jun 2026 18:06:07 -0700, Masahiko Sawada <[email protected]> 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
