Subject: Add header support to text format and matching feature

From: Rémi Lapeyre <[email-masked]>
Date: Fri, 17 Jul 2020 16:54:08 +0200

> It's hard to find an explanation what this patch actually does.  I don't
> want to have to go through threads dating back 4 months to determine
> what was discussed and what was actually implemented.  Since you're
> already using git format-patch, just add something to the commit message.
>
>
> It appears that these are really two separate features, so perhaps they
> should be two patches.

Thanks for the feedback, I've split cleanly the two patches, simplified the
tests and tried to explain the changes in the commit message.

> Also, the new header matching mode could probably use more than one line
> of documentation.

I've improved the documentation, let's me know if it's better.

It seems like cfbot is not happy with the way I'm sending my patches. The wiki
has some good advices on how to write a patch but I couldn't find anything on
how to send it. I've used

   git send-email -v3 --compose --to=... --in-reply-to=... HEAD^^

here but I'm not sure if it's correct. I will see if it works and will try to fix
it if it's not but since it runs once a day it may take some time.

---

From: Magnus Hagander <[email-masked]>
Date: Fri, 17 Jul 2020 17:27:31 +0200

> it if it's not but since it runs once a day it may take some time.

If you have two patches that depend on each other, you should send them as
two attachment to the same email. You now sent them as two separate emails,
and cfbot will then pick up the latest one of them which is only patch 0002
(at least I'm fairly sure that's how it works).

I don't know how to do that with git-send-email, but you can certainly do
it easy with git-format-patch and just attach them using your regular MUA.

(and while the cfbot and the archives have no problems dealing with the
change in subject, it does break threading in some other MUAs, so I would
recommend not doing that and sticking to the existing subject of the thread)

-- 
 Magnus Hagander
 Me: https://www.hagander.net/ <http://www.hagander.net/>
 Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>

---

From: Rémi Lapeyre <[email-masked]>
Date: Fri, 17 Jul 2020 18:48:21 +0200

> 
> I don't know how to do that with git-send-email, but you can certainly do it easy with git-format-patch and just attach them using your regular MUA.
> 
> (and while the cfbot and the archives have no problems dealing with the change in subject, it does break threading in some other MUAs, so I would recommend not doing that and sticking to the existing subject of the thread)
> 

Thanks, here are both patches attached so cfbot can read them.

---

From: vignesh C <[email-masked]>
Date: Sat, 18 Jul 2020 16:53:19 +0530

> Thanks, here are both patches attached so cfbot can read them.

Few comments:
Few tests are failing because of hardcoded path:
+-- test header matching
+CREATE FOREIGN TABLE header_match ("1" int, foo text) SERVER file_server
+OPTIONS (format 'csv', filename
'/Users/remi/src/postgresql/contrib/file_fdw/data/list1.csv',
delimiter ',', header 'match');
+CREATE FOREIGN TABLE header_dont_match (a int, foo text) SERVER file_server
+OPTIONS (format 'csv', filename
'/Users/remi/src/postgresql/contrib/file_fdw/data/list1.csv',
delimiter ',', header 'match');  -- ERROR
Generally path is not specified like this. file_fdw test of make
check-world is failing because of this.

There is one warning present in the changes:
copy.c: In function ‘ProcessCopyOptions’:
copy.c:1208:5: warning: ISO C90 forbids mixed declarations and code
[-Wdeclaration-after-statement]
     char    *sval = defGetString(defel);

There is space before tab in indent in the below code, check for git
diff --check.
+                       if (fldct < list_length(cstate->attnumlist))
+                               ereport(ERROR,
+
(errcode(ERRCODE_BAD_COPY_FILE_FORMAT),
+                                        errmsg("missing header")));

Regards,
Vignesh
EnterpriseDB: http://www.enterprisedb.com

---

From: Rémi Lapeyre <[email-masked]>
Date: Sat, 18 Jul 2020 17:25:55 +0200

Thanks for the feedback,

>     char    *sval = defGetString(defel);

Weirdly this is not caught by clang, even with -Wdeclaration-after-statement.

Here’s a new version that fix all the issues.

Rémi

---

From: "Daniel Verite" <[email-masked]>
Date: Sat, 25 Jul 2020 15:34:22 +0200

> Here’s a new version that fix all the issues.

Here's a review of v4. 

The patch improves COPY in two ways:

- COPY TO and COPY FROM now accept "HEADER ON" for the TEXT format
(previously it was only for CSV)

- COPY FROM also accepts "HEADER match" to tell that there's a header
and that its contents must match the columns of the destination table.
This works for both the CSV and TEXT formats. The syntax for the
columns is the same as for the data and the match is case-sensitive.

The first improvement when submitted alone (in 2018 by Simon Muller)
has been judged not useful enough or even hazardous without any
"match" feature. It was returned with feedback in 2018-10 and
resubmitted by Rémi in 2020-02 with the match feature.

The patches  apply cleanly, "make check" and "make check-world" pass.

In my tests it works fine except for one crash that I can reproduce
on a fresh build and default configuration with:

$ cat >file.txt
i
1

$ psql
postgres=# create table x(i int);
CREATE TABLE
postgres=# \copy x(i) from file.txt with (header match)
COPY 1
postgres=# \copy x(i) from file.txt with (header match)
COPY 1
postgres=# \copy x(i) from file.txt with (header match)
COPY 1
postgres=# \copy x(i) from file.txt with (header match)
COPY 1
postgres=# \copy x(i) from file.txt with (header match)
COPY 1
postgres=# \copy x(i) from file.txt with (header match)
PANIC:	ERRORDATA_STACK_SIZE exceeded
server closed the connection unexpectedly
	This probably means the server terminated abnormally
	before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.

I suspect the reason is the way that PG_TRY/PG_CATCH is used, see below.

Code comments:

+/*
+ * Represents whether the header must be absent, present or present and
match.
+ */
+typedef enum CopyHeader
+{
+	COPY_HEADER_ABSENT,
+	COPY_HEADER_PRESENT,
+	COPY_HEADER_MATCH
+} CopyHeader;
+
 /*
  * This struct contains all the state variables used throughout a COPY
  * operation. For simplicity, we use the same struct for all variants of
COPY,
@@ -136,7 +146,7 @@ typedef struct CopyStateData
	bool		binary; 		/* binary format? */
	bool		freeze; 		/* freeze rows on loading? */
	bool		csv_mode;		/* Comma Separated Value
format? */
-	bool		header_line;	/* CSV or text header line? */
+	CopyHeader  header_line;	/* CSV or text header line? */

After the redefinition into this enum type, there are still a
bunch of references to header_line that treat it like a boolean:

1190:			if (cstate->header_line)
1398:	if (cstate->binary && cstate->header_line)
2119:		if (cstate->header_line)
3635:	if (cstate->cur_lineno == 0 && cstate->header_line)

It works fine since COPY_HEADER_ABSENT is 0 as the first value of the enum,
but maybe it's not good style to count on that.

+			PG_TRY();
+			{
+				if (defGetBoolean(defel))
+					cstate->header_line =
COPY_HEADER_PRESENT;
+				else
+					cstate->header_line =
COPY_HEADER_ABSENT;
+			}
+			PG_CATCH();
+			{
+				char	*sval = defGetString(defel);
+
+				if (!cstate->is_copy_from)
+					PG_RE_THROW();
+
+				if (pg_strcasecmp(sval, "match") == 0)
+					cstate->header_line =
COPY_HEADER_MATCH;
+				else
+					ereport(ERROR,
+					       
(errcode(ERRCODE_SYNTAX_ERROR),
+						 errmsg("header requires a
boolean or \"match\"")));
+			}
+			PG_END_TRY();

It seems wrong to use a PG_CATCH block for this. I understand that
it's because defGetBoolean() calls ereport() on non-booleans, but then
it should be split into an error-throwing function and a
non-error-throwing lexical analysis of the boolean, the above code
calling the latter.
Besides the comments in elog.h above PG_TRY say that
 "the error recovery code
  can either do PG_RE_THROW to propagate the error outwards, or do a
  (sub)transaction abort. Failure to do so may leave the system in an
  inconsistent state for further processing."
Maybe this is what happens with the repeated uses of "match"
eventually failing with ERRORDATA_STACK_SIZE exceeded.

-    HEADER [ <replaceable class="parameter">boolean</replaceable> ]
+    HEADER { <literal>match</literal> | <literal>true</literal> |
<literal>false</literal> }

This should be enclosed in square brackets because HEADER
with no argument is still accepted.

+      names from the table. On input, the first line is discarded when set
+      to <literal>true</literal> or required to match the column names if
set

The elision of "header" as the subject might be misinterpreted as if
it's the first line that is true.  I'd suggest
"when <literal>header>/literal> is set to ..."	to avoid any confusion.

Best regards,
-- 
Daniel Vérité
PostgreSQL-powered mailer: https://www.manitou-mail.org
Twitter: @DanielVerite

---

From: vignesh C <[email-masked]>
Date: Mon, 17 Aug 2020 18:19:09 +0530

Thanks for your comments, Please find my thoughts inline.

> The connection to the server was lost. Attempting reset: Failed.

Fixed, replaced PG_TRY/PG_CATCH with strcmp logic to get the header option.

> but maybe it's not good style to count on that.

Fixed. Changed it to cstate->header_line != COPY_HEADER_ABSENT.

> eventually failing with ERRORDATA_STACK_SIZE exceeded.

Fixed, replaced PG_TRY/PG_CATCH with strcmp logic to get the header option.

> with no argument is still accepted.

Fixed.

> "when <literal>header>/literal> is set to ..."  to avoid any confusion.

Fixed.

Attached v5 patch with the fixes of above comments.
Thoughts?

Regards,
Vignesh
EnterpriseDB: http://www.enterprisedb.com
From 847c0d64dfd4991d52b6c6e47abcd10e23d4bf8b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9mi=20Lapeyre?= <[email-masked]>
Date: Fri, 17 Jul 2020 01:50:06 +0200
Subject: [PATCH v5 1/2] Add header support to "COPY TO" text format

CSV format supports the HEADER option to output a header in the output,
it is convenient when other programs need to consume the output. This
patch adds the same option to the default text format.

Discussion: https://www.postgresql.org/message-id/flat/[email-masked]
---
 contrib/file_fdw/input/file_fdw.source  |  1 -
 contrib/file_fdw/output/file_fdw.source |  4 +---
 doc/src/sgml/ref/copy.sgml              |  3 ++-
 src/backend/commands/copy.c             | 11 +++++++----
 src/test/regress/input/copy.source      | 12 ++++++++++++
 src/test/regress/output/copy.source     |  8 ++++++++
 6 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/contrib/file_fdw/input/file_fdw.source b/contrib/file_fdw/input/file_fdw.source
index 45b728e..83edb71 100644
--- a/contrib/file_fdw/input/file_fdw.source
+++ b/contrib/file_fdw/input/file_fdw.source
@@ -37,7 +37,6 @@ CREATE USER MAPPING FOR regress_no_priv_user SERVER file_server;
 
 -- validator tests
 CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (format 'xml');  -- ERROR
-CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (format 'text', header 'true');      -- ERROR
 CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (format 'text', quote ':');          -- ERROR
 CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (format 'text', escape ':');         -- ERROR
 CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (format 'binary', header 'true');    -- ERROR
diff --git a/contrib/file_fdw/output/file_fdw.source b/contrib/file_fdw/output/file_fdw.source
index 52b4d5f..547b81f 100644
--- a/contrib/file_fdw/output/file_fdw.source
+++ b/contrib/file_fdw/output/file_fdw.source
@@ -33,14 +33,12 @@ CREATE USER MAPPING FOR regress_no_priv_user SERVER file_server;
 -- validator tests
 CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (format 'xml');  -- ERROR
 ERROR:  COPY format "xml" not recognized
-CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (format 'text', header 'true');      -- ERROR
-ERROR:  COPY HEADER available only in CSV mode
 CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (format 'text', quote ':');          -- ERROR
 ERROR:  COPY quote available only in CSV mode
 CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (format 'text', escape ':');         -- ERROR
 ERROR:  COPY escape available only in CSV mode
 CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (format 'binary', header 'true');    -- ERROR
-ERROR:  COPY HEADER available only in CSV mode
+ERROR:  COPY HEADER available only in CSV and text mode
 CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (format 'binary', quote ':');        -- ERROR
 ERROR:  COPY quote available only in CSV mode
 CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (format 'binary', escape ':');       -- ERROR
diff --git a/doc/src/sgml/ref/copy.sgml b/doc/src/sgml/ref/copy.sgml
index 18189ab..c628a69 100644
--- a/doc/src/sgml/ref/copy.sgml
+++ b/doc/src/sgml/ref/copy.sgml
@@ -269,7 +269,8 @@ COPY { <replaceable class="parameter">table_name</replaceable> [ ( <replaceable
       Specifies that the file contains a header line with the names of each
       column in the file.  On output, the first line contains the column
       names from the table, and on input, the first line is ignored.
-      This option is allowed only when using <literal>CSV</literal> format.
+      This option is allowed only when using <literal>CSV</literal> or
+      <literal>text</literal> format.
      </para>
     </listitem>
    </varlistentry>
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index db7d24a..5d5ad43 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -136,7 +136,7 @@ typedef struct CopyStateData
 	bool		binary;			/* binary format? */
 	bool		freeze;			/* freeze rows on loading? */
 	bool		csv_mode;		/* Comma Separated Value format? */
-	bool		header_line;	/* CSV header line? */
+	bool		header_line;	/* CSV or text header line? */
 	char	   *null_print;		/* NULL marker string (server encoding!) */
 	int			null_print_len; /* length of same */
 	char	   *null_print_client;	/* same converted to file encoding */
@@ -1411,10 +1411,10 @@ ProcessCopyOptions(ParseState *pstate,
 				 errmsg("COPY delimiter cannot be \"%s\"", cstate->delim)));
 
 	/* Check header */
-	if (!cstate->csv_mode && cstate->header_line)
+	if (cstate->binary && cstate->header_line)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("COPY HEADER available only in CSV mode")));
+				 errmsg("COPY HEADER available only in CSV and text mode")));
 
 	/* Check quote */
 	if (!cstate->csv_mode && cstate->quote != NULL)
@@ -2147,8 +2147,11 @@ CopyTo(CopyState cstate)
 
 				colname = NameStr(TupleDescAttr(tupDesc, attnum - 1)->attname);
 
-				CopyAttributeOutCSV(cstate, colname, false,
+				if (cstate->csv_mode)
+					CopyAttributeOutCSV(cstate, colname, false,
 									list_length(cstate->attnumlist) == 1);
+				else
+					CopyAttributeOutText(cstate, colname);
 			}
 
 			CopySendEndOfRow(cstate);
diff --git a/src/test/regress/input/copy.source b/src/test/regress/input/copy.source
index a1d529a..2368649 100644
--- a/src/test/regress/input/copy.source
+++ b/src/test/regress/input/copy.source
@@ -134,6 +134,18 @@ this is just a line full of junk that would error out if parsed
 
 copy copytest3 to stdout csv header;
 
+create temp table copytest4 (
+	c1 int,
+	"col with tabulation: 	" text);
+
+copy copytest4 from stdin (header);
+this is just a line full of junk that would error out if parsed
+1	a
+2	b
+\.
+
+copy copytest4 to stdout (header);
+
 -- test copy from with a partitioned table
 create table parted_copytest (
 	a int,
diff --git a/src/test/regress/output/copy.source b/src/test/regress/output/copy.source
index 938d355..c1f7f99 100644
--- a/src/test/regress/output/copy.source
+++ b/src/test/regress/output/copy.source
@@ -95,6 +95,14 @@ copy copytest3 to stdout csv header;
 c1,"col with , comma","col with "" quote"
 1,a,1
 2,b,2
+create temp table copytest4 (
+	c1 int,
+	"col with tabulation: 	" text);
+copy copytest4 from stdin (header);
+copy copytest4 to stdout (header);
+c1	col with tabulation: \t
+1	a
+2	b
 -- test copy from with a partitioned table
 create table parted_copytest (
 	a int,
-- 
1.8.3.1

From 00321a601847ad1c45f6e9fba4ae47bf64e1deb0 Mon Sep 17 00:00:00 2001
From: Vignesh C <[email-masked]>
Date: Mon, 17 Aug 2020 17:48:09 +0530
Subject: [PATCH v5 2/2] Add header matching mode to "COPY FROM"

COPY FROM supports the HEADER option to silently discard the header from
a CSV or text file. It is possible to load by mistake a file that
matches the expected format, for example if two text columns have been
swapped, resulting in garbage in the database.

This option adds the possibility to actually check the header to make
sure it matches what is expected and exit immediatly if it does not.

Discussion: https://www.postgresql.org/message-id/flat/[email-masked]
---
 contrib/file_fdw/input/file_fdw.source  |   6 ++
 contrib/file_fdw/output/file_fdw.source |   9 ++-
 doc/src/sgml/ref/copy.sgml              |  12 ++-
 src/backend/commands/copy.c             | 128 +++++++++++++++++++++++++++++---
 src/test/regress/input/copy.source      |  25 +++++++
 src/test/regress/output/copy.source     |  17 +++++
 6 files changed, 183 insertions(+), 14 deletions(-)

diff --git a/contrib/file_fdw/input/file_fdw.source b/contrib/file_fdw/input/file_fdw.source
index 83edb71..7a3983c 100644
--- a/contrib/file_fdw/input/file_fdw.source
+++ b/contrib/file_fdw/input/file_fdw.source
@@ -79,6 +79,12 @@ CREATE FOREIGN TABLE agg_bad (
 OPTIONS (format 'csv', filename '@abs_srcdir@/data/agg.bad', header 'true', delimiter ';', quote '@', escape '"', null '');
 ALTER FOREIGN TABLE agg_bad ADD CHECK (a >= 0);
 
+-- test header matching
+CREATE FOREIGN TABLE header_match ("1" int, foo text) SERVER file_server
+OPTIONS (format 'csv', filename '@abs_srcdir@/data/list1.csv', delimiter ',', header 'match');
+CREATE FOREIGN TABLE header_dont_match (a int, foo text) SERVER file_server
+OPTIONS (format 'csv', filename '@abs_srcdir@/data/list1.csv', delimiter ',', header 'match');	-- ERROR
+
 -- per-column options tests
 CREATE FOREIGN TABLE text_csv (
     word1 text OPTIONS (force_not_null 'true'),
diff --git a/contrib/file_fdw/output/file_fdw.source b/contrib/file_fdw/output/file_fdw.source
index 547b81f..ebe826b 100644
--- a/contrib/file_fdw/output/file_fdw.source
+++ b/contrib/file_fdw/output/file_fdw.source
@@ -93,6 +93,11 @@ CREATE FOREIGN TABLE agg_bad (
 ) SERVER file_server
 OPTIONS (format 'csv', filename '@abs_srcdir@/data/agg.bad', header 'true', delimiter ';', quote '@', escape '"', null '');
 ALTER FOREIGN TABLE agg_bad ADD CHECK (a >= 0);
+-- test header matching
+CREATE FOREIGN TABLE header_match ("1" int, foo text) SERVER file_server
+OPTIONS (format 'csv', filename '@abs_srcdir@/data/list1.csv', delimiter ',', header 'match');
+CREATE FOREIGN TABLE header_dont_match (a int, foo text) SERVER file_server
+OPTIONS (format 'csv', filename '@abs_srcdir@/data/list1.csv', delimiter ',', header 'match');	-- ERROR
 -- per-column options tests
 CREATE FOREIGN TABLE text_csv (
     word1 text OPTIONS (force_not_null 'true'),
@@ -439,12 +444,14 @@ SET ROLE regress_file_fdw_superuser;
 -- cleanup
 RESET ROLE;
 DROP EXTENSION file_fdw CASCADE;
-NOTICE:  drop cascades to 7 other objects
+NOTICE:  drop cascades to 9 other objects
 DETAIL:  drop cascades to server file_server
 drop cascades to user mapping for regress_file_fdw_superuser on server file_server
 drop cascades to user mapping for regress_no_priv_user on server file_server
 drop cascades to foreign table agg_text
 drop cascades to foreign table agg_csv
 drop cascades to foreign table agg_bad
+drop cascades to foreign table header_match
+drop cascades to foreign table header_dont_match
 drop cascades to foreign table text_csv
 DROP ROLE regress_file_fdw_superuser, regress_file_fdw_user, regress_no_priv_user;
diff --git a/doc/src/sgml/ref/copy.sgml b/doc/src/sgml/ref/copy.sgml
index c628a69..cb8232d 100644
--- a/doc/src/sgml/ref/copy.sgml
+++ b/doc/src/sgml/ref/copy.sgml
@@ -36,7 +36,7 @@ COPY { <replaceable class="parameter">table_name</replaceable> [ ( <replaceable
     FREEZE [ <replaceable class="parameter">boolean</replaceable> ]
     DELIMITER '<replaceable class="parameter">delimiter_character</replaceable>'
     NULL '<replaceable class="parameter">null_string</replaceable>'
-    HEADER [ <replaceable class="parameter">boolean</replaceable> ]
+    HEADER [ <literal>match</literal> | <literal>true</literal> | <literal>false</literal> ]
     QUOTE '<replaceable class="parameter">quote_character</replaceable>'
     ESCAPE '<replaceable class="parameter">escape_character</replaceable>'
     FORCE_QUOTE { ( <replaceable class="parameter">column_name</replaceable> [, ...] ) | * }
@@ -268,9 +268,13 @@ COPY { <replaceable class="parameter">table_name</replaceable> [ ( <replaceable
      <para>
       Specifies that the file contains a header line with the names of each
       column in the file.  On output, the first line contains the column
-      names from the table, and on input, the first line is ignored.
-      This option is allowed only when using <literal>CSV</literal> or
-      <literal>text</literal> format.
+      names from the table. On input, the first line is discarded when
+      <literal>header</literal> is set to <literal>true</literal> or required
+      to match the column names if set to <literal>match</literal>. If the
+      number of columns in the header is not correct, their order differs
+      from the one expected, or the name or case do not match, the copy will
+      be aborted with an error.  This option is allowed only when using
+      <literal>CSV</literal> or <literal>text</literal> format.
      </para>
     </listitem>
    </varlistentry>
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 5d5ad43..0625090 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -96,6 +96,16 @@ typedef enum CopyInsertMethod
 } CopyInsertMethod;
 
 /*
+ * Represents whether the header must be absent, present or present and match.
+ */
+typedef enum CopyHeader
+{
+	COPY_HEADER_ABSENT,
+	COPY_HEADER_PRESENT,
+	COPY_HEADER_MATCH
+} CopyHeader;
+
+/*
  * This struct contains all the state variables used throughout a COPY
  * operation. For simplicity, we use the same struct for all variants of COPY,
  * even though some fields are used in only some cases.
@@ -136,7 +146,7 @@ typedef struct CopyStateData
 	bool		binary;			/* binary format? */
 	bool		freeze;			/* freeze rows on loading? */
 	bool		csv_mode;		/* Comma Separated Value format? */
-	bool		header_line;	/* CSV or text header line? */
+	CopyHeader  header_line;	/* CSV or text header line? */
 	char	   *null_print;		/* NULL marker string (server encoding!) */
 	int			null_print_len; /* length of same */
 	char	   *null_print_client;	/* same converted to file encoding */
@@ -1136,6 +1146,64 @@ DoCopy(ParseState *pstate, const CopyStmt *stmt,
 }
 
 /*
+ * Extract a CopyHeader value from a DefElem.
+ */
+static CopyHeader
+DefGetCopyHeader(DefElem *def)
+{
+	/*
+	 * If no parameter given, assume "true" is meant.
+	 */
+	if (def->arg == NULL)
+		return COPY_HEADER_PRESENT;
+
+	/*
+	 * Allow 0, 1, "true", "false", "on", "off" or "match".
+	 */
+	switch (nodeTag(def->arg))
+	{
+		case T_Integer:
+			switch (intVal(def->arg))
+			{
+				case 0:
+					return COPY_HEADER_ABSENT;
+				case 1:
+					return COPY_HEADER_PRESENT;
+				default:
+					/* otherwise, error out below */
+					break;
+			}
+			break;
+		default:
+			{
+				char	   *sval = defGetString(def);
+
+				/*
+				 * The set of strings accepted here should match up with the
+				 * grammar's opt_boolean_or_string production.
+				 */
+				if (pg_strcasecmp(sval, "true") == 0)
+					return COPY_HEADER_PRESENT;
+				if (pg_strcasecmp(sval, "false") == 0)
+					return COPY_HEADER_ABSENT;
+				if (pg_strcasecmp(sval, "on") == 0)
+					return COPY_HEADER_PRESENT;
+				if (pg_strcasecmp(sval, "off") == 0)
+					return COPY_HEADER_ABSENT;
+				if (pg_strcasecmp(sval, "match") == 0)
+					return COPY_HEADER_MATCH;
+
+			}
+			break;
+	}
+	ereport(ERROR,
+			(errcode(ERRCODE_SYNTAX_ERROR),
+			 errmsg("%s requires a boolean or \"match\"",
+					def->defname)));
+	return COPY_HEADER_ABSENT;				/* keep compiler quiet */
+}
+
+/*
  * Process the statement option list for COPY.
  *
  * Scan the options list (a list of DefElem) and transpose the information
@@ -1230,7 +1298,8 @@ ProcessCopyOptions(ParseState *pstate,
 						(errcode(ERRCODE_SYNTAX_ERROR),
 						 errmsg("conflicting or redundant options"),
 						 parser_errposition(pstate, defel->location)));
-			cstate->header_line = defGetBoolean(defel);
+
+			cstate->header_line = DefGetCopyHeader(defel);
 		}
 		else if (strcmp(defel->defname, "quote") == 0)
 		{
@@ -1411,7 +1480,7 @@ ProcessCopyOptions(ParseState *pstate,
 				 errmsg("COPY delimiter cannot be \"%s\"", cstate->delim)));
 
 	/* Check header */
-	if (cstate->binary && cstate->header_line)
+	if (cstate->binary && cstate->header_line != COPY_HEADER_ABSENT)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("COPY HEADER available only in CSV and text mode")));
@@ -2132,7 +2201,7 @@ CopyTo(CopyState cstate)
 														 cstate->file_encoding);
 
 		/* if a header has been requested send the line */
-		if (cstate->header_line)
+		if (cstate->header_line != COPY_HEADER_ABSENT)
 		{
 			bool		hdr_delim = false;
 
@@ -2149,7 +2218,7 @@ CopyTo(CopyState cstate)
 
 				if (cstate->csv_mode)
 					CopyAttributeOutCSV(cstate, colname, false,
-									list_length(cstate->attnumlist) == 1);
+										list_length(cstate->attnumlist) == 1);
 				else
 					CopyAttributeOutText(cstate, colname);
 			}
@@ -3647,12 +3716,53 @@ NextCopyFromRawFields(CopyState cstate, char ***fields, int *nfields)
 	/* only available for text or csv input */
 	Assert(!cstate->binary);
 
-	/* on input just throw the header line away */
-	if (cstate->cur_lineno == 0 && cstate->header_line)
+	/* on input check that the header line is correct if needed */
+	if (cstate->cur_lineno == 0 && cstate->header_line != COPY_HEADER_ABSENT)
 	{
+		ListCell   *cur;
+		TupleDesc   tupDesc;
+
+		tupDesc = RelationGetDescr(cstate->rel);
+
 		cstate->cur_lineno++;
-		if (CopyReadLine(cstate))
-			return false;		/* done */
+		done = CopyReadLine(cstate);
+
+		if (cstate->header_line == COPY_HEADER_MATCH)
+		{
+			if (cstate->csv_mode)
+				fldct = CopyReadAttributesCSV(cstate);
+			else
+				fldct = CopyReadAttributesText(cstate);
+
+			if (fldct < list_length(cstate->attnumlist))
+				ereport(ERROR,
+						(errcode(ERRCODE_BAD_COPY_FILE_FORMAT),
+						 errmsg("missing header")));
+			else if (fldct > list_length(cstate->attnumlist))
+				ereport(ERROR,
+					(errcode(ERRCODE_BAD_COPY_FILE_FORMAT),
+					 errmsg("extra data after last expected header")));
+
+			foreach(cur, cstate->attnumlist)
+			{
+				int				attnum = lfirst_int(cur);
+				char		  *colName = cstate->raw_fields[attnum - 1];
+				Form_pg_attribute attr = TupleDescAttr(tupDesc, attnum - 1);
+
+				if (colName == NULL)
+					colName = cstate->null_print;
+
+				if (namestrcmp(&attr->attname, colName) != 0) {
+					ereport(ERROR,
+						(errcode(ERRCODE_BAD_COPY_FILE_FORMAT),
+						 errmsg("wrong header for column \"%s\": got \"%s\"",
+								NameStr(attr->attname), colName)));
+				}
+			}
+		}
+
+		if (done)
+			return false;
 	}
 
 	cstate->cur_lineno++;
diff --git a/src/test/regress/input/copy.source b/src/test/regress/input/copy.source
index 2368649..4d21c7d 100644
--- a/src/test/regress/input/copy.source
+++ b/src/test/regress/input/copy.source
@@ -213,3 +213,28 @@ select * from parted_copytest where b = 1;
 select * from parted_copytest where b = 2;
 
 drop table parted_copytest;
+
+-- Test header matching feature
+create table header_copytest (
+	a int,
+	b int,
+	c text
+);
+copy header_copytest from stdin with (header wrong_choice);
+copy header_copytest from stdin with (header match);
+a	b	c
+1	2	foo
+\.
+copy header_copytest from stdin with (header match);
+a	b
+1	2
+\.
+copy header_copytest from stdin with (header match);
+a	b	c	d
+1	2	foo	bar
+\.
+copy header_copytest from stdin with (header match, format csv);
+a,b,c
+1,2,foo
+\.
+drop table header_copytest;
diff --git a/src/test/regress/output/copy.source b/src/test/regress/output/copy.source
index c1f7f99..b792181 100644
--- a/src/test/regress/output/copy.source
+++ b/src/test/regress/output/copy.source
@@ -173,3 +173,20 @@ select * from parted_copytest where b = 2;
 (1 row)
 
 drop table parted_copytest;
+-- Test header matching feature
+create table header_copytest (
+	a int,
+	b int,
+	c text
+);
+copy header_copytest from stdin with (header wrong_choice);
+ERROR:  header requires a boolean or "match"
+copy header_copytest from stdin with (header match);
+copy header_copytest from stdin with (header match);
+ERROR:  missing header
+CONTEXT:  COPY header_copytest, line 1: "a	b"
+copy header_copytest from stdin with (header match);
+ERROR:  extra data after last expected header
+CONTEXT:  COPY header_copytest, line 1: "a	b	c	d"
+copy header_copytest from stdin with (header match, format csv);
+drop table header_copytest;
-- 
1.8.3.1

---

From: Rémi Lapeyre <[email-masked]>
Date: Thu, 27 Aug 2020 16:53:11 +0200

Thanks Daniel for the review and Vignesh for addressing the comments.

I have two remarks with the state of the current patches:
- DefGetCopyHeader() duplicates a lot of code from defGetBoolean(), should we refactor this so that they can share more of their internals? In the current implementation any change to defGetBoolean() should be made to DefGetCopyHeader() too or their behaviour will subtly differ.
- It is possible to set the header option multiple time:
     \copy x(i) from file.txt with (format csv, header off, header on);
  In which case the last one is the one kept. I think this is a bug and it should be fixed, but this is already the behaviour in the current implementation so fixing it would not be backward compatible. Do you think users should not do this and I can fix it or that keeping the current behaviour is better for backward compatibility?

Regards,
Rémi

> Le 17 août 2020 à 14:49, vignesh C <[email-masked]> a écrit :
> 
> <v5-0001-Add-header-support-to-COPY-TO-text-format.patch><v5-0002-Add-header-matching-mode-to-COPY-FROM.patch>

---

From: Michael Paquier <[email-masked]>
Date: Tue, 29 Sep 2020 15:36:24 +0900

On Thu, Aug 27, 2020 at 04:53:11PM +0200, Rémi Lapeyre wrote:
> - DefGetCopyHeader() duplicates a lot of code from defGetBoolean(),
> should we refactor this so that they can share more of their
> internals? In the current implementation any change to
> defGetBoolean() should be made to DefGetCopyHeader() too or their
> behaviour will subtly differ.

The difference comes from the use of "match", and my take would be
here that it is wrong to assume that header can be a boolean-like
parameter with only one exception.  It seems to me that we may
actually be looking at having this stuff as an option different than
"header" at the end to have clear semantics.

>   In which case the last one is the one kept. I think this is a bug
> and it should be fixed, but this is already the behaviour in the
> current implementation so fixing it would not be backward
> compatible. Do you think users should not do this and I can fix it
> or that keeping the current behaviour is better for backward
> compatibility? 

I would agree that this is a bug because we are failing to detect
what's actually a redundant option here as the first option still
causes the flag to be set to false, but that's not something worth a
back-patch IMO.  What we are looking here is something similar
to what is done with "format", where we track if the option has been
specified with format_specified.  The same is actually true with the
"freeze" option here, and it is true that we tend to prefer error-ing
in such cases while there are exceptions like EXPLAIN.  I think that
it would be nicer to be at least consistent with the behavior that
each command has chosen, and COPY is now a mixed bag.

I have marked the patch as returned with feedback for now.
--
Michael

---

From: Rémi Lapeyre <[email-masked]>
Date: Sat, 3 Oct 2020 23:42:52 +0200

> each command has chosen, and COPY is now a mixed bag.

Here’s a new version of the patches that report an error when the options are set multiple time.

Regards,
Rémi

---

From: Michael Paquier <[email-masked]>
Date: Mon, 5 Oct 2020 10:05:54 +0900

> Here’s a new version of the patches that report an error when the options are set multiple time.

Please note that I have applied a fix for the redundant option
handling as of 10c5291, though I have missed that you sent a patch.
Sorry about that.  Looking at it, we have done the same thing
byte-by-byte except that I have added tests for all option
combinations.
--
Michael

---

From: Rémi Lapeyre <[email-masked]>
Date: Tue, 13 Oct 2020 14:49:07 +0200

Thanks Michael for taking care of that!

Here’s the rebased patches with the last one dropped.

Regards,
Rémi

> Le 5 oct. 2020 à 03:05, Michael Paquier <[email-masked]> a écrit :
>

---

From: Rémi Lapeyre <[email-masked]>
Date: Wed, 21 Oct 2020 19:31:30 +0200

It looks like this is not in the current commitfest and that Cabot does not find it. I’m not yet accustomed to the PostgreSQL workflow, should I just create a new entry in the current commitfest?

Regards,
Rémi

> Le 13 oct. 2020 à 14:49, Rémi Lapeyre <[email-masked]> a écrit :
> 
> <v6-0001-Add-header-support-to-COPY-TO-text-format.patch><v6-0002-Add-header-matching-mode-to-COPY-FROM.patch>
>

---

From: "Daniel Verite" <[email-masked]>
Date: Wed, 21 Oct 2020 19:49:19 +0200

Rémi Lapeyre wrote:

> It looks like this is not in the current commitfest and that Cabot does not
> find it. I’m not yet accustomed to the PostgreSQL workflow, should I just
> create a new entry in the current commitfest?

Yes. Because in the last CommitFest it was marked
as "Returned with feedback"
https://commitfest.postgresql.org/29/2504/

Best regards,

---

From: Rémi Lapeyre <[email-masked]>
Date: Tue, 8 Dec 2020 00:40:24 +0100

Hi, here’s a rebased version of the patch.

Best regards,
Rémi

> On 21 Oct 2020, at 19:49, Daniel Verite <[email-masked]> wrote:
> 
> 	Rémi Lapeyre wrote:
>

---

From: David Steele <[email-masked]>
Date: Wed, 10 Mar 2021 12:13:05 -0500

> Hi, here’s a rebased version of the patch.

Michael, since the issue of duplicated options has been fixed do either 
of these patches look like they are ready for commit?

Regards,
-- 
-David
[email-masked]

---

From: Rémi Lapeyre <[email-masked]>
Date: Sun, 11 Apr 2021 01:17:03 +0200

> 
> Michael, since the issue of duplicated options has been fixed do either of these patches look like they are ready for commit?
> 

Here’s a rebased version of the patch.

Cheers,
Rémi

---

From: Zhihong Yu <[email-masked]>
Date: Sat, 10 Apr 2021 19:22:39 -0700

On Sat, Apr 10, 2021 at 4:17 PM Rémi Lapeyre <[email-masked]>
wrote:

> Hi,

Typo: immediately

nit: since header is singular, you can name the table header_doesnt_match

+      from the one expected, or the name or case do not match, the copy
will

For 'the name or case do not match', either use plural for the subjects or
change 'do' to doesn't

-           opts_out->header_line = defGetBoolean(defel);
+           opts_out->header_line = DefGetCopyHeader(defel);

Existing method starts with lower case d, I wonder why the new method
starts with upper case D.

The message seems to be inaccurate: the header may be there - it just
misses some fields.

present and match: it seems present is redundant - if header is absent, how
can it match ?

Cheers

---

From: Rémi Lapeyre <[email-masked]>
Date: Sun, 11 Apr 2021 13:01:22 +0200

> For 'the name or case do not match', either use plural for the subjects or change 'do' to doesn't
> 

Thanks, I fixed both typos.

> Existing method starts with lower case d, I wonder why the new method starts with upper case D.
> 

I don’t remember why I used DefGetCopyHeader, should I change it?

> The message seems to be inaccurate: the header may be there - it just misses some fields.

I changed the error messages, they now are:
    ERROR:  incomplete header, expected 3 columns but got 2
    ERROR:  extra data after last expected header, expected 3 columns but got 4

> present and match: it seems present is redundant - if header is absent, how can it match ?

This now reads "Represents whether the header must be absent, present or match.”.

Cheers,
Rémi

---

From: Zhihong Yu <[email-masked]>
Date: Sun, 11 Apr 2021 06:28:21 -0700

> >> This now reads "Represents whether the header must be absent, present
or match.”.

Since match shouldn't be preceded with be, I think we can say:

Represents whether the header must match, be absent or be present.

Cheers

---

From: Rémi Lapeyre <[email-masked]>
Date: Sun, 11 Apr 2021 16:35:12 +0200

> Represents whether the header must match, be absent or be present.

Thanks, here’s a v10 version of the patch that fixes this.

---

From: Rémi Lapeyre <[email-masked]>
Date: Fri, 31 Dec 2021 18:36:33 +0100

Here’s an updated version of the patch that takes into account the changes in d1029bb5a2. The actual code is the same as v10 which was already marked as ready for committer.

> On 11 Apr 2021, at 16:35, Rémi Lapeyre <[email-masked]> wrote:
> 
>> 
> <v10-0001-Add-header-support-to-COPY-TO-text-format.patch><v10-0002-Add-header-matching-mode-to-COPY-FROM.patch>

---

From: Greg Stark <[email-masked]>
Date: Fri, 25 Mar 2022 00:21:07 -0400

Great to see the first of the two patches committed.

It looks like the second patch got some feedback from Peter in
February and has been marked "Waiting on author" ever since.

Remi, will you have a chance to look at this this month?

Peter, are these comments blocking if Remi doesn't have a chance to
work on it should I move it to the next release or could it be fixed
up and committed?

---

From: "Daniel Verite" <[email-masked]>
Date: Fri, 25 Mar 2022 13:02:33 +0100

Peter Eisentraut wrote:

> - The DefGetCopyHeader() function seems very bulky and might not be 
> necessary.  I think you can just check for the string "match" first and 
> then use defGetBoolean() as before if it didn't match.

The problem is that defGetBoolean() ends like this in the non-matching
case:

	ereport(ERROR,
		(errcode(ERRCODE_SYNTAX_ERROR),
		 errmsg("%s requires a Boolean value",
		def->defname)));

We don't want this error message when the string does not match
since it's really a tri-state option, not a boolean.

To avoid duplicating the code that recognizes true/false/on/off/0/1,
probably defGetBoolean()'s guts should be moved into another function
that does the matching and report to the caller instead of throwing an
error. Then DefGetCopyHeader() could call that non-throwing function.

> - If you define COPY_HEADER_ABSENT = 0 in the enum, then most of the 
> existing truth checks don't need to be changed.

It's already 0 by default. Not changing some truth checks does work, but
then we get some code that treat CopyFromState.header_line like
a boolean and some other code like an enum, which I found not
ideal wrt clarity in an earlier round of review [1]

It's not a major issue though, as it  concerns only 3 lines of code in the
v12
patch:
-	if (opts_out->binary && opts_out->header_line)
+	if (opts_out->binary && opts_out->header_line != COPY_HEADER_ABSENT)

+	/* on input check that the header line is correct if needed */
+	if (cstate->cur_lineno == 0 && cstate->opts.header_line !=
COPY_HEADER_ABSENT)

-		if (cstate->opts.header_line)
+		if (cstate->opts.header_line != COPY_HEADER_ABSENT)

> - In NextCopyFromRawFields(), it looks like you have code that replaces 
> the null_print value if the supplied column name is null.  I don't think 
> we should allow null column values.  Someone, this should be an error. 

+1

[1]
https://www.postgresql.org/message-id/80a9b594-01d6-4ee4-a612-9ae9fd3c1194%40manitou-mail.org

Best regards,
-- 
Daniel Vérité
https://postgresql.verite.pro/
Twitter: @DanielVerite

---

From: Peter Eisentraut <[email-masked]>
Date: Tue, 29 Mar 2022 17:02:04 +0200

> work on it should I move it to the next release or could it be fixed

I will try to finish up this patch.

---

From: Peter Eisentraut <[email-masked]>
Date: Wed, 30 Mar 2022 09:11:09 +0200

> I will try to finish up this patch.

Committed, after some further refinements as discussed.

---

From: Julien Rouhaud <[email-masked]>
Date: Tue, 7 Jun 2022 23:47:44 +0800

Hi,

> Committed, after some further refinements as discussed.

While working on nearby code, I found some problems with this feature.

First, probably nitpicking, the HEADER MATCH is allowed for COPY TO, is that
expected?  The documentation isn't really explicit about it, but there's
nothing to match when exporting data it's a bit surprising.  I'm not opposed to
have HEADER MATCH means HEADER ON for COPY TO, as as-is one can easily reuse
the commands history, but maybe it should be clearly documented?

Then, apparently HEADER MATCH doesn't let you do sanity checks against a custom
column list.  This one looks like a clear oversight, as something like that
should be entirely valid IMHO:

CREATE TABLE tbl(col1 int, col2 int);
COPY tbl (col2, col1) TO '/path/to/file' WITH (HEADER MATCH);
COPY tbl (col2, col1) FROM '/path/to/file' WITH (HEADER MATCH);

but right now it errors out with:

ERROR:  column name mismatch in header line field 1: got "col1", expected "col2"

Note that the error message is bogus if you specify attributes in a
different order from the relation, as the code is mixing access to the tuple
desc and access to the raw fields with the same offset.

This also means that it will actually fail to detect a mismatch in the provided
column list and let you import data in the wrong position as long as the
datatypes are compatible and the column header in the file are in the correct
order.  For instance:

CREATE TABLE abc (a text, b text, c text);
INSERT INTO abc SELECT 'a', 'b', 'c';
COPY abc TO '/path/to/file' WITH (HEADER MATCH);

You can then import the data with any of those:
COPY abc(c, b, a) TO '/path/to/file' WITH (HEADER MATCH);
COPY abc(c, a, b) TO '/path/to/file' WITH (HEADER MATCH);
[...]
SELECT * FROM abc;

Even worse, if you try to do a COPY ... FROM ... WITH (HEADER ON) on a table
that has some dropped attribute(s).  The current code will access random memory
as there's no exact attnum / raw field mapping anymore.

I can work on a fix if needed (with some additional regression test to cover
those cases), but I'm still not sure that having a user provided column list is
supposed to be accepted or not for the HEADER MATCH.  In the meantime I will
add an open item.

---

From: Andrew Dunstan <[email-masked]>
Date: Sun, 12 Jun 2022 09:36:13 -0400

> the commands history, but maybe it should be clearly documented?

I think it makes more sense to have a sanity check to prevent HEADER
MATCH with COPY TO.

> as there's no exact attnum / raw field mapping anymore.

Ouch! That certainly needs to be fixed.

> supposed to be accepted or not for the HEADER MATCH.  In the meantime I will

I think it should, but a temporary alternative would be to forbid HEADER
MATCH with explicit column lists until we can make it work right.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

---

From: Julien Rouhaud <[email-masked]>
Date: Mon, 13 Jun 2022 10:32:13 +0800

Hi,

> I think it makes more sense to have a sanity check to prevent HEADER

I'm fine with it.  I added such a check and mentioned it in the documentation.

> MATCH with explicit column lists until we can make it work right.

I think it would still be problematic if the target table has dropped columns.
Fortunately, as I initially thought the problem is only due to a thinko in the
original commit which used a wrong variable for the raw_fields offset.  Once
fixed (attached v1) I didn't see any other problem in the rest of the logic and
all the added regression tests work as expected.

---

From: Peter Eisentraut <[email-masked]>
Date: Mon, 13 Jun 2022 09:35:16 +0200

> all the added regression tests work as expected.

Thanks for this patch.  I'll check it in detail in a bit.  It looks good 
to me at first glance.

---

From: Michael Paquier <[email-masked]>
Date: Mon, 13 Jun 2022 16:46:46 +0900

> I'm fine with it.  I added such a check and mentioned it in the documentation.

An error looks like the right call at this stage of the game.  I am
not sure what the combination of MATCH with COPY TO would mean,
actually.  And with the concept of SELECT queries on top of it, the
whole idea gets blurrier.

> all the added regression tests work as expected.

Interesting catch.  One thing that I've always found useful when it
comes to tests that stress dropped columns is to have tests where we
reduce the number of total columns that still exist.  An extra thing
is to look after ........pg.dropped.N........ a bit, and I would put
something in one of the headers.

>  	if (pg_strcasecmp(sval, "match") == 0)
> +	{
> +		/* match is only valid for COPY FROM */
> +		if (!is_from)
> +			ereport(ERROR,
> +			 errmsg("%s match is only valid for COPY FROM",
> +					def->defname)));

Some nits.  I would suggest to reword this error message, like "cannot
use \"match\" with HEADER in COPY TO".  There is no need for the extra
comment, and the error code should be ERRCODE_FEATURE_NOT_SUPPORTED.
--
Michael
From af4e06ca5f0a13ad5922abae446bd6716bbdf3c1 Mon Sep 17 00:00:00 2001
From: Julien Rouhaud <[email-masked]>
Date: Mon, 13 Jun 2022 09:49:23 +0800
Subject: [PATCH v2] 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
---
 src/backend/commands/copy.c          | 11 +++++--
 src/backend/commands/copyfromparse.c |  5 ++--
 src/test/regress/expected/copy.out   | 43 ++++++++++++++++++++++++++-
 src/test/regress/sql/copy.sql        | 44 ++++++++++++++++++++++++++--
 doc/src/sgml/ref/copy.sgml           |  2 ++
 5 files changed, 97 insertions(+), 8 deletions(-)

diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index f448d39c7e..e2870e3c11 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,14 @@ defGetCopyHeaderChoice(DefElem *def)
 				if (pg_strcasecmp(sval, "off") == 0)
 					return COPY_HEADER_FALSE;
 				if (pg_strcasecmp(sval, "match") == 0)
+				{
+					if (!is_from)
+						ereport(ERROR,
+								(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+								 errmsg("cannot use \"%s\" with HEADER in COPY TO",
+										sval)));
 					return COPY_HEADER_MATCH;
+				}
 			}
 			break;
 	}
@@ -452,7 +459,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..7f2f4ae4ae 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:  cannot use "match" with HEADER in COPY TO
 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);
+-- errors
+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,34 @@ 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 an extra column, in the middle of the existing set.
+alter table header_copytest drop column b;
+-- works
+copy header_copytest (c, a) from stdin with (header match);
+copy header_copytest (a, c) from stdin with (header match);
+-- errors
+copy header_copytest from stdin with (header match);
+ERROR:  wrong number of fields in header line: field count is 3, expected 2
+CONTEXT:  COPY header_copytest, line 1: "a	........pg.dropped.2........	c"
+copy header_copytest (a, c) from stdin with (header match);
+ERROR:  wrong number of fields in header line: field count is 3, expected 2
+CONTEXT:  COPY header_copytest, line 1: "a	c	b"
+SELECT * FROM header_copytest ORDER BY a;
+ a |  c  
+---+-----
+ 1 | foo
+ 3 | bar
+ 5 | baz
+ 7 | foo
+ 8 | foo
+(5 rows)
+
 drop table header_copytest;
diff --git a/src/test/regress/sql/copy.sql b/src/test/regress/sql/copy.sql
index d72d226f34..285022e07c 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
+\.
+-- errors
+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,28 @@ 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 an extra column, in the middle of the existing set.
+alter table header_copytest drop column b;
+-- works
+copy header_copytest (c, a) from stdin with (header match);
+c	a
+foo	7
 \.
+copy header_copytest (a, c) from stdin with (header match);
+a	c
+8	foo
+\.
+-- errors
+copy header_copytest from stdin with (header match);
+a	........pg.dropped.2........	c
+1	2	foo
+\.
+copy header_copytest (a, c) from stdin with (header match);
+a	c	b
+1	foo	2
+\.
+
+SELECT * FROM header_copytest ORDER BY a;
 drop table header_copytest;
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>
-- 
2.36.1

---

From: Julien Rouhaud <[email-masked]>
Date: Tue, 14 Jun 2022 17:13:19 +0800

On Mon, Jun 13, 2022 at 04:46:46PM +0900, Michael Paquier wrote:
> use \"match\" with HEADER in COPY TO".

Agreed.

> There is no need for the extra

Is there any rule for what error code should be used?

Maybe that's just me but I understand "not supported" as "this makes sense, but
this is currently a limitation that might be lifted later".

Here I don't think it can ever make to use MATCH for a COPY TO, apart from
ignoring its meaning and accept it as an alias for HEADER ON.  But if we don't
allow this loose alias now it would just cause trouble to later allow it so
having an invalid syntax or something like that sounds more suited.

---

From: Peter Eisentraut <[email-masked]>
Date: Wed, 15 Jun 2022 11:14:33 +0200

> this is currently a limitation that might be lifted later".

I tend to agree with that interpretation.

Also, when you consider the way SQL rules and error codes are set up, 
errors that are detected during parse analysis should be a subclass of 
"syntax error or access rule violation".

---

From: "Daniel Verite" <[email-masked]>
Date: Wed, 15 Jun 2022 13:50:14 +0200

Julien Rouhaud wrote:

> Maybe that's just me but I understand "not supported" as "this makes
> sense, but this is currently a limitation that might be lifted
> later".
 
Looking at ProcessCopyOptions(), there are quite a few invalid
combinations of options that produce
ERRCODE_FEATURE_NOT_SUPPORTED currently:

- HEADER in binary mode
- FORCE_QUOTE outside of csv
- FORCE_QUOTE outside of COPY TO
- FORCE_NOT_NULL outside of csv
- FORCE_NOT_NULL outside of COPY FROM
- ESCAPE outside of csv
- delimiter appearing in the NULL specification
- csv quote appearing in the NULL specification

FORCE_QUOTE and FORCE_NOT_NULL are options that only make sense in one
direction, so the errors when using these in the wrong direction are
comparable to the "HEADER MATCH outside of COPY FROM" error that we
want to add. In that sense, ERRCODE_FEATURE_NOT_SUPPORTED would be
consistent.

The other errors in the list above are more about the format itself,
with options that make sense only for one format. So the way we're
supposed to understand ERRCODE_FEATURE_NOT_SUPPORTED in these
other cases is that such format does not support such feature,
but without implying that it's a limitation.

Best regards,

---

From: Peter Eisentraut <[email-masked]>
Date: Thu, 16 Jun 2022 09:24:56 +0200

> but without implying that it's a limitation.

I don't feel very strongly about this.  It makes sense to stay 
consistent with the existing COPY code.

---

From: Michael Paquier <[email-masked]>
Date: Mon, 20 Jun 2022 09:03:23 +0900

On Thu, Jun 16, 2022 at 09:24:56AM +0200, Peter Eisentraut wrote:
> I don't feel very strongly about this.  It makes sense to stay consistent
> with the existing COPY code.

Yes, my previous argument is based on consistency with the
surroundings.  I am not saying that this could not be made better, it
surely can, but I would recommend to tackle that in a separate patch,
and apply that to more areas than this specific one.
--
Michael

---

From: Michael Paquier <[email-masked]>
Date: Wed, 22 Jun 2022 08:34:34 +0900

> and apply that to more areas than this specific one.

Peter, beta2 is planned for next week.  Do you think that you would be
able to address this open item by the end of this week?  If not, and I
have already looked at the proposed patch, I can jump in and help.
--
Michael

---

From: Peter Eisentraut <[email-masked]>
Date: Wed, 22 Jun 2022 12:22:01 +0200

> have already looked at the proposed patch, I can jump in and help.

The latest patch was posted by you, so I was deferring to you to commit 
it.  Would you like me to do it?

---

From: Michael Paquier <[email-masked]>
Date: Wed, 22 Jun 2022 20:00:15 +0900

On Wed, Jun 22, 2022 at 12:22:01PM +0200, Peter Eisentraut wrote:
> The latest patch was posted by you, so I was deferring to you to commit it.
> Would you like me to do it?

OK.  As this is originally a feature you have committed, I originally
thought that you would take care of it, even if I sent a patch.  I'll
handle that tomorrow then, if that's fine for you, of course.  Happy
to help.
--
Michael

---

From: Michael Paquier <[email-masked]>
Date: Thu, 23 Jun 2022 13:26:29 +0900

> handle that tomorrow then, if that's fine for you, of course.  Happy

And done.  Thanks.
--
Michael

---

From: Julien Rouhaud <[email-masked]>
Date: Thu, 23 Jun 2022 12:46:46 +0800

> > handle that tomorrow then, if that's fine for you, of course.  Happy

Thanks!