From 2f49e9feb2827856c8a0e3098500109ddd1962c9 Mon Sep 17 00:00:00 2001 From: Julien Rouhaud Date: Mon, 13 Jun 2022 09:49:23 +0800 Subject: [PATCH v1] Fix processing of header match option for COPY. Thinko in 072132f04en which used the attnum offset to access the raw_fields array, leading to incorrect results of crash. Use the correct variable, and add some regression tests to cover a bit more scenario for the HEADER MATCH option. While at it, disallow HEADER MATCH in COPY TO as there is no validation that can be done in that case. Author: Julien Rouhaud Discussion: https://postgr.es/m/20220607154744.vvmitnqhyxrne5ms@jrouhaud --- doc/src/sgml/ref/copy.sgml | 2 ++ src/backend/commands/copy.c | 12 ++++++++++-- src/backend/commands/copyfromparse.c | 5 +++-- src/test/regress/expected/copy.out | 21 ++++++++++++++++++++- src/test/regress/sql/copy.sql | 24 ++++++++++++++++++++---- 5 files changed, 55 insertions(+), 9 deletions(-) diff --git a/doc/src/sgml/ref/copy.sgml b/doc/src/sgml/ref/copy.sgml index 40af423ccf..8aae711b3b 100644 --- a/doc/src/sgml/ref/copy.sgml +++ b/doc/src/sgml/ref/copy.sgml @@ -282,6 +282,8 @@ COPY { table_name [ ( binary format. + The MATCH option is only valid for COPY + FROM commands. diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c index f448d39c7e..e596bebb0b 100644 --- a/src/backend/commands/copy.c +++ b/src/backend/commands/copy.c @@ -318,7 +318,7 @@ DoCopy(ParseState *pstate, const CopyStmt *stmt, * defGetBoolean() but also accepts the special value "match". */ static CopyHeaderChoice -defGetCopyHeaderChoice(DefElem *def) +defGetCopyHeaderChoice(DefElem *def, bool is_from) { /* * If no parameter given, assume "true" is meant. @@ -360,7 +360,15 @@ defGetCopyHeaderChoice(DefElem *def) if (pg_strcasecmp(sval, "off") == 0) return COPY_HEADER_FALSE; if (pg_strcasecmp(sval, "match") == 0) + { + /* match is only valid for COPY FROM */ + if (!is_from) + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("%s match is only valid for COPY FROM", + def->defname))); return COPY_HEADER_MATCH; + } } break; } @@ -452,7 +460,7 @@ ProcessCopyOptions(ParseState *pstate, if (header_specified) errorConflictingDefElem(defel, pstate); header_specified = true; - opts_out->header_line = defGetCopyHeaderChoice(defel); + opts_out->header_line = defGetCopyHeaderChoice(defel, is_from); } else if (strcmp(defel->defname, "quote") == 0) { diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c index e06534943f..57813b3458 100644 --- a/src/backend/commands/copyfromparse.c +++ b/src/backend/commands/copyfromparse.c @@ -789,11 +789,12 @@ NextCopyFromRawFields(CopyFromState cstate, char ***fields, int *nfields) foreach(cur, cstate->attnumlist) { int attnum = lfirst_int(cur); - char *colName = cstate->raw_fields[attnum - 1]; + char *colName; Form_pg_attribute attr = TupleDescAttr(tupDesc, attnum - 1); - fldnum++; + Assert(fldnum < cstate->max_fields); + colName = cstate->raw_fields[fldnum++]; if (colName == NULL) ereport(ERROR, (errcode(ERRCODE_BAD_COPY_FILE_FORMAT), diff --git a/src/test/regress/expected/copy.out b/src/test/regress/expected/copy.out index e8d6b4fc13..e9d1ec348a 100644 --- a/src/test/regress/expected/copy.out +++ b/src/test/regress/expected/copy.out @@ -182,9 +182,21 @@ create table header_copytest ( b int, c text ); +-- Make sure it works with with dropped columns +alter table header_copytest drop column c; +alter table header_copytest add column c text; +copy header_copytest to stdout with (header match); +ERROR: header match is only valid for COPY FROM copy header_copytest from stdin with (header wrong_choice); ERROR: header requires a Boolean value or "match" +-- works copy header_copytest from stdin with (header match); +copy header_copytest (c, a, b) from stdin with (header match); +copy header_copytest from stdin with (header match, format csv); +-- the rest errors out +copy header_copytest (c, b, a) from stdin with (header match); +ERROR: column name mismatch in header line field 1: got "a", expected "c" +CONTEXT: COPY header_copytest, line 1: "a b c" copy header_copytest from stdin with (header match); ERROR: column name mismatch in header line field 3: got null value ("\N"), expected "c" CONTEXT: COPY header_copytest, line 1: "a b \N" @@ -197,5 +209,12 @@ CONTEXT: COPY header_copytest, line 1: "a b c d" copy header_copytest from stdin with (header match); ERROR: column name mismatch in header line field 3: got "d", expected "c" CONTEXT: COPY header_copytest, line 1: "a b d" -copy header_copytest from stdin with (header match, format csv); +SELECT * FROM header_copytest ORDER BY a; + a | b | c +---+---+----- + 1 | 2 | foo + 3 | 4 | bar + 5 | 6 | baz +(3 rows) + drop table header_copytest; diff --git a/src/test/regress/sql/copy.sql b/src/test/regress/sql/copy.sql index d72d226f34..0b15ed2653 100644 --- a/src/test/regress/sql/copy.sql +++ b/src/test/regress/sql/copy.sql @@ -204,11 +204,29 @@ create table header_copytest ( b int, c text ); +-- Make sure it works with with dropped columns +alter table header_copytest drop column c; +alter table header_copytest add column c text; +copy header_copytest to stdout with (header match); copy header_copytest from stdin with (header wrong_choice); +-- works copy header_copytest from stdin with (header match); a b c 1 2 foo \. +copy header_copytest (c, a, b) from stdin with (header match); +c a b +bar 3 4 +\. +copy header_copytest from stdin with (header match, format csv); +a,b,c +5,6,baz +\. +-- the rest errors out +copy header_copytest (c, b, a) from stdin with (header match); +a b c +1 2 foo +\. copy header_copytest from stdin with (header match); a b \N 1 2 foo @@ -225,8 +243,6 @@ copy header_copytest from stdin with (header match); a b d 1 2 foo \. -copy header_copytest from stdin with (header match, format csv); -a,b,c -1,2,foo -\. + +SELECT * FROM header_copytest ORDER BY a; drop table header_copytest; -- 2.33.1