Fix handling of redundant options with COPY for "freeze" and "header"

The handling of those options was inconsistent, as the processing used
directly the value assigned to the option to check if it was redundant,
leading to patterns like this one to succeed (note that false is
specified first):
COPY hoge to '/path/to/file/' (header off, header on);

And the opposite would fail correctly (note that true is first here):
COPY hoge to '/path/to/file/' (header on, header off);

While on it, add some tests to check for all redundant patterns with the
options of COPY.  I have gone through the code and did not notice
similar mistakes for other commands.

"header" got it wrong since b63990c, and "freeze" was wrong from the
start as of 8de72b6.  No backpatch is done per the lack of complaints.

Reported-by: Rémi Lapeyre
Discussion: https://postgr.es/m/20200929072433[email protected]
Discussion: https://postgr.es/m/0B55BD07-83E4-439F-AACC-FA2D7CF50532@lenstra.fr

diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 2047557e52005ee2cef590e4847f87d446b3ea55..3c7dbad27a21bba97c44beeee427df9bf8dad8bf 100644 (file)
--- 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 e40287d25a41390d22a236c69800e0687e61d58e..c64f0719e7b6c9770b19fa99c152db416400164b 100644 (file)
--- 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 902f4fac19a42c4768b068ad56631ea9dab21a7f..b3c16af48eec8503901263c61843ff2a43421c82 100644 (file)
--- 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;