On 2015-11-16 08:24, Michael Paquier wrote:
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?
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,
100000000) i;
alter table tbl add primary key (a);
analyze tbl;
checkpoint;
\timing
-- new code
--\copy (delete from tbl where a <= 5000000 returning *) to foo.dat
-- old code
--SET work_mem TO '1GB';
--\copy (with t as (delete from tbl where a <= 5000000 returning *) select *
from t) to foo.dat
*** 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();
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
