pgsql: Add support for INSERT ... ON CONFLICT DO NOTHING/UPDATE.

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&amp;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&amp;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&amp;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
------
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(-)

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

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

Alvaro Herrera wrote:

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;

(But as I said earlier, it doesn't really affect me either way, so feel
free to rip it out.)

--
�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 Thu, May 21, 2015 at 4:32 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:

(But as I said earlier, it doesn't really affect me either way, so feel
free to rip it out.)

That appears to be the consensus. Should I post a patch?

--
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 2015-05-21 20:28:41 -0300, Alvaro Herrera wrote:

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.

The oid currently is reported for UPSERT... I agree it's not worth much,
but it seems pointless to break it for a single command.

As for PQcmdTuples, what would a single number returned mean? Are we
returning the sum of both inserted and updated tuples?

Yes.

Isn't this a bit odd?

Imo it's pretty much in line with what's done with INSTEAD OF, FDWs and
such.

If the number is useful, then perhaps we should distinguish the cases;
and if it's not useful, why not just return zero?

There's libraries/frameworks checking if an insert succeeded by looking
at that number, and it seems like a bad idea to needlessly break those.

So I think we're good with ripping it out. Peter?

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

On Thu, May 21, 2015 at 5:48 PM, Andres Freund <andres@anarazel.de> wrote:

If the number is useful, then perhaps we should distinguish the cases;
and if it's not useful, why not just return zero?

There's libraries/frameworks checking if an insert succeeded by looking
at that number, and it seems like a bad idea to needlessly break those.

+1

So I think we're good with ripping it out. Peter?

I'll come up with a patch.

--
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 Thu, May 21, 2015 at 5:51 PM, Peter Geoghegan <pg@heroku.com> wrote:

So I think we're good with ripping it out. Peter?

I'll come up with a patch.

Attached patch rips the command tag out.

--
Peter Geoghegan

On 2015-05-22 12:23:32 -0700, Peter Geoghegan wrote:

On Thu, May 21, 2015 at 5:51 PM, Peter Geoghegan <pg@heroku.com> wrote:

So I think we're good with ripping it out. Peter?

I'll come up with a patch.

Attached patch rips the command tag out.

Pushed.

--
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:

Add support for INSERT ... ON CONFLICT DO NOTHING/UPDATE.

Few comments/questions:

1.
insert.sgml
+      column.  For example, <literal>INSERT ... ON CONFLICT DO UPDATE
+      tab SET table_name.col = 1</> is invalid (this follows the general
+      behavior for <command>UPDATE</>).

Here in above example shouldn't table_name be used instead of *tab*
after UPDATE?

2.
+  <para>
+   Insert new distributor if possible;  otherwise
+   <literal>DO NOTHING</literal>.  Example assumes a unique index has been
+   defined that constrains values appearing in the
+   <literal>did</literal> column on a subset of rows where the
+   <literal>is_active</literal> boolean column evaluates to
+   <literal>true</literal>:
+<programlisting>
+  -- This statement could infer a partial unique index on "did"
+  -- with a predicate of "WHERE is_active", but it could also
+  -- just use a regular unique constraint on "did"
+  INSERT INTO distributors (did, dname) VALUES (10, 'Conrad International')
+  ON CONFLICT (did) WHERE is_active DO NOTHING;
+</programlisting>
+  </para>

What does WHERE index_predicate mean for non-partial indexes
or non-expression indexes?

Actually that could cause error even though it is not used
for a unique-index because it would mean that user needs
to have Select privilige on column in used in WHERE clause.

Create table spec_insert(c1 int, c2 int);
Create unique index idx_si on spec_insert(c1);
insert into spec_insert values(1) ON Conflict (c1) where c2 > 2 DO Nothing;

If above insert is executed by user who doesn't have Select privilege
on C2, it will give error.

3.
heap_abort_speculative()
+ /*
+ * Set the tuple header xmin to InvalidTransactionId.  This makes the
+ * tuple immediately invisible everyone.  (In particular, to any
+ * transactions waiting on the speculative token, woken up later.)

/invisible everyone/invisible to everyone

4.
ExecInsert()
+ * speculatively.  See the executor README for a full discussion
+ * of speculative insertion.

I could not find any updates about speculative insertion in executor/README,
am I missing the update?

5.
ExecInsert()
{
..
if (onconflict != ONCONFLICT_NONE && resultRelInfo->ri_NumIndices > 0)
{
..
if (!ExecCheckIndexConstraints(slot, estate, &conflictTid,
arbiterIndexes))
..
specToken = SpeculativeInsertionLockAcquire(GetCurrentTransactionId());
..
}

Here why do we need to perform speculative insertion for the
case when there is no constraint/index that can cause conflict?

For example, below case:
Create table spec_insert(c1 int);
Create index idx_si on spec_insert(c1);
insert into spec_insert values(1) ON Conflict DO Nothing;

6.
ExecInsert()
{
..
if (ExecOnConflictUpdate(mtstate, resultRelInfo,
&conflictTid, planSlot, slot,
estate, canSetTag, &returning))
{
InstrCountFiltered2(&mtstate->ps, 1);
..
}

ExecOnConflictUpdate()
{
..
if (!ExecQual(onConflictSetWhere, econtext, false))
{
ReleaseBuffer(buffer);
InstrCountFiltered1(&mtstate->ps, 1);
..
}

If ExecOnConflictUpdate() returns due to Qual (Qualification
is not satisfied), then it will result in counting both
Filtered1 and Filtered2. I think for such a case only one
of them should be updated, probably Filtered1.

7.
create table t1(c1 int, c2 int);
create unique index idx_t1 on t1(c1);
insert into t1 values(1,1);
postgres=# insert into t1 values(1, 1) On Conflict(c1) Do Update set c1=2
where c2 > 3;
ERROR: column reference "c2" is ambiguous
LINE 1: ...alues(1, 1) On Conflict(c1) Do Update set c1=2 where c2 > 3;

Why alias is required in Where condition whereas it works for Set?

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

On 22 May 2015 at 00:28, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:

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.

I agree with Alvaro that we need to be able to see a difference.

I also agree with Tom/Robert that we should not invent a new tag.

What I think should happen is that the command tag should vary according to
whether an INSERT or an UPDATE was performed, so we get a visible
difference without any new tags.

--
Simon Riggs http://www.2ndQuadrant.com/
<http://www.2ndquadrant.com/&gt;
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Simon Riggs wrote:

What I think should happen is that the command tag should vary according to
whether an INSERT or an UPDATE was performed, so we get a visible
difference without any new tags.

The problem with doing that is that the same command might have updated
some tuples and inserted others. Also, we build the tag before the
command is executed.

--
�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 27 May 2015 at 15:06, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:

Simon Riggs wrote:

What I think should happen is that the command tag should vary according

to

whether an INSERT or an UPDATE was performed, so we get a visible
difference without any new tags.

The problem with doing that is that the same command might have updated
some tuples and inserted others. Also, we build the tag before the
command is executed.

Hmm, I assumed the various anomalies meant that it was a single row only
command...

Anybody know where the various anomalies are documented? e.g. how if an
insert has been committed that we can't yet see whether we are allowed to
update that using same rules as update.

--
Simon Riggs http://www.2ndQuadrant.com/
<http://www.2ndquadrant.com/&gt;
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Andres Freund wrote:

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.

Is there a reason why heap_finish_speculative and heap_abort_speculative
were made to take a HeapTuple rather than just an ItemPointer? They
only use tuple->t_self in their code (except the former, which in an
assert verifies that the tuple is indeed speculative).

--
�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