public inbox for [email protected]  
help / color / mirror / Atom feed
From: Rémi Lapeyre <[email protected]>
To: Michael Paquier <[email protected]>
Cc: vignesh C <[email protected]>
Cc: Daniel Verite <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Subject: Re: Add header support to text format and matching feature
Date: Sat, 3 Oct 2020 23:42:52 +0200
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<[email protected]>
	<CALDaNm07bcwPY2W68Qghvw2QAV9X8NkV6pihGicko5om=txF+Q@mail.gmail.com>
	<[email protected]>
	<[email protected]>


> 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.

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

Regards,
Rémi



Attachments:

  [application/octet-stream] v5-0001-Add-header-support-to-COPY-TO-text-format.patch (7.0K, ../[email protected]/2-v5-0001-Add-header-support-to-COPY-TO-text-format.patch)
  download | inline diff:
From ae87e1cc7b71ae3c742337dbb5ca2960bc2b2b39 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9mi=20Lapeyre?= <[email protected]>
Date: Fri, 17 Jul 2020 01:50:06 +0200
Subject: [PATCH v5 1/3] Add header support to "COPY TO" text format
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="------------2.28.0"

This is a multi-part message in MIME format.
--------------2.28.0
Content-Type: text/plain; charset=UTF-8; format=fixed
Content-Transfer-Encoding: 8bit


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/CAF1-J-0PtCWMeLtswwGV2M70U26n4g33gpe1rcKQqe6wVQDrFA@mail.gmail.com
---
 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(-)


--------------2.28.0
Content-Type: text/x-patch; name="v5-0001-Add-header-support-to-COPY-TO-text-format.patch"
Content-Transfer-Encoding: 8bit
Content-Disposition: attachment; filename="v5-0001-Add-header-support-to-COPY-TO-text-format.patch"

diff --git a/contrib/file_fdw/input/file_fdw.source b/contrib/file_fdw/input/file_fdw.source
index 45b728eeb3..83edb71077 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 52b4d5f1df..547b81fd16 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 369342b74d..fcab594f09 100644
--- a/doc/src/sgml/ref/copy.sgml
+++ b/doc/src/sgml/ref/copy.sgml
@@ -271,7 +271,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 2047557e52..414940e880 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 a1d529ad36..2368649111 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 938d3551da..c1f7f99747 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,

--------------2.28.0--




  [application/octet-stream] v5-0002-Add-header-matching-mode-to-COPY-FROM.patch (13.3K, ../[email protected]/3-v5-0002-Add-header-matching-mode-to-COPY-FROM.patch)
  download | inline diff:
From 2618b9f83edbc3c6266a8cf6eddd7cee3589b958 Mon Sep 17 00:00:00 2001
From: Vignesh C <[email protected]>
Date: Mon, 17 Aug 2020 17:48:09 +0530
Subject: [PATCH v5 2/3] Add header matching mode to "COPY FROM"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="------------2.28.0"

This is a multi-part message in MIME format.
--------------2.28.0
Content-Type: text/plain; charset=UTF-8; format=fixed
Content-Transfer-Encoding: 8bit


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/CAF1-J-0PtCWMeLtswwGV2M70U26n4g33gpe1rcKQqe6wVQDrFA@mail.gmail.com
---
 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(-)


--------------2.28.0
Content-Type: text/x-patch; name="v5-0002-Add-header-matching-mode-to-COPY-FROM.patch"
Content-Transfer-Encoding: 8bit
Content-Disposition: attachment; filename="v5-0002-Add-header-matching-mode-to-COPY-FROM.patch"

diff --git a/contrib/file_fdw/input/file_fdw.source b/contrib/file_fdw/input/file_fdw.source
index 83edb71077..7a3983c785 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 547b81fd16..ebe826b9f4 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 fcab594f09..a804d0c35b 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> [, ...] ) | * }
@@ -270,9 +270,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 414940e880..97e8514b51 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -95,6 +95,16 @@ typedef enum CopyInsertMethod
 	CIM_MULTI_CONDITIONAL		/* use table_multi_insert only if valid */
 } 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,
@@ -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 */
@@ -1135,6 +1145,64 @@ DoCopy(ParseState *pstate, const CopyStmt *stmt,
 		table_close(rel, NoLock);
 }
 
+/*
+ * 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.
  *
@@ -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 2368649111..4d21c7d524 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 c1f7f99747..b792181fe3 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;

--------------2.28.0--




  [application/octet-stream] v5-0003-Report-an-error-when-options-are-set-multiple-tim.patch (4.4K, ../[email protected]/4-v5-0003-Report-an-error-when-options-are-set-multiple-tim.patch)
  download | inline diff:
From 9e6fd40a83f24bdc9d23f95af21e8c8bed57303e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9mi=20Lapeyre?= <[email protected]>
Date: Sat, 3 Oct 2020 23:13:44 +0200
Subject: [PATCH v5 3/3] Report an error when options are set multiple times in
 COPY
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="------------2.28.0"

This is a multi-part message in MIME format.
--------------2.28.0
Content-Type: text/plain; charset=UTF-8; format=fixed
Content-Transfer-Encoding: 8bit


Discussion: https://www.postgresql.org/message-id/flat/CAF1-J-0PtCWMeLtswwGV2M70U26n4g33gpe1rcKQqe6wVQDrFA@mail.gmail.com
---
 src/backend/commands/copy.c         |  9 +++++++--
 src/test/regress/input/copy.source  |  6 ++++++
 src/test/regress/output/copy.source | 10 ++++++++++
 3 files changed, 23 insertions(+), 2 deletions(-)


--------------2.28.0
Content-Type: text/x-patch; name="v5-0003-Report-an-error-when-options-are-set-multiple-tim.patch"
Content-Transfer-Encoding: 8bit
Content-Disposition: attachment; filename="v5-0003-Report-an-error-when-options-are-set-multiple-tim.patch"

diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 97e8514b51..dd8c43da28 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -1227,6 +1227,8 @@ ProcessCopyOptions(ParseState *pstate,
 				   List *options)
 {
 	bool		format_specified = false;
+	bool		header_specified = false;
+	bool		freeze_specified = false;
 	ListCell   *option;
 
 	/* Support external use for option sanity checking */
@@ -1266,11 +1268,13 @@ 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)
@@ -1293,12 +1297,13 @@ 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 = DefGetCopyHeader(defel);
 		}
 		else if (strcmp(defel->defname, "quote") == 0)
diff --git a/src/test/regress/input/copy.source b/src/test/regress/input/copy.source
index 4d21c7d524..bb6cea519f 100644
--- a/src/test/regress/input/copy.source
+++ b/src/test/regress/input/copy.source
@@ -146,6 +146,9 @@ this is just a line full of junk that would error out if parsed
 
 copy copytest4 to stdout (header);
 
+-- specifying header multiple times should report an error
+copy copytest4 to stdout (header off, header on);
+
 -- test copy from with a partitioned table
 create table parted_copytest (
 	a int,
@@ -182,6 +185,9 @@ group by tableoid order by tableoid::regclass::name;
 
 truncate parted_copytest;
 
+-- specifying freeze multiple times should report an error
+copy copytest4 to stdout (freeze off, freeze on);
+
 -- create before insert row trigger on parted_copytest_a2
 create function part_ins_func() returns trigger language plpgsql as $$
 begin
diff --git a/src/test/regress/output/copy.source b/src/test/regress/output/copy.source
index b792181fe3..b6652de074 100644
--- a/src/test/regress/output/copy.source
+++ b/src/test/regress/output/copy.source
@@ -103,6 +103,11 @@ copy copytest4 to stdout (header);
 c1	col with tabulation: \t
 1	a
 2	b
+-- specifying header multiple times should report an error
+copy copytest4 to stdout (header off, header on);
+ERROR:  conflicting or redundant options
+LINE 1: copy copytest4 to stdout (header off, header on);
+                                              ^
 -- test copy from with a partitioned table
 create table parted_copytest (
 	a int,
@@ -136,6 +141,11 @@ group by tableoid order by tableoid::regclass::name;
 (2 rows)
 
 truncate parted_copytest;
+-- specifying freeze multiple times should report an error
+copy copytest4 to stdout (freeze off, freeze on);
+ERROR:  conflicting or redundant options
+LINE 1: copy copytest4 to stdout (freeze off, freeze on);
+                                              ^
 -- create before insert row trigger on parted_copytest_a2
 create function part_ins_func() returns trigger language plpgsql as $$
 begin

--------------2.28.0--




view thread (41+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Add header support to text format and matching feature
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox