Skip to content

Commit b680ae4

Browse files
tglsfdctglsfdc
committed
Improve unique-constraint-violation error messages to include the exact
values being complained of. In passing, also remove the arbitrary length limitation in the similar error detail message for foreign key violations. Itagaki Takahiro
1 parent 2487d87 commit b680ae4

File tree

20 files changed

+173
-41
lines changed

20 files changed

+173
-41
lines changed

‎contrib/citext/expected/citext.out

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,10 +271,13 @@ SELECT name, 'A' = name AS t FROM try where name = 'A';
271271
-- expected failures on duplicate key
272272
INSERT INTO try (name) VALUES ('a');
273273
ERROR: duplicate key value violates unique constraint "try_pkey"
274+
DETAIL: Key (name)=(a) already exists.
274275
INSERT INTO try (name) VALUES ('A');
275276
ERROR: duplicate key value violates unique constraint "try_pkey"
277+
DETAIL: Key (name)=(A) already exists.
276278
INSERT INTO try (name) VALUES ('aB');
277279
ERROR: duplicate key value violates unique constraint "try_pkey"
280+
DETAIL: Key (name)=(aB) already exists.
278281
-- Make sure that citext_smaller() and citext_lager() work properly.
279282
SELECT citext_smaller( 'aa'::citext, 'ab'::citext ) = 'aa' AS t;
280283
t

‎contrib/citext/expected/citext_1.out

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,10 +271,13 @@ SELECT name, 'A' = name AS t FROM try where name = 'A';
271271
-- expected failures on duplicate key
272272
INSERT INTO try (name) VALUES ('a');
273273
ERROR: duplicate key value violates unique constraint "try_pkey"
274+
DETAIL: Key (name)=(a) already exists.
274275
INSERT INTO try (name) VALUES ('A');
275276
ERROR: duplicate key value violates unique constraint "try_pkey"
277+
DETAIL: Key (name)=(A) already exists.
276278
INSERT INTO try (name) VALUES ('aB');
277279
ERROR: duplicate key value violates unique constraint "try_pkey"
280+
DETAIL: Key (name)=(aB) already exists.
278281
-- Make sure that citext_smaller() and citext_lager() work properly.
279282
SELECT citext_smaller( 'aa'::citext, 'ab'::citext ) = 'aa' AS t;
280283
t

‎src/backend/access/common/indextuple.c

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
*
1010
*
1111
* IDENTIFICATION
12-
* $PostgreSQL: pgsql/src/backend/access/common/indextuple.c,v 1.88 2009/06/11 14:48:53 momjian Exp $
12+
* $PostgreSQL: pgsql/src/backend/access/common/indextuple.c,v 1.89 2009/08/01 19:59:41 tgl Exp $
1313
*
1414
*-------------------------------------------------------------------------
1515
*/
@@ -432,6 +432,27 @@ nocache_index_getattr(IndexTuple tup,
432432
return fetchatt(att[attnum], tp + off);
433433
}
434434

435+
/*
436+
* Convert an index tuple into Datum/isnull arrays.
437+
*
438+
* The caller must allocate sufficient storage for the output arrays.
439+
* (INDEX_MAX_KEYS entries should be enough.)
440+
*/
441+
void
442+
index_deform_tuple(IndexTuple tup, TupleDesc tupleDescriptor,
443+
Datum *values, bool *isnull)
444+
{
445+
int i;
446+
447+
/* Assert to protect callers who allocate fixed-size arrays */
448+
Assert(tupleDescriptor->natts <= INDEX_MAX_KEYS);
449+
450+
for (i = 0; i < tupleDescriptor->natts; i++)
451+
{
452+
values[i] = index_getattr(tup, i + 1, tupleDescriptor, &isnull[i]);
453+
}
454+
}
455+
435456
/*
436457
* Create a palloc'd copy of an index tuple.
437458
*/

‎src/backend/access/index/genam.c

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/access/index/genam.c,v 1.74 2009/06/11 14:48:54 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/access/index/genam.c,v 1.75 2009/08/01 19:59:41 tgl Exp $
1212
*
1313
* NOTES
1414
* many of the old access method routines have been turned into
@@ -24,6 +24,8 @@
2424
#include "miscadmin.h"
2525
#include "pgstat.h"
2626
#include "storage/bufmgr.h"
27+
#include "utils/builtins.h"
28+
#include "utils/lsyscache.h"
2729
#include "utils/rel.h"
2830
#include "utils/tqual.h"
2931

@@ -130,6 +132,59 @@ IndexScanEnd(IndexScanDesc scan)
130132
pfree(scan);
131133
}
132134

135+
/*
136+
* ReportUniqueViolation -- Report a unique-constraint violation.
137+
*
138+
* The index entry represented by values[]/isnull[] violates the unique
139+
* constraint enforced by this index. Throw a suitable error.
140+
*/
141+
void
142+
ReportUniqueViolation(Relation indexRelation, Datum *values, bool *isnull)
143+
{
144+
/*
145+
* XXX for the moment we use the index's tupdesc as a guide to the
146+
* datatypes of the values. This is okay for btree indexes but is in
147+
* fact the wrong thing in general. This will have to be fixed if we
148+
* are ever to support non-btree unique indexes.
149+
*/
150+
TupleDesc tupdesc = RelationGetDescr(indexRelation);
151+
char *key_names;
152+
StringInfoData key_values;
153+
int i;
154+
155+
key_names = pg_get_indexdef_columns(RelationGetRelid(indexRelation), true);
156+
157+
/* Get printable versions of the key values */
158+
initStringInfo(&key_values);
159+
for (i = 0; i < tupdesc->natts; i++)
160+
{
161+
char *val;
162+
163+
if (isnull[i])
164+
val = "null";
165+
else
166+
{
167+
Oid foutoid;
168+
bool typisvarlena;
169+
170+
getTypeOutputInfo(tupdesc->attrs[i]->atttypid,
171+
&foutoid, &typisvarlena);
172+
val = OidOutputFunctionCall(foutoid, values[i]);
173+
}
174+
175+
if (i > 0)
176+
appendStringInfoString(&key_values, ", ");
177+
appendStringInfoString(&key_values, val);
178+
}
179+
180+
ereport(ERROR,
181+
(errcode(ERRCODE_UNIQUE_VIOLATION),
182+
errmsg("duplicate key value violates unique constraint \"%s\"",
183+
RelationGetRelationName(indexRelation)),
184+
errdetail("Key (%s)=(%s) already exists.",
185+
key_names, key_values.data)));
186+
}
187+
133188

134189
/* ----------------------------------------------------------------
135190
* heap-or-index-scan access to system catalogs

‎src/backend/access/nbtree/nbtinsert.c

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/access/nbtree/nbtinsert.c,v 1.171 2009/07/29 20:56:18 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/access/nbtree/nbtinsert.c,v 1.172 2009/08/01 19:59:41 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -362,12 +362,25 @@ _bt_check_unique(Relation rel, IndexTuple itup, Relation heapRel,
362362
}
363363

364364
/*
365-
* This is a definite conflict.
365+
* This is a definite conflict. Break the tuple down
366+
* into datums and report the error. But first, make
367+
* sure we release the buffer locks we're holding ---
368+
* the error reporting code could make catalog accesses,
369+
* which in the worst case might touch this same index
370+
* and cause deadlocks.
366371
*/
367-
ereport(ERROR,
368-
(errcode(ERRCODE_UNIQUE_VIOLATION),
369-
errmsg("duplicate key value violates unique constraint \"%s\"",
370-
RelationGetRelationName(rel))));
372+
if (nbuf != InvalidBuffer)
373+
_bt_relbuf(rel, nbuf);
374+
_bt_relbuf(rel, buf);
375+
376+
{
377+
Datum values[INDEX_MAX_KEYS];
378+
bool isnull[INDEX_MAX_KEYS];
379+
380+
index_deform_tuple(itup, RelationGetDescr(rel),
381+
values, isnull);
382+
ReportUniqueViolation(rel, values, isnull);
383+
}
371384
}
372385
else if (all_dead)
373386
{

‎src/backend/utils/adt/ri_triggers.c

Lines changed: 12 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
*
1616
* Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
1717
*
18-
* $PostgreSQL: pgsql/src/backend/utils/adt/ri_triggers.c,v 1.113 2009/06/11 14:49:04 momjian Exp $
18+
* $PostgreSQL: pgsql/src/backend/utils/adt/ri_triggers.c,v 1.114 2009/08/01 19:59:41 tgl Exp $
1919
*
2020
* ----------
2121
*/
@@ -3413,11 +3413,8 @@ ri_ReportViolation(RI_QueryKey *qkey, const char *constrname,
34133413
HeapTuple violator, TupleDesc tupdesc,
34143414
bool spi_err)
34153415
{
3416-
#define BUFLENGTH 512
3417-
char key_names[BUFLENGTH];
3418-
char key_values[BUFLENGTH];
3419-
char *name_ptr = key_names;
3420-
char *val_ptr = key_values;
3416+
StringInfoData key_names;
3417+
StringInfoData key_values;
34213418
bool onfk;
34223419
int idx,
34233420
key_idx;
@@ -3465,6 +3462,8 @@ ri_ReportViolation(RI_QueryKey *qkey, const char *constrname,
34653462
}
34663463

34673464
/* Get printable versions of the keys involved */
3465+
initStringInfo(&key_names);
3466+
initStringInfo(&key_values);
34683467
for (idx = 0; idx < qkey->nkeypairs; idx++)
34693468
{
34703469
int fnum = qkey->keypair[idx][key_idx];
@@ -3476,20 +3475,13 @@ ri_ReportViolation(RI_QueryKey *qkey, const char *constrname,
34763475
if (!val)
34773476
val = "null";
34783477

3479-
/*
3480-
* Go to "..." if name or value doesn't fit in buffer. We reserve 5
3481-
* bytes to ensure we can add comma, "...", null.
3482-
*/
3483-
if (strlen(name) >= (key_names + BUFLENGTH - 5) - name_ptr ||
3484-
strlen(val) >= (key_values + BUFLENGTH - 5) - val_ptr)
3478+
if (idx > 0)
34853479
{
3486-
sprintf(name_ptr, "...");
3487-
sprintf(val_ptr, "...");
3488-
break;
3480+
appendStringInfoString(&key_names, ", ");
3481+
appendStringInfoString(&key_values, ", ");
34893482
}
3490-
3491-
name_ptr += sprintf(name_ptr, "%s%s", idx > 0 ? "," : "", name);
3492-
val_ptr += sprintf(val_ptr, "%s%s", idx > 0 ? "," : "", val);
3483+
appendStringInfoString(&key_names, name);
3484+
appendStringInfoString(&key_values, val);
34933485
}
34943486

34953487
if (onfk)
@@ -3498,7 +3490,7 @@ ri_ReportViolation(RI_QueryKey *qkey, const char *constrname,
34983490
errmsg("insert or update on table \"%s\" violates foreign key constraint \"%s\"",
34993491
RelationGetRelationName(fk_rel), constrname),
35003492
errdetail("Key (%s)=(%s) is not present in table \"%s\".",
3501-
key_names, key_values,
3493+
key_names.data, key_values.data,
35023494
RelationGetRelationName(pk_rel))));
35033495
else
35043496
ereport(ERROR,
@@ -3507,7 +3499,7 @@ ri_ReportViolation(RI_QueryKey *qkey, const char *constrname,
35073499
RelationGetRelationName(pk_rel),
35083500
constrname, RelationGetRelationName(fk_rel)),
35093501
errdetail("Key (%s)=(%s) is still referenced from table \"%s\".",
3510-
key_names, key_values,
3502+
key_names.data, key_values.data,
35113503
RelationGetRelationName(fk_rel))));
35123504
}
35133505

‎src/backend/utils/adt/ruleutils.c

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
*
1010
*
1111
* IDENTIFICATION
12-
* $PostgreSQL: pgsql/src/backend/utils/adt/ruleutils.c,v 1.305 2009/07/29 20:56:19 tgl Exp $
12+
* $PostgreSQL: pgsql/src/backend/utils/adt/ruleutils.c,v 1.306 2009/08/01 19:59:41 tgl Exp $
1313
*
1414
*-------------------------------------------------------------------------
1515
*/
@@ -142,7 +142,8 @@ static char *pg_get_viewdef_worker(Oid viewoid, int prettyFlags);
142142
static void decompile_column_index_array(Datum column_index_array, Oid relId,
143143
StringInfo buf);
144144
static char *pg_get_ruledef_worker(Oid ruleoid, int prettyFlags);
145-
static char *pg_get_indexdef_worker(Oid indexrelid, int colno, bool showTblSpc,
145+
static char *pg_get_indexdef_worker(Oid indexrelid, int colno,
146+
bool attrsOnly, bool showTblSpc,
146147
int prettyFlags);
147148
static char *pg_get_constraintdef_worker(Oid constraintId, bool fullCommand,
148149
int prettyFlags);
@@ -613,7 +614,7 @@ pg_get_indexdef(PG_FUNCTION_ARGS)
613614
Oid indexrelid = PG_GETARG_OID(0);
614615

615616
PG_RETURN_TEXT_P(string_to_text(pg_get_indexdef_worker(indexrelid, 0,
616-
false, 0)));
617+
false, false, 0)));
617618
}
618619

619620
Datum
@@ -626,18 +627,31 @@ pg_get_indexdef_ext(PG_FUNCTION_ARGS)
626627

627628
prettyFlags = pretty ? PRETTYFLAG_PAREN | PRETTYFLAG_INDENT : 0;
628629
PG_RETURN_TEXT_P(string_to_text(pg_get_indexdef_worker(indexrelid, colno,
629-
false, prettyFlags)));
630+
colno != 0,
631+
false,
632+
prettyFlags)));
630633
}
631634

632635
/* Internal version that returns a palloc'd C string */
633636
char *
634637
pg_get_indexdef_string(Oid indexrelid)
635638
{
636-
return pg_get_indexdef_worker(indexrelid, 0, true, 0);
639+
return pg_get_indexdef_worker(indexrelid, 0, false, true, 0);
640+
}
641+
642+
/* Internal version that just reports the column definitions */
643+
char *
644+
pg_get_indexdef_columns(Oid indexrelid, bool pretty)
645+
{
646+
int prettyFlags;
647+
648+
prettyFlags = pretty ? PRETTYFLAG_PAREN | PRETTYFLAG_INDENT : 0;
649+
return pg_get_indexdef_worker(indexrelid, 0, true, false, prettyFlags);
637650
}
638651

639652
static char *
640-
pg_get_indexdef_worker(Oid indexrelid, int colno, bool showTblSpc,
653+
pg_get_indexdef_worker(Oid indexrelid, int colno,
654+
bool attrsOnly, bool showTblSpc,
641655
int prettyFlags)
642656
{
643657
HeapTuple ht_idx;
@@ -736,7 +750,7 @@ pg_get_indexdef_worker(Oid indexrelid, int colno, bool showTblSpc,
736750
*/
737751
initStringInfo(&buf);
738752

739-
if (!colno)
753+
if (!attrsOnly)
740754
appendStringInfo(&buf, "CREATE %sINDEX %s ON %s USING %s (",
741755
idxrec->indisunique ? "UNIQUE " : "",
742756
quote_identifier(NameStr(idxrelrec->relname)),
@@ -790,8 +804,7 @@ pg_get_indexdef_worker(Oid indexrelid, int colno, bool showTblSpc,
790804
keycoltype = exprType(indexkey);
791805
}
792806

793-
/* Provide decoration only in the colno=0 case */
794-
if (!colno)
807+
if (!attrsOnly && (!colno || colno == keyno + 1))
795808
{
796809
/* Add the operator class name, if not default */
797810
get_opclass_name(indclass->values[keyno], keycoltype, &buf);
@@ -816,7 +829,7 @@ pg_get_indexdef_worker(Oid indexrelid, int colno, bool showTblSpc,
816829
}
817830
}
818831

819-
if (!colno)
832+
if (!attrsOnly)
820833
{
821834
appendStringInfoChar(&buf, ')');
822835

‎src/include/access/genam.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
10-
* $PostgreSQL: pgsql/src/include/access/genam.h,v 1.79 2009/07/29 20:56:19 tgl Exp $
10+
* $PostgreSQL: pgsql/src/include/access/genam.h,v 1.80 2009/08/01 19:59:41 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -164,6 +164,8 @@ extern FmgrInfo *index_getprocinfo(Relation irel, AttrNumber attnum,
164164
extern IndexScanDesc RelationGetIndexScan(Relation indexRelation,
165165
int nkeys, ScanKey key);
166166
extern void IndexScanEnd(IndexScanDesc scan);
167+
extern void ReportUniqueViolation(Relation indexRelation,
168+
Datum *values, bool *isnull);
167169

168170
/*
169171
* heap-or-index access to system catalogs (in genam.c)

‎src/include/access/itup.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
10-
* $PostgreSQL: pgsql/src/include/access/itup.h,v 1.51 2009/01/01 17:23:56 momjian Exp $
10+
* $PostgreSQL: pgsql/src/include/access/itup.h,v 1.52 2009/08/01 19:59:41 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -143,6 +143,8 @@ extern IndexTuple index_form_tuple(TupleDesc tupleDescriptor,
143143
Datum *values, bool *isnull);
144144
extern Datum nocache_index_getattr(IndexTuple tup, int attnum,
145145
TupleDesc tupleDesc, bool *isnull);
146+
extern void index_deform_tuple(IndexTuple tup, TupleDesc tupleDescriptor,
147+
Datum *values, bool *isnull);
146148
extern IndexTuple CopyIndexTuple(IndexTuple source);
147149

148150
#endif /* ITUP_H */

0 commit comments

Comments
 (0)