On Sun, Nov 1, 2015 at 2:49 AM, Marko Tiikkaja <ma...@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
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();
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
