INSERT ... ON CONFLICT error messages

Поиск
Список
Период
Сортировка
От Heikki Linnakangas
Тема INSERT ... ON CONFLICT error messages
Дата
Msg-id 5548E727.6040201@iki.fi
обсуждение исходный текст
Ответ на Re: INSERT ... ON CONFLICT syntax issues  (Peter Geoghegan <pg@heroku.com>)
Ответы Re: INSERT ... ON CONFLICT error messages  (Peter Geoghegan <pg@heroku.com>)
Re: INSERT ... ON CONFLICT error messages  (Peter Geoghegan <pg@heroku.com>)
Список pgsql-hackers
I've read through all the error messages in the patch. Some comments:

====

postgres=# create table foo (id int4);
CREATE TABLE
postgres=# create unique index foo_y on foo (id) where id > 0;
CREATE INDEX
postgres=# insert into foo values (-1)  on conflict (id) where id > 0 do 
nothing;
ERROR:  inferred arbiter partial unique index's predicate does not cover 
tuple proposed for insertion
DETAIL:  ON CONFLICT inference clause implies that the tuple proposed 
for insertion must be covered by the predicate of partial index "foo_y".

I'm surprised. If the inserted value doesn't match the WHERE clause of 
the constraint, there is clearly no conflict, so I would assume the 
above to work without error.

====

postgres=# create table foo (id int4 primary key, col2 int4, constraint 
col2_unique unique (col2) deferrable);
CREATE TABLE
postgres=# insert into foo values (2)  on conflict on foo_pkey do nothing;
ERROR:  ON CONFLICT is not supported on relations with deferred unique 
constraints/exclusion constraints

Why not? I would understand if the specified arbiter constraint is 
deferred, but why does it matter if one of the other constraints is?

====

postgres=# create table foo (id int4 primary key);
CREATE TABLE
postgres=# begin isolation level serializable;
BEGIN
postgres=# select 1; -- to get the snapshot ?column?
----------        1
(1 row)

<in another session, insert 1>
postgres=# insert into foo values (1)  on conflict on foo_pkey do nothing;
ERROR:  could not serialize access due to concurrent insert or update 
directing alternative ON CONFLICT path

What about a delete? Don't you get a serialization error, if another 
backend deletes the conflicting tuple, and you perform an INSERT based 
on the effects of the deleting transaction, which is not supposed to be 
visible to you?

The error message is quite long. How about just giving the default 
"could not serialize access due to concurrent update" message? I don't 
think the "directing alternative ON CONFLICT path" really helps the user.

====

postgres=# insert into foo values (1), (2)  on conflict on foo_pkey do 
update set id = 2;
ERROR:  ON CONFLICT DO UPDATE command could not lock/update 
self-inserted tuple
HINT:  Ensure that no rows proposed for insertion within the same 
command have duplicate constrained values.

"lock/update" doesn't sound good. If the system knows which it is, 
should say so. But in this case, locking the row just an implementation 
detail. We're performing the UPDATE when that error happens, not locking.

How about:

ERROR:  could not update tuple that was inserted in the same command
HINT:  Ensure that the rows being inserted do not conflict with each other.

BTW, the tuple might be an updated version of an existing tuple, i.e. 
the ON CONFLICT DO UPDATE ... was fired on an earlier row. Not 
necessarily a row that was inserted in the same command. The message is 
technically correct, but you need to understand the difference between 
tuple and a row. Actually, should we avoid using the term "tuple" 
altogether in user-facing errors?

====

postgres=# insert into foo values (1)  on conflict (xmin) do nothing;
ERROR:  system columns may not appear in unique index inference 
specification

"may" is usually not the right word to use in error messages, as it 
implies permission to do something (see style guide). It's not totally 
inappropriate in this case, but how about:

ERROR:  system columns cannot be used in an ON CONFLICT clause

====

postgres=# create table foo (id int4, constraint xcheck CHECK (id > 
0));CREATE TABLE
postgres=# insert into foo values (1) on conflict on xcheck do nothing;
ERROR:  constraint in ON CONFLICT clause has no associated index

How about:

ERROR:  "xcheck" is a CHECK constraint
DETAIL:  Only unique or exclusion constraints can be used in ON CONFLICT 
ON clause.

That's assuming that "xcheck" actually is a CHECK constraint. Similarly 
for a foreign key constraint.

====

postgres=# create table foo (id int4, constraint foo_x exclude using 
gist (id with =) );
CREATE TABLE
postgres=# insert into foo values (1)  on conflict on foo_x do update 
set id=-1;
ERROR:  "foo_x" is not a unique index
DETAIL:  ON CONFLICT DO UPDATE requires unique index as arbiter.

I think it would be better to refer to the constraint here, rather than 
the index. The user specified the constraint, the fact that it's backed 
by an index is just an implementation detail.

Actually, the user specified an exclusion constraint, and expected DO 
UPDATE to work with that. But we don't support that. So we should say:

ERROR:  ON CONFLICT ... DO UPDATE not supported with exclusion constraints

====

postgres=# create table foo (id int4 primary key, t text);
CREATE TABLE
postgres=# insert into foo values (1)  on conflict (t) do nothing;
ERROR:  could not infer which unique index to use from 
expressions/columns and predicate provided for ON CONFLICT

I think we should assume that the user listed the columns correctly, but 
there is no constraint on them. The current message implies that the 
list of columns was wrong. Either case is possible, of course, but I'd 
suggest:

ERROR: there is no unique or exclusion constraint matching the ON 
CONFLICT specification

====

postgres=# insert into foo values (1)  on conflict (t nulls first) do 
nothing;
ERROR:  ON CONFLICT does not accept ordering or NULLS FIRST/LAST 
specifications
LINE 1: insert into foo values (1)  on conflict (t nulls first) do n...
^
HINT:  These factors do not affect uniqueness of indexed datums.

I'd suggest:

ERROR:  NULLS FIRST/LAST is not allowed in ON CONFLICT clause

and just leave out the hint. Or say something along the lines "You can 
just leave it out".

Any chance the errlocation could point to the actual NULLS FIRST/LAST 
clause, not the paren starting the column list? And ERRCODE_SYNTAX_ERROR 
would be more appropriate for this.

====

postgres=# insert into foo values (1)  on conflict do update set count = 
excluded.count + 1;
ERROR:  ON CONFLICT DO UPDATE requires explicit arbiter index
LINE 1: insert into foo values (1)  on conflict do update set count ...                                    ^

"Hmm, so where do I stick the index then?" or "But I just created the 
index! Why can't the system find it?"

Can we avoid exposing the term "arbiter index" to the user? From a 
user's point of view, he cannot specify an index directly. He can 
specify a list of columns and an optional WHERE clause, or a constraint 
name.

How about:

ERROR:  DO UPDATE requires a constraint name or index specification
HINT:  For example, ON CONFLICT ON <constraint> or ON CONFLICT (<column>)

====

postgres=# insert into pg_roles values ('myrole')  on conflict do nothing;
ERROR:  ON CONFLICT not supported with catalog relations
LINE 1: insert into pg_roles values ('myrole')  on conflict do nothi...
^

I think the more common term used in error messages is "system catalog 
table"

====

postgres=# create table foo (id int4 primary key);
CREATE TABLE
postgres=# create rule insert_foo as on insert to foo do also insert 
into bar values (-1);
CREATE RULE
postgres=# insert into foo values (1)  on conflict on foo_pkey do update 
set id=1;
ERROR:  INSERT with ON CONFLICT clause may not target relation with 
INSERT or UPDATE rules

That's not strictly true. A "DO INSTEAD NOTHING" rule doesn't prevent ON 
CONFLICT from working, for example. Not sure how to phrase that into the 
message without making it too long.

"may" is again not very appropriate here. Should be "cannot". And the 
term "target relation" might not be very clear to an average user.

====

Phew, that was quite a list..

- Heikki




В списке pgsql-hackers по дате отправления:

Предыдущее
От: Sawada Masahiko
Дата:
Сообщение: Re: Proposal : REINDEX xxx VERBOSE
Следующее
От: Tom Lane
Дата:
Сообщение: Fixing busted citext function declarations