Thread

  1. [PATCH v1] Fix processing of header match option for COPY.

    Julien Rouhaud <julien.rouhaud@free.fr> — 2022-06-13T01:49:23Z

    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 { <replaceable class="parameter">table_name</replaceable> [ ( <replaceable
           of the columns in the header line must match the actual column names of
           the table, otherwise an error is raised.
           This option is not allowed when using <literal>binary</literal> format.
    +      The <literal>MATCH</literal> option is only valid for <command>COPY
    +      FROM</command> commands.
          </para>
         </listitem>
        </varlistentry>
    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
    
    
    --7dpw752g5cnup6cx--