Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 10c5291

Browse files
committed
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.GA15570@paquier.xyz Discussion: https://postgr.es/m/0B55BD07-83E4-439F-AACC-FA2D7CF50532@lenstra.fr
1 parent 97b6144 commit 10c5291

File tree

3 files changed

+67
-2
lines changed

3 files changed

+67
-2
lines changed

src/backend/commands/copy.c

+6-2
Original file line numberDiff line numberDiff line change
@@ -1159,6 +1159,8 @@ ProcessCopyOptions(ParseState *pstate,
11591159
List *options)
11601160
{
11611161
bool format_specified = false;
1162+
bool freeze_specified = false;
1163+
bool header_specified = false;
11621164
ListCell *option;
11631165

11641166
/* Support external use for option sanity checking */
@@ -1198,11 +1200,12 @@ ProcessCopyOptions(ParseState *pstate,
11981200
}
11991201
else if (strcmp(defel->defname, "freeze") == 0)
12001202
{
1201-
if (cstate->freeze)
1203+
if (freeze_specified)
12021204
ereport(ERROR,
12031205
(errcode(ERRCODE_SYNTAX_ERROR),
12041206
errmsg("conflicting or redundant options"),
12051207
parser_errposition(pstate, defel->location)));
1208+
freeze_specified = true;
12061209
cstate->freeze = defGetBoolean(defel);
12071210
}
12081211
else if (strcmp(defel->defname, "delimiter") == 0)
@@ -1225,11 +1228,12 @@ ProcessCopyOptions(ParseState *pstate,
12251228
}
12261229
else if (strcmp(defel->defname, "header") == 0)
12271230
{
1228-
if (cstate->header_line)
1231+
if (header_specified)
12291232
ereport(ERROR,
12301233
(errcode(ERRCODE_SYNTAX_ERROR),
12311234
errmsg("conflicting or redundant options"),
12321235
parser_errposition(pstate, defel->location)));
1236+
header_specified = true;
12331237
cstate->header_line = defGetBoolean(defel);
12341238
}
12351239
else if (strcmp(defel->defname, "quote") == 0)

src/test/regress/expected/copy2.out

+47
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,53 @@ COPY x (a, b, c, d, e) from stdin;
2828
-- non-existent column in column list: should fail
2929
COPY x (xyz) from stdin;
3030
ERROR: column "xyz" of relation "x" does not exist
31+
-- redundant options
32+
COPY x from stdin (format CSV, FORMAT CSV);
33+
ERROR: conflicting or redundant options
34+
LINE 1: COPY x from stdin (format CSV, FORMAT CSV);
35+
^
36+
COPY x from stdin (freeze off, freeze on);
37+
ERROR: conflicting or redundant options
38+
LINE 1: COPY x from stdin (freeze off, freeze on);
39+
^
40+
COPY x from stdin (delimiter ',', delimiter ',');
41+
ERROR: conflicting or redundant options
42+
LINE 1: COPY x from stdin (delimiter ',', delimiter ',');
43+
^
44+
COPY x from stdin (null ' ', null ' ');
45+
ERROR: conflicting or redundant options
46+
LINE 1: COPY x from stdin (null ' ', null ' ');
47+
^
48+
COPY x from stdin (header off, header on);
49+
ERROR: conflicting or redundant options
50+
LINE 1: COPY x from stdin (header off, header on);
51+
^
52+
COPY x from stdin (quote ':', quote ':');
53+
ERROR: conflicting or redundant options
54+
LINE 1: COPY x from stdin (quote ':', quote ':');
55+
^
56+
COPY x from stdin (escape ':', escape ':');
57+
ERROR: conflicting or redundant options
58+
LINE 1: COPY x from stdin (escape ':', escape ':');
59+
^
60+
COPY x from stdin (force_quote (a), force_quote *);
61+
ERROR: conflicting or redundant options
62+
LINE 1: COPY x from stdin (force_quote (a), force_quote *);
63+
^
64+
COPY x from stdin (force_not_null (a), force_not_null (b));
65+
ERROR: conflicting or redundant options
66+
LINE 1: COPY x from stdin (force_not_null (a), force_not_null (b));
67+
^
68+
COPY x from stdin (force_null (a), force_null (b));
69+
ERROR: conflicting or redundant options
70+
COPY x from stdin (convert_selectively (a), convert_selectively (b));
71+
ERROR: conflicting or redundant options
72+
LINE 1: COPY x from stdin (convert_selectively (a), convert_selectiv...
73+
^
74+
COPY x from stdin (encoding 'sql_ascii', encoding 'sql_ascii');
75+
ERROR: conflicting or redundant options
76+
LINE 1: COPY x from stdin (encoding 'sql_ascii', encoding 'sql_ascii...
77+
^
3178
-- too many columns in column list: should fail
3279
COPY x (a, b, c, d, e, d, c) from stdin;
3380
ERROR: column "d" specified more than once

src/test/regress/sql/copy2.sql

+14
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,20 @@ COPY x (a, b, c, d, e) from stdin;
5353
-- non-existent column in column list: should fail
5454
COPY x (xyz) from stdin;
5555

56+
-- redundant options
57+
COPY x from stdin (format CSV, FORMAT CSV);
58+
COPY x from stdin (freeze off, freeze on);
59+
COPY x from stdin (delimiter ',', delimiter ',');
60+
COPY x from stdin (null ' ', null ' ');
61+
COPY x from stdin (header off, header on);
62+
COPY x from stdin (quote ':', quote ':');
63+
COPY x from stdin (escape ':', escape ':');
64+
COPY x from stdin (force_quote (a), force_quote *);
65+
COPY x from stdin (force_not_null (a), force_not_null (b));
66+
COPY x from stdin (force_null (a), force_null (b));
67+
COPY x from stdin (convert_selectively (a), convert_selectively (b));
68+
COPY x from stdin (encoding 'sql_ascii', encoding 'sql_ascii');
69+
5670
-- too many columns in column list: should fail
5771
COPY x (a, b, c, d, e, d, c) from stdin;
5872

0 commit comments

Comments
 (0)