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

On Tue, Mar 4, 2025 at 4:06 PM Sutou Kouhei <kou(at)clear-code(dot)com> wrote:
>
> Hi,
>
> In <CAD21AoAwOP7p6LgmkPGqPuJ5KbJPPQsSZsFzwCDguwzr9F677Q(at)mail(dot)gmail(dot)com>
> "Re: Make COPY format extendable: Extract COPY TO format implementations" on Mon, 3 Mar 2025 11:06:39 -0800,
> Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>
> > I agree with the fix and the patch looks good to me. I've updated the
> > commit message and am going to push, barring any objections.
>
> Thanks!
>
> I've rebased the patch set. Here is a summary again:

Thank you for updating the patches. Here are some review comments on
the 0001 patch:

+ if (strcmp(format, "text") == 0)
+ {
+ /* "csv_mode == false && binary == false" means "text" */
+ return;
+ }
+ else if (strcmp(format, "csv") == 0)
+ {
+ opts_out->csv_mode = true;
+ return;
+ }
+ else if (strcmp(format, "binary") == 0)
+ {
+ opts_out->binary = true;
+ return;
+ }
+
+ /* custom format */
+ if (!is_from)
+ {
+ funcargtypes[0] = INTERNALOID;
+ handlerOid = LookupFuncName(list_make1(makeString(format)), 1,
+ funcargtypes, true);
+ }
+ if (!OidIsValid(handlerOid))
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("COPY format \"%s\" not recognized", format),
+ parser_errposition(pstate, defel->location)));

I think that built-in formats also need to have their handler
functions. This seems to be a conventional way for customizable
features such as tablesample and access methods, and we can simplify
this function.

---
I think we need to update the documentation to describe how users can
define the handler functions and what each callback function is
responsible for.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com