COPY (INSERT/UPDATE/DELETE .. RETURNING ..)

Started by Marko Tiikkajaabout 10 years ago12 messages

marko@joh.to

about 10 years ago

1 attachment(s)

Hi,

Attached is a patch for being able to do $SUBJECT without a CTE. The
reasons this is better than a CTE version are:

1) It's not obvious why a CTE version works but a plain one doesn't
2) This one has less overhead (I measured a ~12% improvement on a
not-too-unreasonable test case)

With regard to RULEs, similar restrictions apply as the ones on
data-modifying statements in WITH.

I can't add this to November's commit fest, but I'm posting this anyway
in case someone is thinking about implementing this feature.

Attachments:

copy_dml_v1.patchtext/x-patch; name=copy_dml_v1.patchDownload

diff --git a/doc/src/sgml/ref/copy.sgml b/doc/src/sgml/ref/copy.sgml
index 2850b47..07e2f45 100644
--- a/doc/src/sgml/ref/copy.sgml
+++ b/doc/src/sgml/ref/copy.sgml
@@ -112,10 +112,17 @@ COPY { <replaceable class="parameter">table_name</replaceable> [ ( <replaceable
     <term><replaceable class="parameter">query</replaceable></term>
     <listitem>
      <para>
-      A <xref linkend="sql-select"> or
-      <xref linkend="sql-values"> command
-      whose results are to be copied.
-      Note that parentheses are required around the query.
+      A <xref linkend="sql-select">, <xref linkend="sql-values">,
+      <xref linkend="sql-insert">, <xref linkend="sql-update"> or
+      <xref linkend="sql-delete"> command whose results are to be
+      copied.  Note that parentheses are required around the query.
+     </para>
+     <para>
+      For <command>INSERT</>, <command>UPDATE</> and
+      <command>DELETE</> queries a RETURNING clause must be provided,
+      and the target relation must not have a conditional rule, nor
+      an <literal>ALSO</> rule, nor an <literal>INSTEAD</> rule
+      that expands to multiple statements.
      </para>
     </listitem>
    </varlistentry>
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 9a52ec6..eb2f126 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -201,7 +201,7 @@ typedef struct CopyStateData
 	int			raw_buf_len;	/* total # of bytes stored */
 } CopyStateData;
 
-/* DestReceiver for COPY (SELECT) TO */
+/* DestReceiver for COPY (query) TO */
 typedef struct
 {
 	DestReceiver pub;			/* publicly-known function pointers */
@@ -772,7 +772,8 @@ CopyLoadRawBuf(CopyState cstate)
  *
  * Either unload or reload contents of table <relation>, depending on <from>.
  * (<from> = TRUE means we are inserting into the table.)  In the "TO" case
- * we also support copying the output of an arbitrary SELECT query.
+ * we also support copying the output of an arbitrary SELECT, INSERT, UPDATE
+ * or DELETE query.
  *
  * If <pipe> is false, transfer is between the table and the file named
  * <filename>.  Otherwise, transfer is between the table and our regular
@@ -1374,11 +1375,11 @@ BeginCopy(bool is_from,
 		Assert(!is_from);
 		cstate->rel = NULL;
 
-		/* Don't allow COPY w/ OIDs from a select */
+		/* Don't allow COPY w/ OIDs from a query */
 		if (cstate->oids)
 			ereport(ERROR,
 					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-					 errmsg("COPY (SELECT) WITH OIDS is not supported")));
+					 errmsg("COPY (query) WITH OIDS is not supported")));
 
 		/*
 		 * Run parse analysis and rewrite.  Note this also acquires sufficient
@@ -1393,9 +1394,36 @@ BeginCopy(bool is_from,
 		rewritten = pg_analyze_and_rewrite((Node *) copyObject(raw_query),
 										   queryString, NULL, 0);
 
-		/* We don't expect more or less than one result query */
-		if (list_length(rewritten) != 1)
-			elog(ERROR, "unexpected rewrite result");
+		/* check that we got back something we can work with */
+		if (rewritten == NIL)
+		{
+			ereport(ERROR,
+					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+					errmsg("DO INSTEAD NOTHING rules are not supported for COPY")));
+		}
+		else if (list_length(rewritten) > 1)
+		{
+			ListCell *lc;
+
+			/* examine queries to determine which error message to issue */
+			foreach(lc, rewritten)
+			{
+				Query	  *q = (Query *) lfirst(lc);
+
+				if (q->querySource == QSRC_QUAL_INSTEAD_RULE)
+					ereport(ERROR,
+							(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+							 errmsg("conditional DO INSTEAD rules are not supported for COPY")));
+				if (q->querySource == QSRC_NON_INSTEAD_RULE)
+					ereport(ERROR,
+							(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+							 errmsg("DO ALSO rules are not supported for the COPY")));
+			}
+
+			ereport(ERROR,
+					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+					 errmsg("multi-statement DO INSTEAD rules are not supported for COPY")));
+		}
 
 		query = (Query *) linitial(rewritten);
 
@@ -1406,9 +1434,20 @@ BeginCopy(bool is_from,
 					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 					 errmsg("COPY (SELECT INTO) is not supported")));
 
-		Assert(query->commandType == CMD_SELECT);
 		Assert(query->utilityStmt == NULL);
 
+		/*
+		 * Similarly the grammar doesn't enforce he presence of a RETURNING
+		 * clause, but we require it.
+		 */
+		if (query->commandType != CMD_SELECT &&
+			query->returningList == NIL)
+		{
+			ereport(ERROR,
+						(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+						 errmsg("COPY query must have a RETURNING clause")));
+		}
+
 		/* plan the query */
 		plan = pg_plan_query(query, 0, NULL);
 
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index fba91d5..d87ae93 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -2561,9 +2561,12 @@ ClosePortalStmt:
  *
  *		QUERY :
  *				COPY relname [(columnList)] FROM/TO file [WITH] [(options)]
- *				COPY ( SELECT ... ) TO file	[WITH] [(options)]
+ *				COPY ( query ) TO file	[WITH] [(options)]
  *
- *				where 'file' can be one of:
+ *				where 'query' is one of:
+ *				{ select | update | insert | delete }
+ *
+ *				and 'file' can be one of:
  *				{ PROGRAM 'command' | STDIN | STDOUT | 'filename' }
  *
  *				In the preferred syntax the options are comma-separated
@@ -2574,7 +2577,7 @@ ClosePortalStmt:
  *				COPY [ BINARY ] table [ WITH OIDS ] FROM/TO file
  *					[ [ USING ] DELIMITERS 'delimiter' ] ]
  *					[ WITH NULL AS 'null string' ]
- *				This option placement is not supported with COPY (SELECT...).
+ *				This option placement is not supported with COPY (query).
  *
  *****************************************************************************/
 
@@ -2607,16 +2610,16 @@ CopyStmt:	COPY opt_binary qualified_name opt_column_list opt_oids
 						n->options = list_concat(n->options, $11);
 					$$ = (Node *)n;
 				}
-			| COPY select_with_parens TO opt_program copy_file_name opt_with copy_options
+			| COPY '(' PreparableStmt ')' TO opt_program copy_file_name opt_with copy_options
 				{
 					CopyStmt *n = makeNode(CopyStmt);
 					n->relation = NULL;
-					n->query = $2;
+					n->query = $3;
 					n->attlist = NIL;
 					n->is_from = false;
-					n->is_program = $4;
-					n->filename = $5;
-					n->options = $7;
+					n->is_program = $6;
+					n->filename = $7;
+					n->options = $9;
 
 					if (n->is_program && n->filename == NULL)
 						ereport(ERROR,
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 9e1c48d..6b763ef 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -1680,7 +1680,7 @@ typedef struct CopyStmt
 {
 	NodeTag		type;
 	RangeVar   *relation;		/* the relation to copy */
-	Node	   *query;			/* the SELECT query to copy */
+	Node	   *query;			/* the query (SELECT or I/U/D with RETURNING) to copy */
 	List	   *attlist;		/* List of column names (as Strings), or NIL
 								 * for all columns */
 	bool		is_from;		/* TO or FROM */
diff --git a/src/test/regress/expected/copydml.out b/src/test/regress/expected/copydml.out
new file mode 100644
index 0000000..913b2f6
--- /dev/null
+++ b/src/test/regress/expected/copydml.out
@@ -0,0 +1,112 @@
+--
+-- Test cases for COPY (INSERT/UPDATE/DELETE) TO
+--
+create table test3 (id serial, t text);
+insert into test3 (t) values ('a');
+insert into test3 (t) values ('b');
+insert into test3 (t) values ('c');
+insert into test3 (t) values ('d');
+insert into test3 (t) values ('e');
+--
+-- Test COPY (insert/update/delete ...)
+--
+copy (insert into test3 (t) values ('f') returning id) to stdout;
+6
+copy (update test3 set t = 'g' where t = 'f' returning id) to stdout;
+6
+copy (delete from test3 where t = 'g' returning id) to stdout;
+6
+--
+-- Test \copy (insert/update/delete ...)
+--
+\copy (insert into test3 (t) values ('f') returning id) to stdout;
+7
+\copy (update test3 set t = 'g' where t = 'f' returning id) to stdout;
+7
+\copy (delete from test3 where t = 'g' returning id) to stdout;
+7
+-- Error cases
+copy (insert into test3 default values) to stdout;
+ERROR:  COPY query must have a RETURNING clause
+copy (update test3 set t = 'g') to stdout;
+ERROR:  COPY query must have a RETURNING clause
+copy (delete from test3) to stdout;
+ERROR:  COPY query must have a RETURNING clause
+create rule qqq as on insert to test3 do instead nothing;
+copy (insert into test3 default values) to stdout;
+ERROR:  DO INSTEAD NOTHING rules are not supported for COPY
+drop rule qqq on test3;
+create rule qqq as on insert to test3 do also delete from test3;
+copy (insert into test3 default values) to stdout;
+ERROR:  DO ALSO rules are not supported for the COPY
+drop rule qqq on test3;
+create rule qqq as on insert to test3 do instead (delete from test3; delete from test3);
+copy (insert into test3 default values) to stdout;
+ERROR:  multi-statement DO INSTEAD rules are not supported for COPY
+drop rule qqq on test3;
+create rule qqq as on insert to test3 where new.t <> 'f' do instead delete from test3;
+copy (insert into test3 default values) to stdout;
+ERROR:  conditional DO INSTEAD rules are not supported for COPY
+drop rule qqq on test3;
+create rule qqq as on update to test3 do instead nothing;
+copy (update test3 set t = 'f') to stdout;
+ERROR:  DO INSTEAD NOTHING rules are not supported for COPY
+drop rule qqq on test3;
+create rule qqq as on update to test3 do also delete from test3;
+copy (update test3 set t = 'f') to stdout;
+ERROR:  DO ALSO rules are not supported for the COPY
+drop rule qqq on test3;
+create rule qqq as on update to test3 do instead (delete from test3; delete from test3);
+copy (update test3 set t = 'f') to stdout;
+ERROR:  multi-statement DO INSTEAD rules are not supported for COPY
+drop rule qqq on test3;
+create rule qqq as on update to test3 where new.t <> 'f' do instead delete from test3;
+copy (update test3 set t = 'f') to stdout;
+ERROR:  conditional DO INSTEAD rules are not supported for COPY
+drop rule qqq on test3;
+create rule qqq as on delete to test3 do instead nothing;
+copy (delete from test3) to stdout;
+ERROR:  DO INSTEAD NOTHING rules are not supported for COPY
+drop rule qqq on test3;
+create rule qqq as on delete to test3 do also insert into test3 default values;
+copy (delete from test3) to stdout;
+ERROR:  DO ALSO rules are not supported for the COPY
+drop rule qqq on test3;
+create rule qqq as on delete to test3 do instead (insert into test3 default values; insert into test3 default values);
+copy (delete from test3) to stdout;
+ERROR:  multi-statement DO INSTEAD rules are not supported for COPY
+drop rule qqq on test3;
+create rule qqq as on delete to test3 where old.t <> 'f' do instead insert into test3 default values;
+copy (delete from test3) to stdout;
+ERROR:  conditional DO INSTEAD rules are not supported for COPY
+drop rule qqq on test3;
+-- triggers
+create function qqq_trig() returns trigger as $$
+begin
+if tg_op in ('INSERT', 'UPDATE') then
+    raise notice '% %', tg_op, new.id;
+    return new;
+else
+    raise notice '% %', tg_op, old.id;
+    return old;
+end if;
+end
+$$ language plpgsql;
+create trigger qqqbef before insert or update or delete on test3
+    for each row execute procedure qqq_trig();
+create trigger qqqaf after insert or update or delete on test3
+    for each row execute procedure qqq_trig();
+copy (insert into test3 (t) values ('f') returning id) to stdout;
+NOTICE:  INSERT 8
+8
+NOTICE:  INSERT 8
+copy (update test3 set t = 'g' where t = 'f' returning id) to stdout;
+NOTICE:  UPDATE 8
+8
+NOTICE:  UPDATE 8
+copy (delete from test3 where t = 'g' returning id) to stdout;
+NOTICE:  DELETE 8
+8
+NOTICE:  DELETE 8
+drop table test3;
+drop function qqq_trig();
diff --git a/src/test/regress/parallel_schedule b/src/test/regress/parallel_schedule
index c63abf4..6d249b5 100644
--- a/src/test/regress/parallel_schedule
+++ b/src/test/regress/parallel_schedule
@@ -48,7 +48,7 @@ test: create_function_2
 # execute two copy tests parallel, to check that copy itself
 # is concurrent safe.
 # ----------
-test: copy copyselect
+test: copy copyselect copydml
 
 # ----------
 # More groups of parallel tests
diff --git a/src/test/regress/serial_schedule b/src/test/regress/serial_schedule
index 88dcd64..295672c 100644
--- a/src/test/regress/serial_schedule
+++ b/src/test/regress/serial_schedule
@@ -57,6 +57,7 @@ test: create_table
 test: create_function_2
 test: copy
 test: copyselect
+test: copydml
 test: create_misc
 test: create_operator
 test: create_index
diff --git a/src/test/regress/sql/copydml.sql b/src/test/regress/sql/copydml.sql
new file mode 100644
index 0000000..8cff8b9
--- /dev/null
+++ b/src/test/regress/sql/copydml.sql
@@ -0,0 +1,91 @@
+--
+-- Test cases for COPY (INSERT/UPDATE/DELETE) TO
+--
+create table test3 (id serial, t text);
+insert into test3 (t) values ('a');
+insert into test3 (t) values ('b');
+insert into test3 (t) values ('c');
+insert into test3 (t) values ('d');
+insert into test3 (t) values ('e');
+
+--
+-- Test COPY (insert/update/delete ...)
+--
+copy (insert into test3 (t) values ('f') returning id) to stdout;
+copy (update test3 set t = 'g' where t = 'f' returning id) to stdout;
+copy (delete from test3 where t = 'g' returning id) to stdout;
+
+--
+-- Test \copy (insert/update/delete ...)
+--
+\copy (insert into test3 (t) values ('f') returning id) to stdout;
+\copy (update test3 set t = 'g' where t = 'f' returning id) to stdout;
+\copy (delete from test3 where t = 'g' returning id) to stdout;
+
+-- Error cases
+copy (insert into test3 default values) to stdout;
+copy (update test3 set t = 'g') to stdout;
+copy (delete from test3) to stdout;
+
+create rule qqq as on insert to test3 do instead nothing;
+copy (insert into test3 default values) to stdout;
+drop rule qqq on test3;
+create rule qqq as on insert to test3 do also delete from test3;
+copy (insert into test3 default values) to stdout;
+drop rule qqq on test3;
+create rule qqq as on insert to test3 do instead (delete from test3; delete from test3);
+copy (insert into test3 default values) to stdout;
+drop rule qqq on test3;
+create rule qqq as on insert to test3 where new.t <> 'f' do instead delete from test3;
+copy (insert into test3 default values) to stdout;
+drop rule qqq on test3;
+
+create rule qqq as on update to test3 do instead nothing;
+copy (update test3 set t = 'f') to stdout;
+drop rule qqq on test3;
+create rule qqq as on update to test3 do also delete from test3;
+copy (update test3 set t = 'f') to stdout;
+drop rule qqq on test3;
+create rule qqq as on update to test3 do instead (delete from test3; delete from test3);
+copy (update test3 set t = 'f') to stdout;
+drop rule qqq on test3;
+create rule qqq as on update to test3 where new.t <> 'f' do instead delete from test3;
+copy (update test3 set t = 'f') to stdout;
+drop rule qqq on test3;
+
+create rule qqq as on delete to test3 do instead nothing;
+copy (delete from test3) to stdout;
+drop rule qqq on test3;
+create rule qqq as on delete to test3 do also insert into test3 default values;
+copy (delete from test3) to stdout;
+drop rule qqq on test3;
+create rule qqq as on delete to test3 do instead (insert into test3 default values; insert into test3 default values);
+copy (delete from test3) to stdout;
+drop rule qqq on test3;
+create rule qqq as on delete to test3 where old.t <> 'f' do instead insert into test3 default values;
+copy (delete from test3) to stdout;
+drop rule qqq on test3;
+
+-- triggers
+create function qqq_trig() returns trigger as $$
+begin
+if tg_op in ('INSERT', 'UPDATE') then
+    raise notice '% %', tg_op, new.id;
+    return new;
+else
+    raise notice '% %', tg_op, old.id;
+    return old;
+end if;
+end
+$$ language plpgsql;
+create trigger qqqbef before insert or update or delete on test3
+    for each row execute procedure qqq_trig();
+create trigger qqqaf after insert or update or delete on test3
+    for each row execute procedure qqq_trig();
+
+copy (insert into test3 (t) values ('f') returning id) to stdout;
+copy (update test3 set t = 'g' where t = 'f' returning id) to stdout;
+copy (delete from test3 where t = 'g' returning id) to stdout;
+
+drop table test3;
+drop function qqq_trig();

Michael Paquier

michael.paquier@gmail.com

about 10 years ago

In reply to: Marko Tiikkaja (#1)

Re: COPY (INSERT/UPDATE/DELETE .. RETURNING ..)

On Sun, Nov 1, 2015 at 2:49 AM, Marko Tiikkaja wrote:

I can't add this to November's commit fest, but I'm posting this anyway in
case someone is thinking about implementing this feature.

Note: this one has been added to CF:
https://commitfest.postgresql.org/7/414/
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Michael Paquier

michael.paquier@gmail.com

about 10 years ago

In reply to: Marko Tiikkaja (#1)

1 attachment(s)

Re: COPY (INSERT/UPDATE/DELETE .. RETURNING ..)

On Sun, Nov 1, 2015 at 2:49 AM, Marko Tiikkaja <marko@joh.to> wrote:

Attached is a patch for being able to do $SUBJECT without a CTE. The
reasons this is better than a CTE version are:

1) It's not obvious why a CTE version works but a plain one doesn't
2) This one has less overhead (I measured a ~12% improvement on a
not-too-unreasonable test case)

Would you mind sharing this test case as well as numbers?

With regard to RULEs, similar restrictions apply as the ones on
data-modifying statements in WITH.

OK, so with this patch BeginCopy holds a light copy of
RewriteQuery@rewriteHandler.c that rewrites queries with rules, and this
patch extends the error handling after calling pg_analyze_and_rewrite. This
looks like the right approach.

INSERT/UPDATE/DELETE ... RETURNING is a Postgres extension not part of the
SQL spec, so my first thought is that there is no reason to reject this
feature based on the grammar allowed.

Here are some comments about the patch.

Nice patch to begin with.

Regression tests are broken when copyselect is run in parallel because
test3 is a table used there as well. A simple idea to fix this is to rename
the table test3 to something else or to use a schema dedicated to this
test, I just renamed it to copydml_test in the patch attached.

-                       | COPY select_with_parens TO opt_program
copy_file_name opt_with copy_options
+                       | COPY '(' PreparableStmt ')' TO opt_program
copy_file_name opt_with copy_options
This does not look right, PreparableStmt is used for statements that are
part of PREPARE queries, any modifications happening there would impact
COPY with this implementation. I think that we had better have a new option
query_with_parens. Please note that the patch attached has not changed that.
-- 
Michael

Attachments:

20151116_copydml_v2.patchtext/x-diff; charset=US-ASCII; name=20151116_copydml_v2.patchDownload

diff --git a/doc/src/sgml/ref/copy.sgml b/doc/src/sgml/ref/copy.sgml
index 2850b47..07e2f45 100644
--- a/doc/src/sgml/ref/copy.sgml
+++ b/doc/src/sgml/ref/copy.sgml
@@ -112,10 +112,17 @@ COPY { <replaceable class="parameter">table_name</replaceable> [ ( <replaceable
     <term><replaceable class="parameter">query</replaceable></term>
     <listitem>
      <para>
-      A <xref linkend="sql-select"> or
-      <xref linkend="sql-values"> command
-      whose results are to be copied.
-      Note that parentheses are required around the query.
+      A <xref linkend="sql-select">, <xref linkend="sql-values">,
+      <xref linkend="sql-insert">, <xref linkend="sql-update"> or
+      <xref linkend="sql-delete"> command whose results are to be
+      copied.  Note that parentheses are required around the query.
+     </para>
+     <para>
+      For <command>INSERT</>, <command>UPDATE</> and
+      <command>DELETE</> queries a RETURNING clause must be provided,
+      and the target relation must not have a conditional rule, nor
+      an <literal>ALSO</> rule, nor an <literal>INSTEAD</> rule
+      that expands to multiple statements.
      </para>
     </listitem>
    </varlistentry>
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 443b9e7..8bed38c 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -201,7 +201,7 @@ typedef struct CopyStateData
 	int			raw_buf_len;	/* total # of bytes stored */
 } CopyStateData;
 
-/* DestReceiver for COPY (SELECT) TO */
+/* DestReceiver for COPY (query) TO */
 typedef struct
 {
 	DestReceiver pub;			/* publicly-known function pointers */
@@ -772,7 +772,8 @@ CopyLoadRawBuf(CopyState cstate)
  *
  * Either unload or reload contents of table <relation>, depending on <from>.
  * (<from> = TRUE means we are inserting into the table.)  In the "TO" case
- * we also support copying the output of an arbitrary SELECT query.
+ * we also support copying the output of an arbitrary SELECT, INSERT, UPDATE
+ * or DELETE query.
  *
  * If <pipe> is false, transfer is between the table and the file named
  * <filename>.  Otherwise, transfer is between the table and our regular
@@ -1374,11 +1375,11 @@ BeginCopy(bool is_from,
 		Assert(!is_from);
 		cstate->rel = NULL;
 
-		/* Don't allow COPY w/ OIDs from a select */
+		/* Don't allow COPY w/ OIDs from a query */
 		if (cstate->oids)
 			ereport(ERROR,
 					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-					 errmsg("COPY (SELECT) WITH OIDS is not supported")));
+					 errmsg("COPY (query) WITH OIDS is not supported")));
 
 		/*
 		 * Run parse analysis and rewrite.  Note this also acquires sufficient
@@ -1393,9 +1394,36 @@ BeginCopy(bool is_from,
 		rewritten = pg_analyze_and_rewrite((Node *) copyObject(raw_query),
 										   queryString, NULL, 0);
 
-		/* We don't expect more or less than one result query */
-		if (list_length(rewritten) != 1)
-			elog(ERROR, "unexpected rewrite result");
+		/* check that we got back something we can work with */
+		if (rewritten == NIL)
+		{
+			ereport(ERROR,
+					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+					errmsg("DO INSTEAD NOTHING rules are not supported for COPY")));
+		}
+		else if (list_length(rewritten) > 1)
+		{
+			ListCell *lc;
+
+			/* examine queries to determine which error message to issue */
+			foreach(lc, rewritten)
+			{
+				Query	  *q = (Query *) lfirst(lc);
+
+				if (q->querySource == QSRC_QUAL_INSTEAD_RULE)
+					ereport(ERROR,
+							(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+							 errmsg("conditional DO INSTEAD rules are not supported for COPY")));
+				if (q->querySource == QSRC_NON_INSTEAD_RULE)
+					ereport(ERROR,
+							(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+							 errmsg("DO ALSO rules are not supported for the COPY")));
+			}
+
+			ereport(ERROR,
+					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+					 errmsg("multi-statement DO INSTEAD rules are not supported for COPY")));
+		}
 
 		query = (Query *) linitial(rewritten);
 
@@ -1406,9 +1434,20 @@ BeginCopy(bool is_from,
 					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 					 errmsg("COPY (SELECT INTO) is not supported")));
 
-		Assert(query->commandType == CMD_SELECT);
 		Assert(query->utilityStmt == NULL);
 
+		/*
+		 * Similarly the grammar doesn't enforce he presence of a RETURNING
+		 * clause, but we require it.
+		 */
+		if (query->commandType != CMD_SELECT &&
+			query->returningList == NIL)
+		{
+			ereport(ERROR,
+						(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+						 errmsg("COPY query must have a RETURNING clause")));
+		}
+
 		/* plan the query */
 		plan = pg_plan_query(query, 0, NULL);
 
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index fba91d5..d87ae93 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -2561,9 +2561,12 @@ ClosePortalStmt:
  *
  *		QUERY :
  *				COPY relname [(columnList)] FROM/TO file [WITH] [(options)]
- *				COPY ( SELECT ... ) TO file	[WITH] [(options)]
+ *				COPY ( query ) TO file	[WITH] [(options)]
  *
- *				where 'file' can be one of:
+ *				where 'query' is one of:
+ *				{ select | update | insert | delete }
+ *
+ *				and 'file' can be one of:
  *				{ PROGRAM 'command' | STDIN | STDOUT | 'filename' }
  *
  *				In the preferred syntax the options are comma-separated
@@ -2574,7 +2577,7 @@ ClosePortalStmt:
  *				COPY [ BINARY ] table [ WITH OIDS ] FROM/TO file
  *					[ [ USING ] DELIMITERS 'delimiter' ] ]
  *					[ WITH NULL AS 'null string' ]
- *				This option placement is not supported with COPY (SELECT...).
+ *				This option placement is not supported with COPY (query).
  *
  *****************************************************************************/
 
@@ -2607,16 +2610,16 @@ CopyStmt:	COPY opt_binary qualified_name opt_column_list opt_oids
 						n->options = list_concat(n->options, $11);
 					$$ = (Node *)n;
 				}
-			| COPY select_with_parens TO opt_program copy_file_name opt_with copy_options
+			| COPY '(' PreparableStmt ')' TO opt_program copy_file_name opt_with copy_options
 				{
 					CopyStmt *n = makeNode(CopyStmt);
 					n->relation = NULL;
-					n->query = $2;
+					n->query = $3;
 					n->attlist = NIL;
 					n->is_from = false;
-					n->is_program = $4;
-					n->filename = $5;
-					n->options = $7;
+					n->is_program = $6;
+					n->filename = $7;
+					n->options = $9;
 
 					if (n->is_program && n->filename == NULL)
 						ereport(ERROR,
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 9e1c48d..6b763ef 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -1680,7 +1680,7 @@ typedef struct CopyStmt
 {
 	NodeTag		type;
 	RangeVar   *relation;		/* the relation to copy */
-	Node	   *query;			/* the SELECT query to copy */
+	Node	   *query;			/* the query (SELECT or I/U/D with RETURNING) to copy */
 	List	   *attlist;		/* List of column names (as Strings), or NIL
 								 * for all columns */
 	bool		is_from;		/* TO or FROM */
diff --git a/src/test/regress/expected/copydml.out b/src/test/regress/expected/copydml.out
new file mode 100644
index 0000000..1b53396
--- /dev/null
+++ b/src/test/regress/expected/copydml.out
@@ -0,0 +1,112 @@
+--
+-- Test cases for COPY (INSERT/UPDATE/DELETE) TO
+--
+create table copydml_test (id serial, t text);
+insert into copydml_test (t) values ('a');
+insert into copydml_test (t) values ('b');
+insert into copydml_test (t) values ('c');
+insert into copydml_test (t) values ('d');
+insert into copydml_test (t) values ('e');
+--
+-- Test COPY (insert/update/delete ...)
+--
+copy (insert into copydml_test (t) values ('f') returning id) to stdout;
+6
+copy (update copydml_test set t = 'g' where t = 'f' returning id) to stdout;
+6
+copy (delete from copydml_test where t = 'g' returning id) to stdout;
+6
+--
+-- Test \copy (insert/update/delete ...)
+--
+\copy (insert into copydml_test (t) values ('f') returning id) to stdout;
+7
+\copy (update copydml_test set t = 'g' where t = 'f' returning id) to stdout;
+7
+\copy (delete from copydml_test where t = 'g' returning id) to stdout;
+7
+-- Error cases
+copy (insert into copydml_test default values) to stdout;
+ERROR:  COPY query must have a RETURNING clause
+copy (update copydml_test set t = 'g') to stdout;
+ERROR:  COPY query must have a RETURNING clause
+copy (delete from copydml_test) to stdout;
+ERROR:  COPY query must have a RETURNING clause
+create rule qqq as on insert to copydml_test do instead nothing;
+copy (insert into copydml_test default values) to stdout;
+ERROR:  DO INSTEAD NOTHING rules are not supported for COPY
+drop rule qqq on copydml_test;
+create rule qqq as on insert to copydml_test do also delete from copydml_test;
+copy (insert into copydml_test default values) to stdout;
+ERROR:  DO ALSO rules are not supported for the COPY
+drop rule qqq on copydml_test;
+create rule qqq as on insert to copydml_test do instead (delete from copydml_test; delete from copydml_test);
+copy (insert into copydml_test default values) to stdout;
+ERROR:  multi-statement DO INSTEAD rules are not supported for COPY
+drop rule qqq on copydml_test;
+create rule qqq as on insert to copydml_test where new.t <> 'f' do instead delete from copydml_test;
+copy (insert into copydml_test default values) to stdout;
+ERROR:  conditional DO INSTEAD rules are not supported for COPY
+drop rule qqq on copydml_test;
+create rule qqq as on update to copydml_test do instead nothing;
+copy (update copydml_test set t = 'f') to stdout;
+ERROR:  DO INSTEAD NOTHING rules are not supported for COPY
+drop rule qqq on copydml_test;
+create rule qqq as on update to copydml_test do also delete from copydml_test;
+copy (update copydml_test set t = 'f') to stdout;
+ERROR:  DO ALSO rules are not supported for the COPY
+drop rule qqq on copydml_test;
+create rule qqq as on update to copydml_test do instead (delete from copydml_test; delete from copydml_test);
+copy (update copydml_test set t = 'f') to stdout;
+ERROR:  multi-statement DO INSTEAD rules are not supported for COPY
+drop rule qqq on copydml_test;
+create rule qqq as on update to copydml_test where new.t <> 'f' do instead delete from copydml_test;
+copy (update copydml_test set t = 'f') to stdout;
+ERROR:  conditional DO INSTEAD rules are not supported for COPY
+drop rule qqq on copydml_test;
+create rule qqq as on delete to copydml_test do instead nothing;
+copy (delete from copydml_test) to stdout;
+ERROR:  DO INSTEAD NOTHING rules are not supported for COPY
+drop rule qqq on copydml_test;
+create rule qqq as on delete to copydml_test do also insert into copydml_test default values;
+copy (delete from copydml_test) to stdout;
+ERROR:  DO ALSO rules are not supported for the COPY
+drop rule qqq on copydml_test;
+create rule qqq as on delete to copydml_test do instead (insert into copydml_test default values; insert into copydml_test default values);
+copy (delete from copydml_test) to stdout;
+ERROR:  multi-statement DO INSTEAD rules are not supported for COPY
+drop rule qqq on copydml_test;
+create rule qqq as on delete to copydml_test where old.t <> 'f' do instead insert into copydml_test default values;
+copy (delete from copydml_test) to stdout;
+ERROR:  conditional DO INSTEAD rules are not supported for COPY
+drop rule qqq on copydml_test;
+-- triggers
+create function qqq_trig() returns trigger as $$
+begin
+if tg_op in ('INSERT', 'UPDATE') then
+    raise notice '% %', tg_op, new.id;
+    return new;
+else
+    raise notice '% %', tg_op, old.id;
+    return old;
+end if;
+end
+$$ language plpgsql;
+create trigger qqqbef before insert or update or delete on copydml_test
+    for each row execute procedure qqq_trig();
+create trigger qqqaf after insert or update or delete on copydml_test
+    for each row execute procedure qqq_trig();
+copy (insert into copydml_test (t) values ('f') returning id) to stdout;
+NOTICE:  INSERT 8
+8
+NOTICE:  INSERT 8
+copy (update copydml_test set t = 'g' where t = 'f' returning id) to stdout;
+NOTICE:  UPDATE 8
+8
+NOTICE:  UPDATE 8
+copy (delete from copydml_test where t = 'g' returning id) to stdout;
+NOTICE:  DELETE 8
+8
+NOTICE:  DELETE 8
+drop table copydml_test;
+drop function qqq_trig();
diff --git a/src/test/regress/parallel_schedule b/src/test/regress/parallel_schedule
index 3987b4c..b1bc7c7 100644
--- a/src/test/regress/parallel_schedule
+++ b/src/test/regress/parallel_schedule
@@ -48,7 +48,7 @@ test: create_function_2
 # execute two copy tests parallel, to check that copy itself
 # is concurrent safe.
 # ----------
-test: copy copyselect
+test: copy copyselect copydml
 
 # ----------
 # More groups of parallel tests
diff --git a/src/test/regress/serial_schedule b/src/test/regress/serial_schedule
index 379f272..ade9ef1 100644
--- a/src/test/regress/serial_schedule
+++ b/src/test/regress/serial_schedule
@@ -57,6 +57,7 @@ test: create_table
 test: create_function_2
 test: copy
 test: copyselect
+test: copydml
 test: create_misc
 test: create_operator
 test: create_index
diff --git a/src/test/regress/sql/copydml.sql b/src/test/regress/sql/copydml.sql
new file mode 100644
index 0000000..9a29f9c
--- /dev/null
+++ b/src/test/regress/sql/copydml.sql
@@ -0,0 +1,91 @@
+--
+-- Test cases for COPY (INSERT/UPDATE/DELETE) TO
+--
+create table copydml_test (id serial, t text);
+insert into copydml_test (t) values ('a');
+insert into copydml_test (t) values ('b');
+insert into copydml_test (t) values ('c');
+insert into copydml_test (t) values ('d');
+insert into copydml_test (t) values ('e');
+
+--
+-- Test COPY (insert/update/delete ...)
+--
+copy (insert into copydml_test (t) values ('f') returning id) to stdout;
+copy (update copydml_test set t = 'g' where t = 'f' returning id) to stdout;
+copy (delete from copydml_test where t = 'g' returning id) to stdout;
+
+--
+-- Test \copy (insert/update/delete ...)
+--
+\copy (insert into copydml_test (t) values ('f') returning id) to stdout;
+\copy (update copydml_test set t = 'g' where t = 'f' returning id) to stdout;
+\copy (delete from copydml_test where t = 'g' returning id) to stdout;
+
+-- Error cases
+copy (insert into copydml_test default values) to stdout;
+copy (update copydml_test set t = 'g') to stdout;
+copy (delete from copydml_test) to stdout;
+
+create rule qqq as on insert to copydml_test do instead nothing;
+copy (insert into copydml_test default values) to stdout;
+drop rule qqq on copydml_test;
+create rule qqq as on insert to copydml_test do also delete from copydml_test;
+copy (insert into copydml_test default values) to stdout;
+drop rule qqq on copydml_test;
+create rule qqq as on insert to copydml_test do instead (delete from copydml_test; delete from copydml_test);
+copy (insert into copydml_test default values) to stdout;
+drop rule qqq on copydml_test;
+create rule qqq as on insert to copydml_test where new.t <> 'f' do instead delete from copydml_test;
+copy (insert into copydml_test default values) to stdout;
+drop rule qqq on copydml_test;
+
+create rule qqq as on update to copydml_test do instead nothing;
+copy (update copydml_test set t = 'f') to stdout;
+drop rule qqq on copydml_test;
+create rule qqq as on update to copydml_test do also delete from copydml_test;
+copy (update copydml_test set t = 'f') to stdout;
+drop rule qqq on copydml_test;
+create rule qqq as on update to copydml_test do instead (delete from copydml_test; delete from copydml_test);
+copy (update copydml_test set t = 'f') to stdout;
+drop rule qqq on copydml_test;
+create rule qqq as on update to copydml_test where new.t <> 'f' do instead delete from copydml_test;
+copy (update copydml_test set t = 'f') to stdout;
+drop rule qqq on copydml_test;
+
+create rule qqq as on delete to copydml_test do instead nothing;
+copy (delete from copydml_test) to stdout;
+drop rule qqq on copydml_test;
+create rule qqq as on delete to copydml_test do also insert into copydml_test default values;
+copy (delete from copydml_test) to stdout;
+drop rule qqq on copydml_test;
+create rule qqq as on delete to copydml_test do instead (insert into copydml_test default values; insert into copydml_test default values);
+copy (delete from copydml_test) to stdout;
+drop rule qqq on copydml_test;
+create rule qqq as on delete to copydml_test where old.t <> 'f' do instead insert into copydml_test default values;
+copy (delete from copydml_test) to stdout;
+drop rule qqq on copydml_test;
+
+-- triggers
+create function qqq_trig() returns trigger as $$
+begin
+if tg_op in ('INSERT', 'UPDATE') then
+    raise notice '% %', tg_op, new.id;
+    return new;
+else
+    raise notice '% %', tg_op, old.id;
+    return old;
+end if;
+end
+$$ language plpgsql;
+create trigger qqqbef before insert or update or delete on copydml_test
+    for each row execute procedure qqq_trig();
+create trigger qqqaf after insert or update or delete on copydml_test
+    for each row execute procedure qqq_trig();
+
+copy (insert into copydml_test (t) values ('f') returning id) to stdout;
+copy (update copydml_test set t = 'g' where t = 'f' returning id) to stdout;
+copy (delete from copydml_test where t = 'g' returning id) to stdout;
+
+drop table copydml_test;
+drop function qqq_trig();

Marko Tiikkaja

marko@joh.to

about 10 years ago

In reply to: Michael Paquier (#3)

Re: COPY (INSERT/UPDATE/DELETE .. RETURNING ..)

On 2015-11-16 08:24, Michael Paquier wrote:

On Sun, Nov 1, 2015 at 2:49 AM, Marko Tiikkaja <marko@joh.to> wrote:

Attached is a patch for being able to do $SUBJECT without a CTE. The
reasons this is better than a CTE version are:

1) It's not obvious why a CTE version works but a plain one doesn't
2) This one has less overhead (I measured a ~12% improvement on a
not-too-unreasonable test case)

Would you mind sharing this test case as well as numbers?

I'll try and re-puzzle it together tomorrow. It looks like I wasn't
smart enough to actually save the test case anywhere.

Regression tests are broken when copyselect is run in parallel because
test3 is a table used there as well. A simple idea to fix this is to rename
the table test3 to something else or to use a schema dedicated to this
test, I just renamed it to copydml_test in the patch attached.

Seems reasonable.

-                       | COPY select_with_parens TO opt_program
copy_file_name opt_with copy_options
+                       | COPY '(' PreparableStmt ')' TO opt_program
copy_file_name opt_with copy_options
This does not look right, PreparableStmt is used for statements that are
part of PREPARE queries, any modifications happening there would impact
COPY with this implementation. I think that we had better have a new option
query_with_parens. Please note that the patch attached has not changed that.

This was discussed in 2010 when CTEs got the same treatment, and I agree
with what was decided back then. If someone needs to make
PreparableStmt different from what COPY and CTEs support, we can split
them up. But they're completely identical after this patch, so
splitting them up right now is somewhat pointless.

Further, if someone's going to add new stuff to PreparableStmt, she
should probably think about whether it would make sense to add it to
COPY and CTEs from day one, too, and in that case not splitting them up
is actually a win.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Michael Paquier

michael.paquier@gmail.com

about 10 years ago

In reply to: Marko Tiikkaja (#4)

Re: COPY (INSERT/UPDATE/DELETE .. RETURNING ..)

On Thu, Nov 19, 2015 at 11:04 AM, Marko Tiikkaja wrote:

This was discussed in 2010 when CTEs got the same treatment, and I agree
with what was decided back then. If someone needs to make PreparableStmt
different from what COPY and CTEs support, we can split them up. But
they're completely identical after this patch, so splitting them up right
now is somewhat pointless.
Further, if someone's going to add new stuff to PreparableStmt, she should
probably think about whether it would make sense to add it to COPY and CTEs
from day one, too, and in that case not splitting them up is actually a win.

Personally, I would take it on the safe side and actually update it.
If someone were to add a new node type in PreparableStmt I am pretty
sure that we are going to forget to update the COPY part, leading us
to unwelcome bugs. And that would not be cool.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Marko Tiikkaja

marko@joh.to

about 10 years ago

In reply to: Michael Paquier (#5)

Re: COPY (INSERT/UPDATE/DELETE .. RETURNING ..)

On 11/19/15 1:17 PM, Michael Paquier wrote:

On Thu, Nov 19, 2015 at 11:04 AM, Marko Tiikkaja wrote:

Further, if someone's going to add new stuff to PreparableStmt, she should
probably think about whether it would make sense to add it to COPY and CTEs
from day one, too, and in that case not splitting them up is actually a win.

Personally, I would take it on the safe side and actually update it.
If someone were to add a new node type in PreparableStmt I am pretty
sure that we are going to forget to update the COPY part, leading us
to unwelcome bugs. And that would not be cool.

They'd have to get past this:

+       if (query->commandType != CMD_SELECT &&
+           query->returningList == NIL)
+       {
+           ereport(ERROR,
+                       (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                        errmsg("COPY query must have a RETURNING 
clause")));
+       }

Of course, something might break if we added a new statement type which
supported RETURNING, but I'm really not worried about that. I'm not
dead set against adding some Assert(IsA()) calls here, but I don't see
the point.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Michael Paquier

michael.paquier@gmail.com

about 10 years ago

In reply to: Marko Tiikkaja (#6)

Re: COPY (INSERT/UPDATE/DELETE .. RETURNING ..)

On Thu, Nov 19, 2015 at 9:22 PM, Marko Tiikkaja <marko@joh.to> wrote:

Of course, something might break if we added a new statement type which
supported RETURNING, but I'm really not worried about that. I'm not dead
set against adding some Assert(IsA()) calls here, but I don't see the point.

gram.y has a long comment before select_no_parens regarding why we
shouldn't do it this way, did you notice it? I haven't crafted yet a
backward incompatible query with your patch yet, but something smells
fishy here.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Marko Tiikkaja

marko@joh.to

about 10 years ago

In reply to: Michael Paquier (#7)

Re: COPY (INSERT/UPDATE/DELETE .. RETURNING ..)

On 11/19/15 7:39 PM, Michael Paquier wrote:

On Thu, Nov 19, 2015 at 9:22 PM, Marko Tiikkaja <marko@joh.to> wrote:

Of course, something might break if we added a new statement type which
supported RETURNING, but I'm really not worried about that. I'm not dead
set against adding some Assert(IsA()) calls here, but I don't see the point.

gram.y has a long comment before select_no_parens regarding why we
shouldn't do it this way, did you notice it?

It talks about not doing '(' SelectStmt ')' "in the expression
grammar". I don't see what that has to do with this patch.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Marko Tiikkaja

marko@joh.to

about 10 years ago

In reply to: Michael Paquier (#3)

2 attachment(s)

Re: COPY (INSERT/UPDATE/DELETE .. RETURNING ..)

On 2015-11-16 08:24, Michael Paquier wrote:

On Sun, Nov 1, 2015 at 2:49 AM, Marko Tiikkaja <marko@joh.to> wrote:

Attached is a patch for being able to do $SUBJECT without a CTE. The
reasons this is better than a CTE version are:

1) It's not obvious why a CTE version works but a plain one doesn't
2) This one has less overhead (I measured a ~12% improvement on a
not-too-unreasonable test case)

Would you mind sharing this test case as well as numbers?

Attached the test case I used; removing a batch of old rows from a table
through an index. Best out of three runs, I get 13.1 seconds versus
15.0 seconds, which should amount to an improvement of around 12.7%.

Also attached a v3 of the patch which adds an Assert on top of the test
case changes suggested by you.

Attachments:

copy_dml_returning_test.sqltext/plain; charset=UTF-8; name=copy_dml_returning_test.sql; x-mac-creator=0; x-mac-type=0Download

copy_dml_returning_v3.patchtext/x-patch; name=copy_dml_returning_v3.patchDownload

*** a/doc/src/sgml/ref/copy.sgml
--- b/doc/src/sgml/ref/copy.sgml
***************
*** 112,121 **** COPY { <replaceable class="parameter">table_name</replaceable> [ ( <replaceable
      <term><replaceable class="parameter">query</replaceable></term>
      <listitem>
       <para>
!       A <xref linkend="sql-select"> or
!       <xref linkend="sql-values"> command
!       whose results are to be copied.
!       Note that parentheses are required around the query.
       </para>
      </listitem>
     </varlistentry>
--- 112,128 ----
      <term><replaceable class="parameter">query</replaceable></term>
      <listitem>
       <para>
!       A <xref linkend="sql-select">, <xref linkend="sql-values">,
!       <xref linkend="sql-insert">, <xref linkend="sql-update"> or
!       <xref linkend="sql-delete"> command whose results are to be
!       copied.  Note that parentheses are required around the query.
!      </para>
!      <para>
!       For <command>INSERT</>, <command>UPDATE</> and
!       <command>DELETE</> queries a RETURNING clause must be provided,
!       and the target relation must not have a conditional rule, nor
!       an <literal>ALSO</> rule, nor an <literal>INSTEAD</> rule
!       that expands to multiple statements.
       </para>
      </listitem>
     </varlistentry>
*** a/src/backend/commands/copy.c
--- b/src/backend/commands/copy.c
***************
*** 201,207 **** typedef struct CopyStateData
  	int			raw_buf_len;	/* total # of bytes stored */
  } CopyStateData;
  
! /* DestReceiver for COPY (SELECT) TO */
  typedef struct
  {
  	DestReceiver pub;			/* publicly-known function pointers */
--- 201,207 ----
  	int			raw_buf_len;	/* total # of bytes stored */
  } CopyStateData;
  
! /* DestReceiver for COPY (query) TO */
  typedef struct
  {
  	DestReceiver pub;			/* publicly-known function pointers */
***************
*** 772,778 **** CopyLoadRawBuf(CopyState cstate)
   *
   * Either unload or reload contents of table <relation>, depending on <from>.
   * (<from> = TRUE means we are inserting into the table.)  In the "TO" case
!  * we also support copying the output of an arbitrary SELECT query.
   *
   * If <pipe> is false, transfer is between the table and the file named
   * <filename>.  Otherwise, transfer is between the table and our regular
--- 772,779 ----
   *
   * Either unload or reload contents of table <relation>, depending on <from>.
   * (<from> = TRUE means we are inserting into the table.)  In the "TO" case
!  * we also support copying the output of an arbitrary SELECT, INSERT, UPDATE
!  * or DELETE query.
   *
   * If <pipe> is false, transfer is between the table and the file named
   * <filename>.  Otherwise, transfer is between the table and our regular
***************
*** 1374,1384 **** BeginCopy(bool is_from,
  		Assert(!is_from);
  		cstate->rel = NULL;
  
! 		/* Don't allow COPY w/ OIDs from a select */
  		if (cstate->oids)
  			ereport(ERROR,
  					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
! 					 errmsg("COPY (SELECT) WITH OIDS is not supported")));
  
  		/*
  		 * Run parse analysis and rewrite.  Note this also acquires sufficient
--- 1375,1385 ----
  		Assert(!is_from);
  		cstate->rel = NULL;
  
! 		/* Don't allow COPY w/ OIDs from a query */
  		if (cstate->oids)
  			ereport(ERROR,
  					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
! 					 errmsg("COPY (query) WITH OIDS is not supported")));
  
  		/*
  		 * Run parse analysis and rewrite.  Note this also acquires sufficient
***************
*** 1393,1401 **** BeginCopy(bool is_from,
  		rewritten = pg_analyze_and_rewrite((Node *) copyObject(raw_query),
  										   queryString, NULL, 0);
  
! 		/* We don't expect more or less than one result query */
! 		if (list_length(rewritten) != 1)
! 			elog(ERROR, "unexpected rewrite result");
  
  		query = (Query *) linitial(rewritten);
  
--- 1394,1429 ----
  		rewritten = pg_analyze_and_rewrite((Node *) copyObject(raw_query),
  										   queryString, NULL, 0);
  
! 		/* check that we got back something we can work with */
! 		if (rewritten == NIL)
! 		{
! 			ereport(ERROR,
! 					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
! 					errmsg("DO INSTEAD NOTHING rules are not supported for COPY")));
! 		}
! 		else if (list_length(rewritten) > 1)
! 		{
! 			ListCell *lc;
! 
! 			/* examine queries to determine which error message to issue */
! 			foreach(lc, rewritten)
! 			{
! 				Query	  *q = (Query *) lfirst(lc);
! 
! 				if (q->querySource == QSRC_QUAL_INSTEAD_RULE)
! 					ereport(ERROR,
! 							(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
! 							 errmsg("conditional DO INSTEAD rules are not supported for COPY")));
! 				if (q->querySource == QSRC_NON_INSTEAD_RULE)
! 					ereport(ERROR,
! 							(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
! 							 errmsg("DO ALSO rules are not supported for the COPY")));
! 			}
! 
! 			ereport(ERROR,
! 					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
! 					 errmsg("multi-statement DO INSTEAD rules are not supported for COPY")));
! 		}
  
  		query = (Query *) linitial(rewritten);
  
***************
*** 1406,1414 **** BeginCopy(bool is_from,
  					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
  					 errmsg("COPY (SELECT INTO) is not supported")));
  
- 		Assert(query->commandType == CMD_SELECT);
  		Assert(query->utilityStmt == NULL);
  
  		/* plan the query */
  		plan = pg_plan_query(query, 0, NULL);
  
--- 1434,1457 ----
  					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
  					 errmsg("COPY (SELECT INTO) is not supported")));
  
  		Assert(query->utilityStmt == NULL);
  
+ 		/*
+ 		 * Similarly the grammar doesn't enforce he presence of a RETURNING
+ 		 * clause, but we require it.
+ 		 */
+ 		if (query->commandType != CMD_SELECT)
+ 		{
+ 			Assert(query->commandType == CMD_INSERT ||
+ 				   query->commandType == CMD_UPDATE ||
+ 				   query->commandType == CMD_DELETE);
+ 
+ 			if (query->returningList == NIL)
+ 				ereport(ERROR,
+ 							(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ 							 errmsg("COPY query must have a RETURNING clause")));
+ 		}
+ 
  		/* plan the query */
  		plan = pg_plan_query(query, 0, NULL);
  
*** a/src/backend/parser/gram.y
--- b/src/backend/parser/gram.y
***************
*** 2561,2569 **** ClosePortalStmt:
   *
   *		QUERY :
   *				COPY relname [(columnList)] FROM/TO file [WITH] [(options)]
!  *				COPY ( SELECT ... ) TO file	[WITH] [(options)]
   *
!  *				where 'file' can be one of:
   *				{ PROGRAM 'command' | STDIN | STDOUT | 'filename' }
   *
   *				In the preferred syntax the options are comma-separated
--- 2561,2572 ----
   *
   *		QUERY :
   *				COPY relname [(columnList)] FROM/TO file [WITH] [(options)]
!  *				COPY ( query ) TO file	[WITH] [(options)]
   *
!  *				where 'query' is one of:
!  *				{ select | update | insert | delete }
!  *
!  *				and 'file' can be one of:
   *				{ PROGRAM 'command' | STDIN | STDOUT | 'filename' }
   *
   *				In the preferred syntax the options are comma-separated
***************
*** 2574,2580 **** ClosePortalStmt:
   *				COPY [ BINARY ] table [ WITH OIDS ] FROM/TO file
   *					[ [ USING ] DELIMITERS 'delimiter' ] ]
   *					[ WITH NULL AS 'null string' ]
!  *				This option placement is not supported with COPY (SELECT...).
   *
   *****************************************************************************/
  
--- 2577,2583 ----
   *				COPY [ BINARY ] table [ WITH OIDS ] FROM/TO file
   *					[ [ USING ] DELIMITERS 'delimiter' ] ]
   *					[ WITH NULL AS 'null string' ]
!  *				This option placement is not supported with COPY (query).
   *
   *****************************************************************************/
  
***************
*** 2607,2622 **** CopyStmt:	COPY opt_binary qualified_name opt_column_list opt_oids
  						n->options = list_concat(n->options, $11);
  					$$ = (Node *)n;
  				}
! 			| COPY select_with_parens TO opt_program copy_file_name opt_with copy_options
  				{
  					CopyStmt *n = makeNode(CopyStmt);
  					n->relation = NULL;
! 					n->query = $2;
  					n->attlist = NIL;
  					n->is_from = false;
! 					n->is_program = $4;
! 					n->filename = $5;
! 					n->options = $7;
  
  					if (n->is_program && n->filename == NULL)
  						ereport(ERROR,
--- 2610,2625 ----
  						n->options = list_concat(n->options, $11);
  					$$ = (Node *)n;
  				}
! 			| COPY '(' PreparableStmt ')' TO opt_program copy_file_name opt_with copy_options
  				{
  					CopyStmt *n = makeNode(CopyStmt);
  					n->relation = NULL;
! 					n->query = $3;
  					n->attlist = NIL;
  					n->is_from = false;
! 					n->is_program = $6;
! 					n->filename = $7;
! 					n->options = $9;
  
  					if (n->is_program && n->filename == NULL)
  						ereport(ERROR,
*** a/src/include/nodes/parsenodes.h
--- b/src/include/nodes/parsenodes.h
***************
*** 1680,1686 **** typedef struct CopyStmt
  {
  	NodeTag		type;
  	RangeVar   *relation;		/* the relation to copy */
! 	Node	   *query;			/* the SELECT query to copy */
  	List	   *attlist;		/* List of column names (as Strings), or NIL
  								 * for all columns */
  	bool		is_from;		/* TO or FROM */
--- 1680,1686 ----
  {
  	NodeTag		type;
  	RangeVar   *relation;		/* the relation to copy */
! 	Node	   *query;			/* the query (SELECT or I/U/D with RETURNING) to copy */
  	List	   *attlist;		/* List of column names (as Strings), or NIL
  								 * for all columns */
  	bool		is_from;		/* TO or FROM */
*** /dev/null
--- b/src/test/regress/expected/copydml.out
***************
*** 0 ****
--- 1,112 ----
+ --
+ -- Test cases for COPY (INSERT/UPDATE/DELETE) TO
+ --
+ create table copydml_test (id serial, t text);
+ insert into copydml_test (t) values ('a');
+ insert into copydml_test (t) values ('b');
+ insert into copydml_test (t) values ('c');
+ insert into copydml_test (t) values ('d');
+ insert into copydml_test (t) values ('e');
+ --
+ -- Test COPY (insert/update/delete ...)
+ --
+ copy (insert into copydml_test (t) values ('f') returning id) to stdout;
+ 6
+ copy (update copydml_test set t = 'g' where t = 'f' returning id) to stdout;
+ 6
+ copy (delete from copydml_test where t = 'g' returning id) to stdout;
+ 6
+ --
+ -- Test \copy (insert/update/delete ...)
+ --
+ \copy (insert into copydml_test (t) values ('f') returning id) to stdout;
+ 7
+ \copy (update copydml_test set t = 'g' where t = 'f' returning id) to stdout;
+ 7
+ \copy (delete from copydml_test where t = 'g' returning id) to stdout;
+ 7
+ -- Error cases
+ copy (insert into copydml_test default values) to stdout;
+ ERROR:  COPY query must have a RETURNING clause
+ copy (update copydml_test set t = 'g') to stdout;
+ ERROR:  COPY query must have a RETURNING clause
+ copy (delete from copydml_test) to stdout;
+ ERROR:  COPY query must have a RETURNING clause
+ create rule qqq as on insert to copydml_test do instead nothing;
+ copy (insert into copydml_test default values) to stdout;
+ ERROR:  DO INSTEAD NOTHING rules are not supported for COPY
+ drop rule qqq on copydml_test;
+ create rule qqq as on insert to copydml_test do also delete from copydml_test;
+ copy (insert into copydml_test default values) to stdout;
+ ERROR:  DO ALSO rules are not supported for the COPY
+ drop rule qqq on copydml_test;
+ create rule qqq as on insert to copydml_test do instead (delete from copydml_test; delete from copydml_test);
+ copy (insert into copydml_test default values) to stdout;
+ ERROR:  multi-statement DO INSTEAD rules are not supported for COPY
+ drop rule qqq on copydml_test;
+ create rule qqq as on insert to copydml_test where new.t <> 'f' do instead delete from copydml_test;
+ copy (insert into copydml_test default values) to stdout;
+ ERROR:  conditional DO INSTEAD rules are not supported for COPY
+ drop rule qqq on copydml_test;
+ create rule qqq as on update to copydml_test do instead nothing;
+ copy (update copydml_test set t = 'f') to stdout;
+ ERROR:  DO INSTEAD NOTHING rules are not supported for COPY
+ drop rule qqq on copydml_test;
+ create rule qqq as on update to copydml_test do also delete from copydml_test;
+ copy (update copydml_test set t = 'f') to stdout;
+ ERROR:  DO ALSO rules are not supported for the COPY
+ drop rule qqq on copydml_test;
+ create rule qqq as on update to copydml_test do instead (delete from copydml_test; delete from copydml_test);
+ copy (update copydml_test set t = 'f') to stdout;
+ ERROR:  multi-statement DO INSTEAD rules are not supported for COPY
+ drop rule qqq on copydml_test;
+ create rule qqq as on update to copydml_test where new.t <> 'f' do instead delete from copydml_test;
+ copy (update copydml_test set t = 'f') to stdout;
+ ERROR:  conditional DO INSTEAD rules are not supported for COPY
+ drop rule qqq on copydml_test;
+ create rule qqq as on delete to copydml_test do instead nothing;
+ copy (delete from copydml_test) to stdout;
+ ERROR:  DO INSTEAD NOTHING rules are not supported for COPY
+ drop rule qqq on copydml_test;
+ create rule qqq as on delete to copydml_test do also insert into copydml_test default values;
+ copy (delete from copydml_test) to stdout;
+ ERROR:  DO ALSO rules are not supported for the COPY
+ drop rule qqq on copydml_test;
+ create rule qqq as on delete to copydml_test do instead (insert into copydml_test default values; insert into copydml_test default values);
+ copy (delete from copydml_test) to stdout;
+ ERROR:  multi-statement DO INSTEAD rules are not supported for COPY
+ drop rule qqq on copydml_test;
+ create rule qqq as on delete to copydml_test where old.t <> 'f' do instead insert into copydml_test default values;
+ copy (delete from copydml_test) to stdout;
+ ERROR:  conditional DO INSTEAD rules are not supported for COPY
+ drop rule qqq on copydml_test;
+ -- triggers
+ create function qqq_trig() returns trigger as $$
+ begin
+ if tg_op in ('INSERT', 'UPDATE') then
+     raise notice '% %', tg_op, new.id;
+     return new;
+ else
+     raise notice '% %', tg_op, old.id;
+     return old;
+ end if;
+ end
+ $$ language plpgsql;
+ create trigger qqqbef before insert or update or delete on copydml_test
+     for each row execute procedure qqq_trig();
+ create trigger qqqaf after insert or update or delete on copydml_test
+     for each row execute procedure qqq_trig();
+ copy (insert into copydml_test (t) values ('f') returning id) to stdout;
+ NOTICE:  INSERT 8
+ 8
+ NOTICE:  INSERT 8
+ copy (update copydml_test set t = 'g' where t = 'f' returning id) to stdout;
+ NOTICE:  UPDATE 8
+ 8
+ NOTICE:  UPDATE 8
+ copy (delete from copydml_test where t = 'g' returning id) to stdout;
+ NOTICE:  DELETE 8
+ 8
+ NOTICE:  DELETE 8
+ drop table copydml_test;
+ drop function qqq_trig();
*** a/src/test/regress/parallel_schedule
--- b/src/test/regress/parallel_schedule
***************
*** 48,54 **** test: create_function_2
  # execute two copy tests parallel, to check that copy itself
  # is concurrent safe.
  # ----------
! test: copy copyselect
  
  # ----------
  # More groups of parallel tests
--- 48,54 ----
  # execute two copy tests parallel, to check that copy itself
  # is concurrent safe.
  # ----------
! test: copy copyselect copydml
  
  # ----------
  # More groups of parallel tests
*** a/src/test/regress/serial_schedule
--- b/src/test/regress/serial_schedule
***************
*** 57,62 **** test: create_table
--- 57,63 ----
  test: create_function_2
  test: copy
  test: copyselect
+ test: copydml
  test: create_misc
  test: create_operator
  test: create_index
*** /dev/null
--- b/src/test/regress/sql/copydml.sql
***************
*** 0 ****
--- 1,91 ----
+ --
+ -- Test cases for COPY (INSERT/UPDATE/DELETE) TO
+ --
+ create table copydml_test (id serial, t text);
+ insert into copydml_test (t) values ('a');
+ insert into copydml_test (t) values ('b');
+ insert into copydml_test (t) values ('c');
+ insert into copydml_test (t) values ('d');
+ insert into copydml_test (t) values ('e');
+ 
+ --
+ -- Test COPY (insert/update/delete ...)
+ --
+ copy (insert into copydml_test (t) values ('f') returning id) to stdout;
+ copy (update copydml_test set t = 'g' where t = 'f' returning id) to stdout;
+ copy (delete from copydml_test where t = 'g' returning id) to stdout;
+ 
+ --
+ -- Test \copy (insert/update/delete ...)
+ --
+ \copy (insert into copydml_test (t) values ('f') returning id) to stdout;
+ \copy (update copydml_test set t = 'g' where t = 'f' returning id) to stdout;
+ \copy (delete from copydml_test where t = 'g' returning id) to stdout;
+ 
+ -- Error cases
+ copy (insert into copydml_test default values) to stdout;
+ copy (update copydml_test set t = 'g') to stdout;
+ copy (delete from copydml_test) to stdout;
+ 
+ create rule qqq as on insert to copydml_test do instead nothing;
+ copy (insert into copydml_test default values) to stdout;
+ drop rule qqq on copydml_test;
+ create rule qqq as on insert to copydml_test do also delete from copydml_test;
+ copy (insert into copydml_test default values) to stdout;
+ drop rule qqq on copydml_test;
+ create rule qqq as on insert to copydml_test do instead (delete from copydml_test; delete from copydml_test);
+ copy (insert into copydml_test default values) to stdout;
+ drop rule qqq on copydml_test;
+ create rule qqq as on insert to copydml_test where new.t <> 'f' do instead delete from copydml_test;
+ copy (insert into copydml_test default values) to stdout;
+ drop rule qqq on copydml_test;
+ 
+ create rule qqq as on update to copydml_test do instead nothing;
+ copy (update copydml_test set t = 'f') to stdout;
+ drop rule qqq on copydml_test;
+ create rule qqq as on update to copydml_test do also delete from copydml_test;
+ copy (update copydml_test set t = 'f') to stdout;
+ drop rule qqq on copydml_test;
+ create rule qqq as on update to copydml_test do instead (delete from copydml_test; delete from copydml_test);
+ copy (update copydml_test set t = 'f') to stdout;
+ drop rule qqq on copydml_test;
+ create rule qqq as on update to copydml_test where new.t <> 'f' do instead delete from copydml_test;
+ copy (update copydml_test set t = 'f') to stdout;
+ drop rule qqq on copydml_test;
+ 
+ create rule qqq as on delete to copydml_test do instead nothing;
+ copy (delete from copydml_test) to stdout;
+ drop rule qqq on copydml_test;
+ create rule qqq as on delete to copydml_test do also insert into copydml_test default values;
+ copy (delete from copydml_test) to stdout;
+ drop rule qqq on copydml_test;
+ create rule qqq as on delete to copydml_test do instead (insert into copydml_test default values; insert into copydml_test default values);
+ copy (delete from copydml_test) to stdout;
+ drop rule qqq on copydml_test;
+ create rule qqq as on delete to copydml_test where old.t <> 'f' do instead insert into copydml_test default values;
+ copy (delete from copydml_test) to stdout;
+ drop rule qqq on copydml_test;
+ 
+ -- triggers
+ create function qqq_trig() returns trigger as $$
+ begin
+ if tg_op in ('INSERT', 'UPDATE') then
+     raise notice '% %', tg_op, new.id;
+     return new;
+ else
+     raise notice '% %', tg_op, old.id;
+     return old;
+ end if;
+ end
+ $$ language plpgsql;
+ create trigger qqqbef before insert or update or delete on copydml_test
+     for each row execute procedure qqq_trig();
+ create trigger qqqaf after insert or update or delete on copydml_test
+     for each row execute procedure qqq_trig();
+ 
+ copy (insert into copydml_test (t) values ('f') returning id) to stdout;
+ copy (update copydml_test set t = 'g' where t = 'f' returning id) to stdout;
+ copy (delete from copydml_test where t = 'g' returning id) to stdout;
+ 
+ drop table copydml_test;
+ drop function qqq_trig();

#10

Michael Paquier

michael.paquier@gmail.com

about 10 years ago

In reply to: Marko Tiikkaja (#9)

1 attachment(s)

Re: COPY (INSERT/UPDATE/DELETE .. RETURNING ..)

On Sun, Nov 22, 2015 at 9:54 AM, Marko Tiikkaja wrote:

Attached the test case I used; removing a batch of old rows from a table
through an index. Best out of three runs, I get 13.1 seconds versus 15.0
seconds, which should amount to an improvement of around 12.7%.

Thanks for the test case. I haven't seen that much, up to 5% on my
laptop still that's not bad thinking that it saves one execution step.

Also attached a v3 of the patch which adds an Assert on top of the test case
changes suggested by you.

Sorry for the late reply. I have been playing a bit more with this
patch and I am withdrawing my comments regarding the use of SelectStmt
here. I have looked at the patch again, noticing typos as well as
comments not updated, particularly for psql's \copy, resulting in the
attached.

Patch is switched to "ready for committer".
--
Michael

Attachments:

20151126_copydml_v4.patchapplication/x-patch; name=20151126_copydml_v4.patchDownload

diff --git a/doc/src/sgml/ref/copy.sgml b/doc/src/sgml/ref/copy.sgml
index 2850b47..07e2f45 100644
--- a/doc/src/sgml/ref/copy.sgml
+++ b/doc/src/sgml/ref/copy.sgml
@@ -112,10 +112,17 @@ COPY { <replaceable class="parameter">table_name</replaceable> [ ( <replaceable
     <term><replaceable class="parameter">query</replaceable></term>
     <listitem>
      <para>
-      A <xref linkend="sql-select"> or
-      <xref linkend="sql-values"> command
-      whose results are to be copied.
-      Note that parentheses are required around the query.
+      A <xref linkend="sql-select">, <xref linkend="sql-values">,
+      <xref linkend="sql-insert">, <xref linkend="sql-update"> or
+      <xref linkend="sql-delete"> command whose results are to be
+      copied.  Note that parentheses are required around the query.
+     </para>
+     <para>
+      For <command>INSERT</>, <command>UPDATE</> and
+      <command>DELETE</> queries a RETURNING clause must be provided,
+      and the target relation must not have a conditional rule, nor
+      an <literal>ALSO</> rule, nor an <literal>INSTEAD</> rule
+      that expands to multiple statements.
      </para>
     </listitem>
    </varlistentry>
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 47c6262..7dbe04e 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -201,7 +201,7 @@ typedef struct CopyStateData
 	int			raw_buf_len;	/* total # of bytes stored */
 } CopyStateData;
 
-/* DestReceiver for COPY (SELECT) TO */
+/* DestReceiver for COPY (query) TO */
 typedef struct
 {
 	DestReceiver pub;			/* publicly-known function pointers */
@@ -772,7 +772,8 @@ CopyLoadRawBuf(CopyState cstate)
  *
  * Either unload or reload contents of table <relation>, depending on <from>.
  * (<from> = TRUE means we are inserting into the table.)  In the "TO" case
- * we also support copying the output of an arbitrary SELECT query.
+ * we also support copying the output of an arbitrary SELECT, INSERT, UPDATE
+ * or DELETE query.
  *
  * If <pipe> is false, transfer is between the table and the file named
  * <filename>.  Otherwise, transfer is between the table and our regular
@@ -1374,11 +1375,11 @@ BeginCopy(bool is_from,
 		Assert(!is_from);
 		cstate->rel = NULL;
 
-		/* Don't allow COPY w/ OIDs from a select */
+		/* Don't allow COPY w/ OIDs from a query */
 		if (cstate->oids)
 			ereport(ERROR,
 					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-					 errmsg("COPY (SELECT) WITH OIDS is not supported")));
+					 errmsg("COPY (query) WITH OIDS is not supported")));
 
 		/*
 		 * Run parse analysis and rewrite.  Note this also acquires sufficient
@@ -1393,9 +1394,36 @@ BeginCopy(bool is_from,
 		rewritten = pg_analyze_and_rewrite((Node *) copyObject(raw_query),
 										   queryString, NULL, 0);
 
-		/* We don't expect more or less than one result query */
-		if (list_length(rewritten) != 1)
-			elog(ERROR, "unexpected rewrite result");
+		/* check that we got back something we can work with */
+		if (rewritten == NIL)
+		{
+			ereport(ERROR,
+					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+					errmsg("DO INSTEAD NOTHING rules are not supported for COPY")));
+		}
+		else if (list_length(rewritten) > 1)
+		{
+			ListCell *lc;
+
+			/* examine queries to determine which error message to issue */
+			foreach(lc, rewritten)
+			{
+				Query	  *q = (Query *) lfirst(lc);
+
+				if (q->querySource == QSRC_QUAL_INSTEAD_RULE)
+					ereport(ERROR,
+							(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+							 errmsg("conditional DO INSTEAD rules are not supported for COPY")));
+				if (q->querySource == QSRC_NON_INSTEAD_RULE)
+					ereport(ERROR,
+							(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+							 errmsg("DO ALSO rules are not supported for the COPY")));
+			}
+
+			ereport(ERROR,
+					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+					 errmsg("multi-statement DO INSTEAD rules are not supported for COPY")));
+		}
 
 		query = (Query *) linitial(rewritten);
 
@@ -1406,9 +1434,24 @@ BeginCopy(bool is_from,
 					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 					 errmsg("COPY (SELECT INTO) is not supported")));
 
-		Assert(query->commandType == CMD_SELECT);
 		Assert(query->utilityStmt == NULL);
 
+		/*
+		 * Similarly the grammar doesn't enforce the presence of a RETURNING
+		 * clause, but this is required here.
+		 */
+		if (query->commandType != CMD_SELECT &&
+			query->returningList == NIL)
+		{
+			Assert(query->commandType == CMD_INSERT ||
+				   query->commandType == CMD_UPDATE ||
+				   query->commandType == CMD_DELETE);
+
+			ereport(ERROR,
+						(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+						 errmsg("COPY query must have a RETURNING clause")));
+		}
+
 		/* plan the query */
 		plan = pg_plan_query(query, 0, NULL);
 
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index fba91d5..7916df8 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -2561,9 +2561,12 @@ ClosePortalStmt:
  *
  *		QUERY :
  *				COPY relname [(columnList)] FROM/TO file [WITH] [(options)]
- *				COPY ( SELECT ... ) TO file	[WITH] [(options)]
+ *				COPY ( query ) TO file	[WITH] [(options)]
  *
- *				where 'file' can be one of:
+ *				where 'query' can be one of:
+ *				{ SELECT | UPDATE | INSERT | DELETE }
+ *
+ *				and 'file' can be one of:
  *				{ PROGRAM 'command' | STDIN | STDOUT | 'filename' }
  *
  *				In the preferred syntax the options are comma-separated
@@ -2574,7 +2577,7 @@ ClosePortalStmt:
  *				COPY [ BINARY ] table [ WITH OIDS ] FROM/TO file
  *					[ [ USING ] DELIMITERS 'delimiter' ] ]
  *					[ WITH NULL AS 'null string' ]
- *				This option placement is not supported with COPY (SELECT...).
+ *				This option placement is not supported with COPY (query...).
  *
  *****************************************************************************/
 
@@ -2607,16 +2610,16 @@ CopyStmt:	COPY opt_binary qualified_name opt_column_list opt_oids
 						n->options = list_concat(n->options, $11);
 					$$ = (Node *)n;
 				}
-			| COPY select_with_parens TO opt_program copy_file_name opt_with copy_options
+			| COPY '(' PreparableStmt ')' TO opt_program copy_file_name opt_with copy_options
 				{
 					CopyStmt *n = makeNode(CopyStmt);
 					n->relation = NULL;
-					n->query = $2;
+					n->query = $3;
 					n->attlist = NIL;
 					n->is_from = false;
-					n->is_program = $4;
-					n->filename = $5;
-					n->options = $7;
+					n->is_program = $6;
+					n->filename = $7;
+					n->options = $9;
 
 					if (n->is_program && n->filename == NULL)
 						ereport(ERROR,
diff --git a/src/bin/psql/copy.c b/src/bin/psql/copy.c
index f1eb518..c0fc283 100644
--- a/src/bin/psql/copy.c
+++ b/src/bin/psql/copy.c
@@ -32,10 +32,12 @@
  *
  * The documented syntax is:
  *	\copy tablename [(columnlist)] from|to filename [options]
- *	\copy ( select stmt ) to filename [options]
+ *	\copy ( query stmt ) to filename [options]
  *
  * where 'filename' can be one of the following:
  *	'<file path>' | PROGRAM '<command>' | stdin | stdout | pstdout | pstdout
+ * and 'query' can be one of the following:
+ *  SELECT | UPDATE | INSERT | DELETE
  *
  * An undocumented fact is that you can still write BINARY before the
  * tablename; this is a hangover from the pre-7.3 syntax.  The options
@@ -118,7 +120,7 @@ parse_slash_copy(const char *args)
 			goto error;
 	}
 
-	/* Handle COPY (SELECT) case */
+	/* Handle COPY (query) case */
 	if (token[0] == '(')
 	{
 		int			parens = 1;
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 9e1c48d..9142e94 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -1680,7 +1680,8 @@ typedef struct CopyStmt
 {
 	NodeTag		type;
 	RangeVar   *relation;		/* the relation to copy */
-	Node	   *query;			/* the SELECT query to copy */
+	Node	   *query;			/* the query (SELECT or DML statement with
+								 * RETURNING) to copy */
 	List	   *attlist;		/* List of column names (as Strings), or NIL
 								 * for all columns */
 	bool		is_from;		/* TO or FROM */
diff --git a/src/test/regress/expected/copydml.out b/src/test/regress/expected/copydml.out
new file mode 100644
index 0000000..1b53396
--- /dev/null
+++ b/src/test/regress/expected/copydml.out
@@ -0,0 +1,112 @@
+--
+-- Test cases for COPY (INSERT/UPDATE/DELETE) TO
+--
+create table copydml_test (id serial, t text);
+insert into copydml_test (t) values ('a');
+insert into copydml_test (t) values ('b');
+insert into copydml_test (t) values ('c');
+insert into copydml_test (t) values ('d');
+insert into copydml_test (t) values ('e');
+--
+-- Test COPY (insert/update/delete ...)
+--
+copy (insert into copydml_test (t) values ('f') returning id) to stdout;
+6
+copy (update copydml_test set t = 'g' where t = 'f' returning id) to stdout;
+6
+copy (delete from copydml_test where t = 'g' returning id) to stdout;
+6
+--
+-- Test \copy (insert/update/delete ...)
+--
+\copy (insert into copydml_test (t) values ('f') returning id) to stdout;
+7
+\copy (update copydml_test set t = 'g' where t = 'f' returning id) to stdout;
+7
+\copy (delete from copydml_test where t = 'g' returning id) to stdout;
+7
+-- Error cases
+copy (insert into copydml_test default values) to stdout;
+ERROR:  COPY query must have a RETURNING clause
+copy (update copydml_test set t = 'g') to stdout;
+ERROR:  COPY query must have a RETURNING clause
+copy (delete from copydml_test) to stdout;
+ERROR:  COPY query must have a RETURNING clause
+create rule qqq as on insert to copydml_test do instead nothing;
+copy (insert into copydml_test default values) to stdout;
+ERROR:  DO INSTEAD NOTHING rules are not supported for COPY
+drop rule qqq on copydml_test;
+create rule qqq as on insert to copydml_test do also delete from copydml_test;
+copy (insert into copydml_test default values) to stdout;
+ERROR:  DO ALSO rules are not supported for the COPY
+drop rule qqq on copydml_test;
+create rule qqq as on insert to copydml_test do instead (delete from copydml_test; delete from copydml_test);
+copy (insert into copydml_test default values) to stdout;
+ERROR:  multi-statement DO INSTEAD rules are not supported for COPY
+drop rule qqq on copydml_test;
+create rule qqq as on insert to copydml_test where new.t <> 'f' do instead delete from copydml_test;
+copy (insert into copydml_test default values) to stdout;
+ERROR:  conditional DO INSTEAD rules are not supported for COPY
+drop rule qqq on copydml_test;
+create rule qqq as on update to copydml_test do instead nothing;
+copy (update copydml_test set t = 'f') to stdout;
+ERROR:  DO INSTEAD NOTHING rules are not supported for COPY
+drop rule qqq on copydml_test;
+create rule qqq as on update to copydml_test do also delete from copydml_test;
+copy (update copydml_test set t = 'f') to stdout;
+ERROR:  DO ALSO rules are not supported for the COPY
+drop rule qqq on copydml_test;
+create rule qqq as on update to copydml_test do instead (delete from copydml_test; delete from copydml_test);
+copy (update copydml_test set t = 'f') to stdout;
+ERROR:  multi-statement DO INSTEAD rules are not supported for COPY
+drop rule qqq on copydml_test;
+create rule qqq as on update to copydml_test where new.t <> 'f' do instead delete from copydml_test;
+copy (update copydml_test set t = 'f') to stdout;
+ERROR:  conditional DO INSTEAD rules are not supported for COPY
+drop rule qqq on copydml_test;
+create rule qqq as on delete to copydml_test do instead nothing;
+copy (delete from copydml_test) to stdout;
+ERROR:  DO INSTEAD NOTHING rules are not supported for COPY
+drop rule qqq on copydml_test;
+create rule qqq as on delete to copydml_test do also insert into copydml_test default values;
+copy (delete from copydml_test) to stdout;
+ERROR:  DO ALSO rules are not supported for the COPY
+drop rule qqq on copydml_test;
+create rule qqq as on delete to copydml_test do instead (insert into copydml_test default values; insert into copydml_test default values);
+copy (delete from copydml_test) to stdout;
+ERROR:  multi-statement DO INSTEAD rules are not supported for COPY
+drop rule qqq on copydml_test;
+create rule qqq as on delete to copydml_test where old.t <> 'f' do instead insert into copydml_test default values;
+copy (delete from copydml_test) to stdout;
+ERROR:  conditional DO INSTEAD rules are not supported for COPY
+drop rule qqq on copydml_test;
+-- triggers
+create function qqq_trig() returns trigger as $$
+begin
+if tg_op in ('INSERT', 'UPDATE') then
+    raise notice '% %', tg_op, new.id;
+    return new;
+else
+    raise notice '% %', tg_op, old.id;
+    return old;
+end if;
+end
+$$ language plpgsql;
+create trigger qqqbef before insert or update or delete on copydml_test
+    for each row execute procedure qqq_trig();
+create trigger qqqaf after insert or update or delete on copydml_test
+    for each row execute procedure qqq_trig();
+copy (insert into copydml_test (t) values ('f') returning id) to stdout;
+NOTICE:  INSERT 8
+8
+NOTICE:  INSERT 8
+copy (update copydml_test set t = 'g' where t = 'f' returning id) to stdout;
+NOTICE:  UPDATE 8
+8
+NOTICE:  UPDATE 8
+copy (delete from copydml_test where t = 'g' returning id) to stdout;
+NOTICE:  DELETE 8
+8
+NOTICE:  DELETE 8
+drop table copydml_test;
+drop function qqq_trig();
diff --git a/src/test/regress/parallel_schedule b/src/test/regress/parallel_schedule
index 3987b4c..b1bc7c7 100644
--- a/src/test/regress/parallel_schedule
+++ b/src/test/regress/parallel_schedule
@@ -48,7 +48,7 @@ test: create_function_2
 # execute two copy tests parallel, to check that copy itself
 # is concurrent safe.
 # ----------
-test: copy copyselect
+test: copy copyselect copydml
 
 # ----------
 # More groups of parallel tests
diff --git a/src/test/regress/serial_schedule b/src/test/regress/serial_schedule
index 379f272..ade9ef1 100644
--- a/src/test/regress/serial_schedule
+++ b/src/test/regress/serial_schedule
@@ -57,6 +57,7 @@ test: create_table
 test: create_function_2
 test: copy
 test: copyselect
+test: copydml
 test: create_misc
 test: create_operator
 test: create_index
diff --git a/src/test/regress/sql/copydml.sql b/src/test/regress/sql/copydml.sql
new file mode 100644
index 0000000..9a29f9c
--- /dev/null
+++ b/src/test/regress/sql/copydml.sql
@@ -0,0 +1,91 @@
+--
+-- Test cases for COPY (INSERT/UPDATE/DELETE) TO
+--
+create table copydml_test (id serial, t text);
+insert into copydml_test (t) values ('a');
+insert into copydml_test (t) values ('b');
+insert into copydml_test (t) values ('c');
+insert into copydml_test (t) values ('d');
+insert into copydml_test (t) values ('e');
+
+--
+-- Test COPY (insert/update/delete ...)
+--
+copy (insert into copydml_test (t) values ('f') returning id) to stdout;
+copy (update copydml_test set t = 'g' where t = 'f' returning id) to stdout;
+copy (delete from copydml_test where t = 'g' returning id) to stdout;
+
+--
+-- Test \copy (insert/update/delete ...)
+--
+\copy (insert into copydml_test (t) values ('f') returning id) to stdout;
+\copy (update copydml_test set t = 'g' where t = 'f' returning id) to stdout;
+\copy (delete from copydml_test where t = 'g' returning id) to stdout;
+
+-- Error cases
+copy (insert into copydml_test default values) to stdout;
+copy (update copydml_test set t = 'g') to stdout;
+copy (delete from copydml_test) to stdout;
+
+create rule qqq as on insert to copydml_test do instead nothing;
+copy (insert into copydml_test default values) to stdout;
+drop rule qqq on copydml_test;
+create rule qqq as on insert to copydml_test do also delete from copydml_test;
+copy (insert into copydml_test default values) to stdout;
+drop rule qqq on copydml_test;
+create rule qqq as on insert to copydml_test do instead (delete from copydml_test; delete from copydml_test);
+copy (insert into copydml_test default values) to stdout;
+drop rule qqq on copydml_test;
+create rule qqq as on insert to copydml_test where new.t <> 'f' do instead delete from copydml_test;
+copy (insert into copydml_test default values) to stdout;
+drop rule qqq on copydml_test;
+
+create rule qqq as on update to copydml_test do instead nothing;
+copy (update copydml_test set t = 'f') to stdout;
+drop rule qqq on copydml_test;
+create rule qqq as on update to copydml_test do also delete from copydml_test;
+copy (update copydml_test set t = 'f') to stdout;
+drop rule qqq on copydml_test;
+create rule qqq as on update to copydml_test do instead (delete from copydml_test; delete from copydml_test);
+copy (update copydml_test set t = 'f') to stdout;
+drop rule qqq on copydml_test;
+create rule qqq as on update to copydml_test where new.t <> 'f' do instead delete from copydml_test;
+copy (update copydml_test set t = 'f') to stdout;
+drop rule qqq on copydml_test;
+
+create rule qqq as on delete to copydml_test do instead nothing;
+copy (delete from copydml_test) to stdout;
+drop rule qqq on copydml_test;
+create rule qqq as on delete to copydml_test do also insert into copydml_test default values;
+copy (delete from copydml_test) to stdout;
+drop rule qqq on copydml_test;
+create rule qqq as on delete to copydml_test do instead (insert into copydml_test default values; insert into copydml_test default values);
+copy (delete from copydml_test) to stdout;
+drop rule qqq on copydml_test;
+create rule qqq as on delete to copydml_test where old.t <> 'f' do instead insert into copydml_test default values;
+copy (delete from copydml_test) to stdout;
+drop rule qqq on copydml_test;
+
+-- triggers
+create function qqq_trig() returns trigger as $$
+begin
+if tg_op in ('INSERT', 'UPDATE') then
+    raise notice '% %', tg_op, new.id;
+    return new;
+else
+    raise notice '% %', tg_op, old.id;
+    return old;
+end if;
+end
+$$ language plpgsql;
+create trigger qqqbef before insert or update or delete on copydml_test
+    for each row execute procedure qqq_trig();
+create trigger qqqaf after insert or update or delete on copydml_test
+    for each row execute procedure qqq_trig();
+
+copy (insert into copydml_test (t) values ('f') returning id) to stdout;
+copy (update copydml_test set t = 'g' where t = 'f' returning id) to stdout;
+copy (delete from copydml_test where t = 'g' returning id) to stdout;
+
+drop table copydml_test;
+drop function qqq_trig();

#11

Teodor Sigaev

teodor@sigaev.ru

about 10 years ago

In reply to: Michael Paquier (#10)

Re: COPY (INSERT/UPDATE/DELETE .. RETURNING ..)

Patch is switched to "ready for committer".

Committed, thank you
--
Teodor Sigaev E-mail: teodor@sigaev.ru
WWW: http://www.sigaev.ru/

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#12

Michael Paquier

michael.paquier@gmail.com

about 10 years ago

In reply to: Teodor Sigaev (#11)

Re: COPY (INSERT/UPDATE/DELETE .. RETURNING ..)

On Sat, Nov 28, 2015 at 1:15 AM, Teodor Sigaev <teodor@sigaev.ru> wrote:

Patch is switched to "ready for committer".

Committed, thank you

Thanks.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers