Re: Duplicate key value error

Is it possible to print which key value is duplicated when 'Duplicate
key value violates unique constraint' occurs? Foreign key violation
error reports such kind of information.

Vlad Arkhipov <arhipov(at)dc(dot)baikal(dot)ru> wrote:

> Is it possible to print which key value is duplicated when 'Duplicate
> key value violates unique constraint' occurs? Foreign key violation
> error reports such kind of information.

I think it is not difficult from a technical standpoint.
The attached patch adds DETAIL messages to duplicate key value error:

postgres=# INSERT INTO tbl(pk1, pk2) VALUES ('A', 1);
ERROR: duplicate key value violates unique constraint "tbl_pkey"
DETAIL: Key (pk1,pk2)=(A,1) already exists.

If no objection, I'd like to submit the patch to the next commit-fest (8.5).

Regards,
---
ITAGAKI Takahiro
NTT Open Source Software Center

Itagaki Takahiro <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp> writes:
> Vlad Arkhipov <arhipov(at)dc(dot)baikal(dot)ru> wrote:
>> Is it possible to print which key value is duplicated when 'Duplicate
>> key value violates unique constraint' occurs? Foreign key violation
>> error reports such kind of information.

> If no objection, I'd like to submit the patch to the next commit-fest (8.5).

I added this to CommitFest-2009-First. One comment is that I'd prefer
to refactor this so it's not btree-specific, but could be used by other
index AMs. I'm not quite sure where to put the code instead, though.

regards, tom lane

Em Fri, 03 Apr 2009 04:23:10 -0300, Itagaki Takahiro
<itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp> escreveu:
> Vlad Arkhipov <arhipov(at)dc(dot)baikal(dot)ru> wrote:
>
>> Is it possible to print which key value is duplicated when 'Duplicate
>> key value violates unique constraint' occurs? Foreign key violation
>> error reports such kind of information.
>
> I think it is not difficult from a technical standpoint.
> The attached patch adds DETAIL messages to duplicate key value error:
>
> postgres=# INSERT INTO tbl(pk1, pk2) VALUES ('A', 1);
> ERROR: duplicate key value violates unique constraint "tbl_pkey"
> DETAIL: Key (pk1,pk2)=(A,1) already exists.
>
> If no objection, I'd like to submit the patch to the next commit-fest
> (8.5).

Hi Takahiro, i'm the reviewer of your patch, and the following are my
comments about it:

The patch was applied totalty clean to CVS HEAD and compiled in Ubuntu
8.04, Ubuntu 8.10 and AIX 5.3, but failed in follow tests:

src/test/regress/expected/uuid.out
src/test/regress/expected/constraints.out
src/test/regress/expected/create_index.out
src/test/regress/expected/inherit.out
src/test/regress/expected/transactions.out
src/test/regress/expected/arrays.out
src/test/regress/expected/plpgsql.out
src/test/regress/expected/alter_table.out
src/test/regress/expected/tablespace.out

Would be good to modify the outputs to expect a new "DETAIL:" line.

Another comment is that the patch isn't in the standart context form, but
unified.

About the feature, it work as expected when I've INSERTed in both single
and compound-key or UPDATEd the key values to violates the constraint,
also in concurrently transactions. As expected too, when i INSERT or
UPDATE the key with a value thath overflow the 512 bytes i'm getting the
output as follow:

---
guedes=# INSERT INTO test_dup_char_key VALUES (repeat('x',1024), 'qq');
ERROR: duplicate key value violates unique constraint
"test_dup_char_key_pkey"
DETALHE: Key (...)=(...) already exists.
---

I'm thinking if could be better to shows Key (my_key)=(...) instead Key
(...)=(...) -- well, i don't know how much people uses a key with more
512B and how often it is to they don't know wich key it is, (just reading
a log, for example) to we consider this important.

On the other hand there is a comment by Tom [1] about "to refactor this so
it's not btree-specific, but could be used by other index AMs", so could
be better trying to think about this in a way to find another alternative,
if it is possible.

[1] http://archives.postgresql.org/pgsql-hackers/2009-04/msg00234.php

Thanks for your patch!

[]s
Dickson S. Guedes
http://pgcon.postgresql.org.br
http://www.postgresql.org.br

"Dickson S. Guedes" <listas(at)guedesoft(dot)net> wrote:

> Hi Takahiro, i'm the reviewer of your patch, and the following are my
> comments about it:

Thank you for reviewing. An updated patch is attached.

> The patch was applied totalty clean to CVS HEAD and compiled in Ubuntu
> 8.04, Ubuntu 8.10 and AIX 5.3, but failed in follow tests:
> Would be good to modify the outputs to expect a new "DETAIL:" line.

I adjusted expected output of regression test in the new patch.

> I'm thinking if could be better to shows Key (my_key)=(...) instead Key
> (...)=(...) -- well, i don't know how much people uses a key with more
> 512B and how often it is to they don't know wich key it is, (just reading
> a log, for example) to we consider this important.

I modified the format logic to use StringInfo and don't cut off the message
in 512 bytes. Key names and values will be never into '...'. I changed both
both report_unique_violation() and ri_ReportViolation().

> On the other hand there is a comment by Tom [1] about "to refactor this so
> it's not btree-specific, but could be used by other index AMs"
> [1] http://archives.postgresql.org/pgsql-hackers/2009-04/msg00234.php

I exported the reporting function to itup.h.

[include/access/itup.h]
extern void report_unique_violation(Relation rel, IndexTuple itup);

Regards,
---
ITAGAKI Takahiro
NTT Open Source Software Center

Em Tue, 21 Jul 2009 02:07:47 -0300, Itagaki Takahiro
<itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp> escreveu:
> I modified the format logic to use StringInfo and don't cut off the
> message in 512 bytes. Key names and values will be never into '...'. I
> changed both both report_unique_violation() and ri_ReportViolation().

Hi Takahiro!

Hum, for key names ok, but for values, wouldn't this worse the output when
it is greater than 512 bytes?
--
Dickson S. Guedes
mail/xmpp: guedes(at)guedesoft(dot)net - skype: guediz
http://guedesoft.net - http://www.postgresql.org.br
http://www.rnp.br/keyserver/pks/lookup?search=0x8F3E3C06D428D10A

"Dickson S. Guedes" <listas(at)guedesoft(dot)net> wrote:

> Hum, for key names ok, but for values, wouldn't this worse the output when
> it is greater than 512 bytes?

Why do we need to cut values in 512 bytes? It might be generate larger logs
before, but we don't have any limits for length of log messages.

ex) ERROR: ...
STATEMENT: (very long query without length-limit)

Also, we cannot use so long keys because length of key must be less then
BLCKSZ. Error messages is 256kB at longest (INDEX_MAX_KEYS=32 * BLKCSZ=8kB),
so I think it is not so dangerous.

Regards,
---
ITAGAKI Takahiro
NTT Open Source Software Center

Itagaki Takahiro <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp> writes:
> Thank you for reviewing. An updated patch is attached.

I applied this with some revisions.

I didn't like the fact that error reports for functional indexes came out
with "pg_expression_n" instead of something useful. I tweaked things to
use the pg_get_indexdef code to obtain proper descriptions of the columns.
(This also resulted in deciding to make the separator ", " instead of just
",", because that's what pg_get_indexdef does.)

The API for the reporting function wasn't going to work at all for
non-btree indexes, because they don't necessarily have index tuples
that contain exactly the values to be reported. (In particular, hash
doesn't.) I refactored things a bit to pass Datum/isnull arrays instead
of an IndexTuple. This also led to moving the reporting function to
genam.c, because it no longer had anything to do with IndexTuples.

We're still not there for being able to support unique hash indexes with
this, because it's still supposing that the index's tupledescriptor tells
the input column datatypes. However that's an internal matter now for the
reporting function, instead of being baked into its API. (I actually
don't see a very easy way to get the column datatypes --- we'll need to
think about that some, when and if unique hash indexes ever happen.)

regards, tom lane