Re: [HACKERS] COPY (INSERT/UPDATE/DELETE .. RETURNING ..)

[Michael Paquier]

On Sat, Nov 28, 2015 at 1:15 AM, Teodor Sigaev  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

Re: [HACKERS] COPY (INSERT/UPDATE/DELETE .. RETURNING ..)

[Teodor Sigaev]

Patch is switched to "ready for committer".

Committed, thank you
--
Teodor Sigaev   E-mail: teo...@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

Re: [HACKERS] COPY (INSERT/UPDATE/DELETE .. RETURNING ..)

[Michael Paquier]

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
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 { table_name [ ( query
 
  
-  A  or
-   command
-  whose results are to be copied.
-  Note that parentheses are required around the query.
+  A , ,
+  ,  or
+   command whose results are to be
+  copied.  Note that parentheses are required around the query.
+ 
+ 
+  For INSERT, UPDATE and
+  DELETE queries a RETURNING clause must be provided,
+  and the target relation must not have a conditional rule, nor
+  an ALSO rule, nor an INSTEAD rule
+  that expands to multiple statements.
  
 

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 , depending on .
  * ( = 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  is false, transfer is between the table and the file named
  * .  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,
+		

Re: [HACKERS] COPY (INSERT/UPDATE/DELETE .. RETURNING ..)

[Marko Tiikkaja]

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

On Sun, Nov 1, 2015 at 2:49 AM, Marko Tiikkaja  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.



.m
drop table if exists tbl;
create table tbl(a serial, b int not null, c text not null);
insert into tbl (b,c) select i, random()::text from generate_series(1, 
1) i;
alter table tbl add primary key (a);
analyze tbl;
checkpoint;
\timing

-- new code
--\copy (delete from tbl where a <= 500 returning *) to foo.dat

-- old code
--SET work_mem TO '1GB';
--\copy (with t as (delete from tbl where a <= 500 returning *) select * 
from t) to foo.dat
*** a/doc/src/sgml/ref/copy.sgml
--- b/doc/src/sgml/ref/copy.sgml
***
*** 112,121  COPY { table_name [ ( query
  
   
!   A  or
!command
!   whose results are to be copied.
!   Note that parentheses are required around the query.
   
  
 
--- 112,128 
  query
  
   
!   A , ,
!   ,  or
!command whose results are to be
!   copied.  Note that parentheses are required around the query.
!  
!  
!   For INSERT, UPDATE and
!   DELETE queries a RETURNING clause must be provided,
!   and the target relation must not have a conditional rule, nor
!   an ALSO rule, nor an INSTEAD rule
!   that expands to multiple statements.
   
  
 
*** 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 , depending on .
   * ( = TRUE means we are inserting into the table.)  In the "TO" case
!  * we also support copying the output of an arbitrary SELECT query.
   *
   * If  is false, transfer is between the table and the file named
   * .  Otherwise, transfer is between the table and our regular
--- 772,779 
   *
   * Either unload or reload contents of table , depending on .
   * ( = 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  is false, transfer is between the table and the file named
   * .  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 */
! 			

Re: [HACKERS] COPY (INSERT/UPDATE/DELETE .. RETURNING ..)

[Marko Tiikkaja]

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

On Thu, Nov 19, 2015 at 9:22 PM, Marko Tiikkaja  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.



.m


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

Re: [HACKERS] COPY (INSERT/UPDATE/DELETE .. RETURNING ..)

[Michael Paquier]

On Thu, Nov 19, 2015 at 9:22 PM, Marko Tiikkaja  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

Re: [HACKERS] COPY (INSERT/UPDATE/DELETE .. RETURNING ..)

[Michael Paquier]

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

Re: [HACKERS] COPY (INSERT/UPDATE/DELETE .. RETURNING ..)

[Marko Tiikkaja]

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.



.m


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

Re: [HACKERS] COPY (INSERT/UPDATE/DELETE .. RETURNING ..)

[Marko Tiikkaja]

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

On Sun, Nov 1, 2015 at 2:49 AM, Marko Tiikkaja  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.



.m


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

Re: [HACKERS] COPY (INSERT/UPDATE/DELETE .. RETURNING ..)

[Michael Paquier]

On Sun, Nov 1, 2015 at 2:49 AM, Marko Tiikkaja  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
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 { table_name [ ( query
 
  
-  A  or
-   command
-  whose results are to be copied.
-  Note that parentheses are required around the query.
+  A , ,
+  ,  or
+   command whose results are to be
+  copied.  Note that parentheses are required around the query.
+ 
+ 
+  For INSERT, UPDATE and
+  DELETE queries a RETURNING clause must be provided,
+  and the target relation must not have a conditional rule, nor
+  an ALSO rule, nor an INSTEAD rule
+  that expands to multiple statements.
  
 

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 , depending on .
  * ( = 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  is false, transfer is between the table and the file named
  * .  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)
+	

Re: [HACKERS] COPY (INSERT/UPDATE/DELETE .. RETURNING ..)

[Michael Paquier]

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

[HACKERS] COPY (INSERT/UPDATE/DELETE .. RETURNING ..)

[Marko Tiikkaja]

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.



.m
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 { table_name [ ( query
 
  
-  A  or
-   command
-  whose results are to be copied.
-  Note that parentheses are required around the query.
+  A , ,
+  ,  or
+   command whose results are to be
+  copied.  Note that parentheses are required around the query.
+ 
+ 
+  For INSERT, UPDATE and
+  DELETE queries a RETURNING clause must be provided,
+  and the target relation must not have a conditional rule, nor
+  an ALSO rule, nor an INSTEAD rule
+  that expands to multiple statements.
  
 

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 , depending on .
  * ( = 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  is false, transfer is between the table and the file named
  * .  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 (