Обсуждение: 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@dc.baikal.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.takahiro@oss.ntt.co.jp> writes: > Vlad Arkhipov <arhipov@dc.baikal.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.takahiro@oss.ntt.co.jp> escreveu: > Vlad Arkhipov <arhipov@dc.baikal.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@guedesoft.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.takahiro@oss.ntt.co.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@guedesoft.net - skype: guediz http://guedesoft.net - http://www.postgresql.org.br http://www.rnp.br/keyserver/pks/lookup?search=0x8F3E3C06D428D10A
"Dickson S. Guedes" <listas@guedesoft.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.takahiro@oss.ntt.co.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