Hi all, While diving into the CF, I have noticed the following message from Remy (in CC): https://www.postgresql.org/message-id/0b55bd07-83e4-439f-aacc-fa2d7cf50...@lenstra.fr
The following two cases should fail the same way, but the second does not because we check directly the flag value extracted from the DefElem to see if the option is repeated or not: =# copy (select 1) to '/tmp/data.txt' (header on, header off); ERROR: 42601: conflicting or redundant options =# copy (select 1) to '/tmp/data.txt' (header off, header on); ERROR: 0A000: COPY HEADER available only in CSV mode Looking quickly at the usages of defGetBoolean() across the code, it seems that we are rather consistent on a command-basis to handle such cases (EXPLAIN does not care, subscriptions do, etc.), while COPY is a mixed bag that clearly aims at checking for redundant options correctly. So, attached is a patch to do that, with tests for the various options while on it. This is not something worth a back-patch in my opinion. Any thoughts? -- Michael
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 2047557e52..3c7dbad27a 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -1159,6 +1159,8 @@ ProcessCopyOptions(ParseState *pstate,
List *options)
{
bool format_specified = false;
+ bool freeze_specified = false;
+ bool header_specified = false;
ListCell *option;
/* Support external use for option sanity checking */
@@ -1198,11 +1200,12 @@ ProcessCopyOptions(ParseState *pstate,
}
else if (strcmp(defel->defname, "freeze") == 0)
{
- if (cstate->freeze)
+ if (freeze_specified)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("conflicting or redundant options"),
parser_errposition(pstate, defel->location)));
+ freeze_specified = true;
cstate->freeze = defGetBoolean(defel);
}
else if (strcmp(defel->defname, "delimiter") == 0)
@@ -1225,11 +1228,12 @@ ProcessCopyOptions(ParseState *pstate,
}
else if (strcmp(defel->defname, "header") == 0)
{
- if (cstate->header_line)
+ if (header_specified)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("conflicting or redundant options"),
parser_errposition(pstate, defel->location)));
+ header_specified = true;
cstate->header_line = defGetBoolean(defel);
}
else if (strcmp(defel->defname, "quote") == 0)
diff --git a/src/test/regress/expected/copy2.out b/src/test/regress/expected/copy2.out
index e40287d25a..c64f0719e7 100644
--- a/src/test/regress/expected/copy2.out
+++ b/src/test/regress/expected/copy2.out
@@ -28,6 +28,53 @@ COPY x (a, b, c, d, e) from stdin;
-- non-existent column in column list: should fail
COPY x (xyz) from stdin;
ERROR: column "xyz" of relation "x" does not exist
+-- redundant options
+COPY x from stdin (format CSV, FORMAT CSV);
+ERROR: conflicting or redundant options
+LINE 1: COPY x from stdin (format CSV, FORMAT CSV);
+ ^
+COPY x from stdin (freeze off, freeze on);
+ERROR: conflicting or redundant options
+LINE 1: COPY x from stdin (freeze off, freeze on);
+ ^
+COPY x from stdin (delimiter ',', delimiter ',');
+ERROR: conflicting or redundant options
+LINE 1: COPY x from stdin (delimiter ',', delimiter ',');
+ ^
+COPY x from stdin (null ' ', null ' ');
+ERROR: conflicting or redundant options
+LINE 1: COPY x from stdin (null ' ', null ' ');
+ ^
+COPY x from stdin (header off, header on);
+ERROR: conflicting or redundant options
+LINE 1: COPY x from stdin (header off, header on);
+ ^
+COPY x from stdin (quote ':', quote ':');
+ERROR: conflicting or redundant options
+LINE 1: COPY x from stdin (quote ':', quote ':');
+ ^
+COPY x from stdin (escape ':', escape ':');
+ERROR: conflicting or redundant options
+LINE 1: COPY x from stdin (escape ':', escape ':');
+ ^
+COPY x from stdin (force_quote (a), force_quote *);
+ERROR: conflicting or redundant options
+LINE 1: COPY x from stdin (force_quote (a), force_quote *);
+ ^
+COPY x from stdin (force_not_null (a), force_not_null (b));
+ERROR: conflicting or redundant options
+LINE 1: COPY x from stdin (force_not_null (a), force_not_null (b));
+ ^
+COPY x from stdin (force_null (a), force_null (b));
+ERROR: conflicting or redundant options
+COPY x from stdin (convert_selectively (a), convert_selectively (b));
+ERROR: conflicting or redundant options
+LINE 1: COPY x from stdin (convert_selectively (a), convert_selectiv...
+ ^
+COPY x from stdin (encoding 'sql_ascii', encoding 'sql_ascii');
+ERROR: conflicting or redundant options
+LINE 1: COPY x from stdin (encoding 'sql_ascii', encoding 'sql_ascii...
+ ^
-- too many columns in column list: should fail
COPY x (a, b, c, d, e, d, c) from stdin;
ERROR: column "d" specified more than once
diff --git a/src/test/regress/sql/copy2.sql b/src/test/regress/sql/copy2.sql
index 902f4fac19..b3c16af48e 100644
--- a/src/test/regress/sql/copy2.sql
+++ b/src/test/regress/sql/copy2.sql
@@ -53,6 +53,20 @@ COPY x (a, b, c, d, e) from stdin;
-- non-existent column in column list: should fail
COPY x (xyz) from stdin;
+-- redundant options
+COPY x from stdin (format CSV, FORMAT CSV);
+COPY x from stdin (freeze off, freeze on);
+COPY x from stdin (delimiter ',', delimiter ',');
+COPY x from stdin (null ' ', null ' ');
+COPY x from stdin (header off, header on);
+COPY x from stdin (quote ':', quote ':');
+COPY x from stdin (escape ':', escape ':');
+COPY x from stdin (force_quote (a), force_quote *);
+COPY x from stdin (force_not_null (a), force_not_null (b));
+COPY x from stdin (force_null (a), force_null (b));
+COPY x from stdin (convert_selectively (a), convert_selectively (b));
+COPY x from stdin (encoding 'sql_ascii', encoding 'sql_ascii');
+
-- too many columns in column list: should fail
COPY x (a, b, c, d, e, d, c) from stdin;
signature.asc
Description: PGP signature
