On Tue, Mar 4, 2025 at 4:06 PM Sutou Kouhei <k...@clear-code.com> wrote: > > Hi, > > In <cad21aoawop7p6lgmkpgqpuj5kbjppqsszsfzwcdguwzr9f6...@mail.gmail.com> > "Re: Make COPY format extendable: Extract COPY TO format implementations" > on Mon, 3 Mar 2025 11:06:39 -0800, > Masahiko Sawada <sawada.m...@gmail.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