pgsql: Add support for INSERT ... ON CONFLICT DO NOTHING/UPDATE.
Add support for INSERT ... ON CONFLICT DO NOTHING/UPDATE.
The newly added ON CONFLICT clause allows to specify an alternative to
raising a unique or exclusion constraint violation error when inserting.
ON CONFLICT refers to constraints that can either be specified using a
inference clause (by specifying the columns of a unique constraint) or
by naming a unique or exclusion constraint. DO NOTHING avoids the
constraint violation, without touching the pre-existing row. DO UPDATE
SET ... [WHERE ...] updates the pre-existing tuple, and has access to
both the tuple proposed for insertion and the existing tuple; the
optional WHERE clause can be used to prevent an update from being
executed. The UPDATE SET and WHERE clauses have access to the tuple
proposed for insertion using the "magic" EXCLUDED alias, and to the
pre-existing tuple using the table name or its alias.
This feature is often referred to as upsert.
This is implemented using a new infrastructure called "speculative
insertion". It is an optimistic variant of regular insertion that first
does a pre-check for existing tuples and then attempts an insert. If a
violating tuple was inserted concurrently, the speculatively inserted
tuple is deleted and a new attempt is made. If the pre-check finds a
matching tuple the alternative DO NOTHING or DO UPDATE action is taken.
If the insertion succeeds without detecting a conflict, the tuple is
deemed inserted.
To handle the possible ambiguity between the excluded alias and a table
named excluded, and for convenience with long relation names, INSERT
INTO now can alias its target table.
Bumps catversion as stored rules change.
Author: Peter Geoghegan, with significant contributions from Heikki
Linnakangas and Andres Freund. Testing infrastructure by Jeff Janes.
Reviewed-By: Heikki Linnakangas, Andres Freund, Robert Haas, Simon Riggs,
Dean Rasheed, Stephen Frost and many others.
Branch
------
master
Details
-------
http://git.postgresql.org/pg/commitdiff/168d5805e4c08bed7b95d351bf097cff7c07dd65
Modified Files
--------------
contrib/pg_stat_statements/pg_stat_statements.c | 25 +
contrib/postgres_fdw/deparse.c | 7 +-
contrib/postgres_fdw/expected/postgres_fdw.out | 5 +
contrib/postgres_fdw/postgres_fdw.c | 15 +-
contrib/postgres_fdw/postgres_fdw.h | 2 +-
contrib/postgres_fdw/sql/postgres_fdw.sql | 3 +
contrib/test_decoding/expected/ddl.out | 34 ++
contrib/test_decoding/expected/toast.out | 9 +-
contrib/test_decoding/sql/ddl.sql | 22 +
contrib/test_decoding/sql/toast.sql | 5 +
doc/src/sgml/fdwhandler.sgml | 7 +
doc/src/sgml/keywords.sgml | 7 +
doc/src/sgml/mvcc.sgml | 23 +-
doc/src/sgml/plpgsql.sgml | 14 +-
doc/src/sgml/postgres-fdw.sgml | 8 +
doc/src/sgml/protocol.sgml | 13 +-
doc/src/sgml/ref/create_policy.sgml | 63 ++-
doc/src/sgml/ref/create_rule.sgml | 6 +-
doc/src/sgml/ref/create_table.sgml | 4 +-
doc/src/sgml/ref/create_trigger.sgml | 5 +-
doc/src/sgml/ref/create_view.sgml | 9 +-
doc/src/sgml/ref/insert.sgml | 403 ++++++++++++++++-
doc/src/sgml/trigger.sgml | 48 +-
src/backend/access/heap/heapam.c | 377 ++++++++++++++--
src/backend/access/heap/hio.c | 27 +-
src/backend/access/heap/tuptoaster.c | 8 +
src/backend/access/nbtree/nbtinsert.c | 28 +-
src/backend/access/rmgrdesc/heapdesc.c | 9 +
src/backend/catalog/index.c | 53 ++-
src/backend/catalog/indexing.c | 2 +-
src/backend/catalog/sql_features.txt | 2 +-
src/backend/commands/constraint.c | 2 +-
src/backend/commands/copy.c | 7 +-
src/backend/commands/explain.c | 70 ++-
src/backend/commands/trigger.c | 19 +-
src/backend/executor/execIndexing.c | 417 ++++++++++++++---
src/backend/executor/execMain.c | 53 ++-
src/backend/executor/nodeLockRows.c | 12 +-
src/backend/executor/nodeModifyTable.c | 459 ++++++++++++++++++-
src/backend/nodes/copyfuncs.c | 84 ++++
src/backend/nodes/equalfuncs.c | 62 +++
src/backend/nodes/nodeFuncs.c | 87 ++++
src/backend/nodes/outfuncs.c | 41 +-
src/backend/nodes/readfuncs.c | 40 ++
src/backend/optimizer/plan/createplan.c | 26 +-
src/backend/optimizer/plan/planner.c | 27 ++
src/backend/optimizer/plan/setrefs.c | 52 ++-
src/backend/optimizer/plan/subselect.c | 4 +
src/backend/optimizer/prep/prepjointree.c | 6 +
src/backend/optimizer/prep/preptlist.c | 13 +
src/backend/optimizer/util/plancat.c | 352 +++++++++++++++
src/backend/parser/analyze.c | 149 +++++-
src/backend/parser/gram.y | 121 ++++-
src/backend/parser/parse_clause.c | 203 +++++++++
src/backend/parser/parse_collate.c | 2 +
src/backend/parser/parse_target.c | 11 +-
src/backend/replication/logical/decode.c | 66 ++-
src/backend/replication/logical/reorderbuffer.c | 159 +++++--
src/backend/rewrite/rewriteHandler.c | 87 +++-
src/backend/rewrite/rowsecurity.c | 82 +++-
src/backend/storage/lmgr/lmgr.c | 91 ++++
src/backend/tcop/pquery.c | 17 +-
src/backend/utils/adt/lockfuncs.c | 1 +
src/backend/utils/adt/ruleutils.c | 108 +++--
src/backend/utils/time/tqual.c | 29 +-
src/bin/psql/common.c | 5 +-
src/include/access/heapam.h | 3 +
src/include/access/heapam_xlog.h | 54 ++-
src/include/access/hio.h | 2 +-
src/include/access/htup_details.h | 36 +-
src/include/catalog/catversion.h | 2 +-
src/include/catalog/index.h | 2 +
src/include/executor/executor.h | 13 +-
src/include/nodes/execnodes.h | 15 +
src/include/nodes/nodes.h | 17 +
src/include/nodes/parsenodes.h | 45 +-
src/include/nodes/plannodes.h | 8 +
src/include/nodes/primnodes.h | 42 ++
src/include/optimizer/plancat.h | 2 +
src/include/optimizer/planmain.h | 2 +-
src/include/optimizer/prep.h | 3 +
src/include/parser/kwlist.h | 1 +
src/include/parser/parse_clause.h | 4 +
src/include/replication/reorderbuffer.h | 9 +-
src/include/rewrite/rowsecurity.h | 3 +-
src/include/storage/lmgr.h | 5 +
src/include/storage/lock.h | 10 +
src/include/utils/snapshot.h | 22 +-
.../expected/insert-conflict-do-nothing.out | 23 +
.../expected/insert-conflict-do-update-2.out | 23 +
.../expected/insert-conflict-do-update-3.out | 26 ++
.../expected/insert-conflict-do-update.out | 23 +
src/test/isolation/isolation_schedule | 4 +
.../specs/insert-conflict-do-nothing.spec | 41 ++
.../specs/insert-conflict-do-update-2.spec | 41 ++
.../specs/insert-conflict-do-update-3.spec | 69 +++
.../isolation/specs/insert-conflict-do-update.spec | 40 ++
src/test/regress/expected/errors.out | 4 +-
src/test/regress/expected/insert_conflict.out | 476 ++++++++++++++++++++
src/test/regress/expected/privileges.out | 29 +-
src/test/regress/expected/returning.out | 24 +
src/test/regress/expected/rowsecurity.out | 132 ++++++
src/test/regress/expected/rules.out | 90 ++++
src/test/regress/expected/subselect.out | 22 +
src/test/regress/expected/triggers.out | 102 ++++-
src/test/regress/expected/updatable_views.out | 61 +++
src/test/regress/expected/update.out | 34 ++
src/test/regress/expected/with.out | 82 ++++
src/test/regress/input/constraints.source | 12 +
src/test/regress/output/constraints.source | 24 +-
src/test/regress/parallel_schedule | 1 +
src/test/regress/serial_schedule | 1 +
src/test/regress/sql/insert_conflict.sql | 284 ++++++++++++
src/test/regress/sql/privileges.sql | 19 +-
src/test/regress/sql/returning.sql | 6 +
src/test/regress/sql/rowsecurity.sql | 112 +++++
src/test/regress/sql/rules.sql | 59 +++
src/test/regress/sql/subselect.sql | 14 +
src/test/regress/sql/triggers.sql | 69 ++-
src/test/regress/sql/updatable_views.sql | 9 +
src/test/regress/sql/update.sql | 21 +
src/test/regress/sql/with.sql | 57 +++
122 files changed, 6106 insertions(+), 435 deletions(-)
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
On Fri, May 8, 2015 at 12:43 PM, Andres Freund <andres@anarazel.de> wrote:
Add support for INSERT ... ON CONFLICT DO NOTHING/UPDATE.
magpie is complaining:
http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=magpie&dt=2015-05-08%2003%3A45%3A15
And I can reproduce the failure as well on my OSX laptop at least.
--
Michael
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
On May 7, 2015 9:07:55 PM PDT, Michael Paquier <michael.paquier@gmail.com> wrote:
On Fri, May 8, 2015 at 12:43 PM, Andres Freund <andres@anarazel.de>
wrote:Add support for INSERT ... ON CONFLICT DO NOTHING/UPDATE.
magpie is complaining:
http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=magpie&dt=2015-05-08%2003%3A45%3A15
And I can reproduce the failure as well on my OSX laptop at least.
Pushed something that hopefully fixes that already. Works for you?
Andres
---
Please excuse brevity and formatting - I am writing this on my mobile phone.
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
On Fri, May 8, 2015 at 1:09 PM, Andres Freund <andres@anarazel.de> wrote:
On May 7, 2015 9:07:55 PM PDT, Michael Paquier <michael.paquier@gmail.com> wrote:
On Fri, May 8, 2015 at 12:43 PM, Andres Freund <andres@anarazel.de>
wrote:Add support for INSERT ... ON CONFLICT DO NOTHING/UPDATE.
magpie is complaining:
http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=magpie&dt=2015-05-08%2003%3A45%3A15
And I can reproduce the failure as well on my OSX laptop at least.Pushed something that hopefully fixes that already. Works for you?
That's fine. Thanks for the quick fix.
--
Michael
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
On 8 May 2015 at 04:43, Andres Freund <andres@anarazel.de> wrote:
Add support for INSERT ... ON CONFLICT DO NOTHING/UPDATE.
The newly added ON CONFLICT clause allows to specify an alternative to
raising a unique or exclusion constraint violation error when inserting.
ON CONFLICT refers to constraints that can either be specified using a
inference clause (by specifying the columns of a unique constraint) or
by naming a unique or exclusion constraint. DO NOTHING avoids the
constraint violation, without touching the pre-existing row. DO UPDATE
SET ... [WHERE ...] updates the pre-existing tuple, and has access to
both the tuple proposed for insertion and the existing tuple; the
optional WHERE clause can be used to prevent an update from being
executed. The UPDATE SET and WHERE clauses have access to the tuple
proposed for insertion using the "magic" EXCLUDED alias, and to the
pre-existing tuple using the table name or its alias.This feature is often referred to as upsert.
This is implemented using a new infrastructure called "speculative
insertion". It is an optimistic variant of regular insertion that first
does a pre-check for existing tuples and then attempts an insert. If a
violating tuple was inserted concurrently, the speculatively inserted
tuple is deleted and a new attempt is made. If the pre-check finds a
matching tuple the alternative DO NOTHING or DO UPDATE action is taken.
If the insertion succeeds without detecting a conflict, the tuple is
deemed inserted.To handle the possible ambiguity between the excluded alias and a table
named excluded, and for convenience with long relation names, INSERT
INTO now can alias its target table.Bumps catversion as stored rules change.
Author: Peter Geoghegan, with significant contributions from Heikki
Linnakangas and Andres Freund. Testing infrastructure by Jeff Janes.
Reviewed-By: Heikki Linnakangas, Andres Freund, Robert Haas, Simon Riggs,
Dean Rasheed, Stephen Frost and many others.Branch
------
masterDetails
-------
http://git.postgresql.org/pg/commitdiff/168d5805e4c08bed7b95d351bf097cff7c07dd65Modified Files
--------------
contrib/pg_stat_statements/pg_stat_statements.c | 25 +
contrib/postgres_fdw/deparse.c | 7 +-
contrib/postgres_fdw/expected/postgres_fdw.out | 5 +
contrib/postgres_fdw/postgres_fdw.c | 15 +-
contrib/postgres_fdw/postgres_fdw.h | 2 +-
contrib/postgres_fdw/sql/postgres_fdw.sql | 3 +
contrib/test_decoding/expected/ddl.out | 34 ++
contrib/test_decoding/expected/toast.out | 9 +-
contrib/test_decoding/sql/ddl.sql | 22 +
contrib/test_decoding/sql/toast.sql | 5 +
doc/src/sgml/fdwhandler.sgml | 7 +
doc/src/sgml/keywords.sgml | 7 +
doc/src/sgml/mvcc.sgml | 23 +-
doc/src/sgml/plpgsql.sgml | 14 +-
doc/src/sgml/postgres-fdw.sgml | 8 +
doc/src/sgml/protocol.sgml | 13 +-
doc/src/sgml/ref/create_policy.sgml | 63 ++-
doc/src/sgml/ref/create_rule.sgml | 6 +-
doc/src/sgml/ref/create_table.sgml | 4 +-
doc/src/sgml/ref/create_trigger.sgml | 5 +-
doc/src/sgml/ref/create_view.sgml | 9 +-
doc/src/sgml/ref/insert.sgml | 403 ++++++++++++++++-
doc/src/sgml/trigger.sgml | 48 +-
src/backend/access/heap/heapam.c | 377 ++++++++++++++--
src/backend/access/heap/hio.c | 27 +-
src/backend/access/heap/tuptoaster.c | 8 +
src/backend/access/nbtree/nbtinsert.c | 28 +-
src/backend/access/rmgrdesc/heapdesc.c | 9 +
src/backend/catalog/index.c | 53 ++-
src/backend/catalog/indexing.c | 2 +-
src/backend/catalog/sql_features.txt | 2 +-
src/backend/commands/constraint.c | 2 +-
src/backend/commands/copy.c | 7 +-
src/backend/commands/explain.c | 70 ++-
src/backend/commands/trigger.c | 19 +-
src/backend/executor/execIndexing.c | 417 ++++++++++++++---
src/backend/executor/execMain.c | 53 ++-
src/backend/executor/nodeLockRows.c | 12 +-
src/backend/executor/nodeModifyTable.c | 459 ++++++++++++++++++-
src/backend/nodes/copyfuncs.c | 84 ++++
src/backend/nodes/equalfuncs.c | 62 +++
src/backend/nodes/nodeFuncs.c | 87 ++++
src/backend/nodes/outfuncs.c | 41 +-
src/backend/nodes/readfuncs.c | 40 ++
src/backend/optimizer/plan/createplan.c | 26 +-
src/backend/optimizer/plan/planner.c | 27 ++
src/backend/optimizer/plan/setrefs.c | 52 ++-
src/backend/optimizer/plan/subselect.c | 4 +
src/backend/optimizer/prep/prepjointree.c | 6 +
src/backend/optimizer/prep/preptlist.c | 13 +
src/backend/optimizer/util/plancat.c | 352 +++++++++++++++
src/backend/parser/analyze.c | 149 +++++-
src/backend/parser/gram.y | 121 ++++-
src/backend/parser/parse_clause.c | 203 +++++++++
src/backend/parser/parse_collate.c | 2 +
src/backend/parser/parse_target.c | 11 +-
src/backend/replication/logical/decode.c | 66 ++-
src/backend/replication/logical/reorderbuffer.c | 159 +++++--
src/backend/rewrite/rewriteHandler.c | 87 +++-
src/backend/rewrite/rowsecurity.c | 82 +++-
src/backend/storage/lmgr/lmgr.c | 91 ++++
src/backend/tcop/pquery.c | 17 +-
src/backend/utils/adt/lockfuncs.c | 1 +
src/backend/utils/adt/ruleutils.c | 108 +++--
src/backend/utils/time/tqual.c | 29 +-
src/bin/psql/common.c | 5 +-
src/include/access/heapam.h | 3 +
src/include/access/heapam_xlog.h | 54 ++-
src/include/access/hio.h | 2 +-
src/include/access/htup_details.h | 36 +-
src/include/catalog/catversion.h | 2 +-
src/include/catalog/index.h | 2 +
src/include/executor/executor.h | 13 +-
src/include/nodes/execnodes.h | 15 +
src/include/nodes/nodes.h | 17 +
src/include/nodes/parsenodes.h | 45 +-
src/include/nodes/plannodes.h | 8 +
src/include/nodes/primnodes.h | 42 ++
src/include/optimizer/plancat.h | 2 +
src/include/optimizer/planmain.h | 2 +-
src/include/optimizer/prep.h | 3 +
src/include/parser/kwlist.h | 1 +
src/include/parser/parse_clause.h | 4 +
src/include/replication/reorderbuffer.h | 9 +-
src/include/rewrite/rowsecurity.h | 3 +-
src/include/storage/lmgr.h | 5 +
src/include/storage/lock.h | 10 +
src/include/utils/snapshot.h | 22 +-
.../expected/insert-conflict-do-nothing.out | 23 +
.../expected/insert-conflict-do-update-2.out | 23 +
.../expected/insert-conflict-do-update-3.out | 26 ++
.../expected/insert-conflict-do-update.out | 23 +
src/test/isolation/isolation_schedule | 4 +
.../specs/insert-conflict-do-nothing.spec | 41 ++
.../specs/insert-conflict-do-update-2.spec | 41 ++
.../specs/insert-conflict-do-update-3.spec | 69 +++
.../isolation/specs/insert-conflict-do-update.spec | 40 ++
src/test/regress/expected/errors.out | 4 +-
src/test/regress/expected/insert_conflict.out | 476 ++++++++++++++++++++
src/test/regress/expected/privileges.out | 29 +-
src/test/regress/expected/returning.out | 24 +
src/test/regress/expected/rowsecurity.out | 132 ++++++
src/test/regress/expected/rules.out | 90 ++++
src/test/regress/expected/subselect.out | 22 +
src/test/regress/expected/triggers.out | 102 ++++-
src/test/regress/expected/updatable_views.out | 61 +++
src/test/regress/expected/update.out | 34 ++
src/test/regress/expected/with.out | 82 ++++
src/test/regress/input/constraints.source | 12 +
src/test/regress/output/constraints.source | 24 +-
src/test/regress/parallel_schedule | 1 +
src/test/regress/serial_schedule | 1 +
src/test/regress/sql/insert_conflict.sql | 284 ++++++++++++
src/test/regress/sql/privileges.sql | 19 +-
src/test/regress/sql/returning.sql | 6 +
src/test/regress/sql/rowsecurity.sql | 112 +++++
src/test/regress/sql/rules.sql | 59 +++
src/test/regress/sql/subselect.sql | 14 +
src/test/regress/sql/triggers.sql | 69 ++-
src/test/regress/sql/updatable_views.sql | 9 +
src/test/regress/sql/update.sql | 21 +
src/test/regress/sql/with.sql | 57 +++
122 files changed, 6106 insertions(+), 435 deletions(-)
I haven't had time for a proper read of this patch, but I did
immediately notice this:
HINT: For example, ON CONFLICT ON CONFLICT (<column>).
This should perhaps either be:
HINT: For example, ON CONFLICT (<column>).
or
HINT: For example, ON CONFLICT ON CONSTRAINT <constraint_name>.
But at the moment it seems to be neither.
--
Thom
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
On Mon, May 11, 2015 at 1:49 AM, Thom Brown <thom@linux.com> wrote:
I haven't had time for a proper read of this patch, but I did
immediately notice this:HINT: For example, ON CONFLICT ON CONFLICT (<column>).
This should perhaps either be:
HINT: For example, ON CONFLICT (<column>).
or
HINT: For example, ON CONFLICT ON CONSTRAINT <constraint_name>.
But at the moment it seems to be neither.
What I'd intended here was the first suggestion of yours. Initially,
it was actually a combination of Thom's two suggestions, but this was
messed up at some point.
What I suggest is that this be changed to match the first suggestion
here (the intended message), since the "ON CONSTRAINT ... " variant is
really just an escape hatch that I don't expect will see much use. I
tried to encourage use of the conventional inference mechanism
everywhere.
--
Peter Geoghegan
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
Andres Freund wrote:
Add support for INSERT ... ON CONFLICT DO NOTHING/UPDATE.
Hm, I just realized that the command tag for INSERT ON CONFLICT is still
just INSERT. Is that okay? To me, the behavior is different enough
that it should have its own tag. I'm not too set on this, but maybe
others share this opinion?
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2015-05-20 18:58:16 -0300, Alvaro Herrera wrote:
Andres Freund wrote:
Add support for INSERT ... ON CONFLICT DO NOTHING/UPDATE.
Hm, I just realized that the command tag for INSERT ON CONFLICT is still
just INSERT. Is that okay? To me, the behavior is different enough
that it should have its own tag. I'm not too set on this, but maybe
others share this opinion?
It's actually INSERT for DO NOTHING, and UPSERT for DO UPDATE. It's even
documented ;)
Andres
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, May 20, 2015 at 2:58 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
Hm, I just realized that the command tag for INSERT ON CONFLICT is still
just INSERT. Is that okay? To me, the behavior is different enough
that it should have its own tag. I'm not too set on this, but maybe
others share this opinion?
No it isn't. ON CONFLICT DO UPDATE has an "UPSERT" command tag. DO
NOTHING has INSERT as its command tag.
Are you using an old psql? I thought that that would just result in no
command tag being displayed.
--
Peter Geoghegan
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Peter Geoghegan wrote:
On Wed, May 20, 2015 at 2:58 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:Hm, I just realized that the command tag for INSERT ON CONFLICT is still
just INSERT. Is that okay? To me, the behavior is different enough
that it should have its own tag. I'm not too set on this, but maybe
others share this opinion?No it isn't. ON CONFLICT DO UPDATE has an "UPSERT" command tag. DO
NOTHING has INSERT as its command tag.
... ah ...
Are you using an old psql? I thought that that would just result in no
command tag being displayed.
Well, I'm using an editor to read the code of CreateCommandTag(), not
executing anything. I guess that function needs an update, then, no?
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, May 20, 2015 at 3:09 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
Are you using an old psql? I thought that that would just result in no
command tag being displayed.Well, I'm using an editor to read the code of CreateCommandTag(), not
executing anything. I guess that function needs an update, then, no?
I think you're right. The initial commit neglected to update that, and
only handled it from ProcessQuery(). So it works for PlannedStmts, not
raw parse trees.
--
Peter Geoghegan
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, May 20, 2015 at 3:14 PM, Peter Geoghegan <pg@heroku.com> wrote:
I think you're right. The initial commit neglected to update that, and
only handled it from ProcessQuery(). So it works for PlannedStmts, not
raw parse trees.
Attached patch fixes this. Thanks for the report.
--
Peter Geoghegan
Attachments:
commandtag-bug.patchtext/x-patch; charset=US-ASCII; name=commandtag-bug.patchDownload+8-1
On 2015-05-20 15:21:49 -0700, Peter Geoghegan wrote:
On Wed, May 20, 2015 at 3:14 PM, Peter Geoghegan <pg@heroku.com> wrote:
I think you're right. The initial commit neglected to update that, and
only handled it from ProcessQuery(). So it works for PlannedStmts, not
raw parse trees.Attached patch fixes this. Thanks for the report.
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index a95eff1..8fd5ee8 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -1898,7 +1898,14 @@ CreateCommandTag(Node *parsetree) { /* raw plannable queries */ case T_InsertStmt: - tag = "INSERT"; + { + InsertStmt *stmt = (InsertStmt *) parsetree; + + tag = "INSERT"; + if (stmt->onConflictClause && + stmt->onConflictClause->action == ONCONFLICT_UPDATE) + tag = "UPSERT"; + } break;case T_DeleteStmt:
You realize there's other instances of this in the same damn function?
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Andres Freund <andres@anarazel.de> writes:
You realize there's other instances of this in the same damn function?
Not to mention that several places in libpq/fe-exec.c should be
taught about this new tag. And who-knows-what in other client-side
libraries. I am not really sure that it was a good idea to invent
this command tag. In fact, I'm pretty sure it was a *bad* idea ---
what will happen if we ever create a statement actually named UPSERT?
I think we should fix this by ripping out the variant tag, not trying
to propagate it everywhere it would need to go. Cute ideas are not
the same as good ideas.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2015-05-20 21:22:08 -0400, Tom Lane wrote:
Not to mention that several places in libpq/fe-exec.c should be
taught about this new tag. And who-knows-what in other client-side
libraries. I am not really sure that it was a good idea to invent
this command tag. In fact, I'm pretty sure it was a *bad* idea ---
what will happen if we ever create a statement actually named UPSERT?I think we should fix this by ripping out the variant tag, not trying
to propagate it everywhere it would need to go. Cute ideas are not
the same as good ideas.
I'm not particularly worried about conflicting with a potential future
UPSERT command. But I do see no corresponding benefit in having a
differerent command tag, so I'm inclined to agree that ripping it out is
likely the best way forward.
On the other hand, this was noticed because Alvaro just argued that it
*should* have a new command tag. Alvaro, where do you see the advantage?
Greetings,
Andres Freund
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, May 20, 2015 at 6:22 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
I am not really sure that it was a good idea to invent
this command tag. In fact, I'm pretty sure it was a *bad* idea ---
what will happen if we ever create a statement actually named UPSERT?
Why would we invent a statement actually named UPSERT?
I think we should fix this by ripping out the variant tag, not trying
to propagate it everywhere it would need to go. Cute ideas are not
the same as good ideas.
I don't feel particularly strongly about it one way or the other. The
way the command tag reports number of rows affected beside the INSERT
tag in psql is relevant. If some of those rows were actually updated,
that could mislead. I'm not saying that it outweighs your concern, but
it was the reason for inventing a variant tag, and it is a
consideration.
--
Peter Geoghegan
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, May 20, 2015 at 6:12 PM, Andres Freund <andres@anarazel.de> wrote:
You realize there's other instances of this in the same damn function?
I was misled by the argument name, "parsetree" -- in the past,
CreateCommandTag() actually only processed raw parse trees. Beyond
that, I wasn't aware that it is possible to produce a command tag for
every possible representation of an optimizable statement at every
stage of query processing.
I guess that I'll know next time I add a command tag.
--
Peter Geoghegan
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On May 20, 2015, at 9:22 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Andres Freund <andres@anarazel.de> writes:
You realize there's other instances of this in the same damn function?
Not to mention that several places in libpq/fe-exec.c should be
taught about this new tag. And who-knows-what in other client-side
libraries. I am not really sure that it was a good idea to invent
this command tag. In fact, I'm pretty sure it was a *bad* idea ---
what will happen if we ever create a statement actually named UPSERT?I think we should fix this by ripping out the variant tag, not trying
to propagate it everywhere it would need to go.
+1
...Robert
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 05/21/2015 05:08 AM, Peter Geoghegan wrote:
On Wed, May 20, 2015 at 6:22 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
I am not really sure that it was a good idea to invent
this command tag. In fact, I'm pretty sure it was a *bad* idea ---
what will happen if we ever create a statement actually named UPSERT?Why would we invent a statement actually named UPSERT?
And if we did, surely it would do some sort of an upsert operation, we
could use the UPSERT command tag for that too.
That said, I'm also not sure adding the UPSERT command tag is worth the
trouble. I'm OK with ripping it out. The row count returned in the
command tag is handy in the simple cases, but it gets complicated as
soon as you have rules or triggers, so you can't rely much on it anyway.
So as long as we document what the count means for an INSERT ... ON
CONFLICT, it should be OK to use the INSERT tag.
- Heikki
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Andres Freund wrote:
On 2015-05-20 21:22:08 -0400, Tom Lane wrote:
Not to mention that several places in libpq/fe-exec.c should be
taught about this new tag. And who-knows-what in other client-side
libraries. I am not really sure that it was a good idea to invent
this command tag. In fact, I'm pretty sure it was a *bad* idea ---
what will happen if we ever create a statement actually named UPSERT?I think we should fix this by ripping out the variant tag, not trying
to propagate it everywhere it would need to go. Cute ideas are not
the same as good ideas.I'm not particularly worried about conflicting with a potential future
UPSERT command. But I do see no corresponding benefit in having a
differerent command tag, so I'm inclined to agree that ripping it out is
likely the best way forward.On the other hand, this was noticed because Alvaro just argued that it
*should* have a new command tag. Alvaro, where do you see the advantage?
Well, I was just skimming nearby code and noticed that CreateCommandTag
hadn't been updated. As I said elsewhere, I'm not even running
commands. I'm not really set on having the tag be different.
That said, I'm not sure about having it be the same, either: first, I
don't think we need to update the fe-exec.c code at all -- I mean, all
the things I see there are very old compatibility stuff; reporting the
OID of the just-inserted row? Seriously, who has OIDs in user tables
anymore? Surely we wouldn't try to do that for INSERT ON CONFLICT DO
UPDATE at all ... if anyone wants that functionality, they can use
RETURNING, which is far saner.
As for PQcmdTuples, what would a single number returned mean? Are we
returning the sum of both inserted and updated tuples? Isn't this a bit
odd? If the number is useful, then perhaps we should distinguish the
cases; and if it's not useful, why not just return zero?
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers