Re: Make COPY format extendable: Extract COPY TO format implementations - Mailing list pgsql-hackers

On Thu, Mar 27, 2025 at 11:29 AM Sutou Kouhei <kou@clear-code.com> wrote:
> We can merge 0001 quickly, right?

I did a brief review of v39-0001 and v39-0002.

text:
COPY_FILE
COPY_FRONTEND
still appear on comments in copyfrom_internal.h and copyto.c,
Should it be removed?


+#include "commands/copyto_internal.h"
#include "commands/progress.h"
#include "executor/execdesc.h"
#include "executor/executor.h"
#include "executor/tuptable.h"

"copyto_internal.h" already include:

#include "executor/execdesc.h"
#include "executor/tuptable.h"
so you should removed
"
#include "executor/execdesc.h"
#include "executor/tuptable.h"
"
in copyto.c.



CREATE FUNCTION test_copy_format(internal)
    RETURNS copy_handler
    AS 'MODULE_PATHNAME', 'test_copy_format'
    LANGUAGE C;
src/backend/commands/copy.c: ProcessCopyOptions
            if (strcmp(fmt, "text") == 0)
                 /* default format */ ;
            else if (strcmp(fmt, "csv") == 0)
                opts_out->csv_mode = true;
            else if (strcmp(fmt, "binary") == 0)
                opts_out->binary = true;
            else
            {
                List       *qualified_format;
                ....
            }
what if our customized format name is one of "csv", "binary", "text",
then that ELSE branch will never be reached.
then our customized format is being shadowed?


https://www.postgresql.org/docs/current/error-message-reporting.html
"The extra parentheses were required before PostgreSQL version 12, but
are now optional."

means that
    ereport(NOTICE, (errmsg("CopyFromInFunc: attribute: %s",
format_type_be(atttypid))));
can change to
    ereport(NOTICE, errmsg("CopyFromInFunc: attribute: %s",
format_type_be(atttypid)));

all
ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), ....

can also be simplified to

ereport(ERROR, errcode(ERRCODE_INVALID_PARAMETER_VALUE), ....