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

On Thu, Mar 27, 2025 at 11:29 AM Sutou Kouhei <kou(at)clear-code(dot)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), ....