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

Author: tglsfdc

contrib/citext/expected/citext.out

@@ -271,10 +271,13 @@ SELECT name, 'A' = name AS t      FROM try where name = 'A';
 -- expected failures on duplicate key
 INSERT INTO try (name) VALUES ('a');
 ERROR:  duplicate key value violates unique constraint "try_pkey"
+DETAIL:  Key (name)=(a) already exists.
 INSERT INTO try (name) VALUES ('A');
 ERROR:  duplicate key value violates unique constraint "try_pkey"
+DETAIL:  Key (name)=(A) already exists.
 INSERT INTO try (name) VALUES ('aB');
 ERROR:  duplicate key value violates unique constraint "try_pkey"
+DETAIL:  Key (name)=(aB) already exists.
 -- Make sure that citext_smaller() and citext_lager() work properly.
 SELECT citext_smaller( 'aa'::citext, 'ab'::citext ) = 'aa' AS t;
  t 

contrib/citext/expected/citext_1.out

@@ -271,10 +271,13 @@ SELECT name, 'A' = name AS t      FROM try where name = 'A';
 -- expected failures on duplicate key
 INSERT INTO try (name) VALUES ('a');
 ERROR:  duplicate key value violates unique constraint "try_pkey"
+DETAIL:  Key (name)=(a) already exists.
 INSERT INTO try (name) VALUES ('A');
 ERROR:  duplicate key value violates unique constraint "try_pkey"
+DETAIL:  Key (name)=(A) already exists.
 INSERT INTO try (name) VALUES ('aB');
 ERROR:  duplicate key value violates unique constraint "try_pkey"
+DETAIL:  Key (name)=(aB) already exists.
 -- Make sure that citext_smaller() and citext_lager() work properly.
 SELECT citext_smaller( 'aa'::citext, 'ab'::citext ) = 'aa' AS t;
  t 

src/backend/access/common/indextuple.c

@@ -9,7 +9,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/common/indextuple.c,v 1.88 2009/06/11 14:48:53 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/common/indextuple.c,v 1.89 2009/08/01 19:59:41 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -432,6 +432,27 @@ nocache_index_getattr(IndexTuple tup,
 	return fetchatt(att[attnum], tp + off);
 }
 
+/*
+ * Convert an index tuple into Datum/isnull arrays.
+ *
+ * The caller must allocate sufficient storage for the output arrays.
+ * (INDEX_MAX_KEYS entries should be enough.)
+ */
+void
+index_deform_tuple(IndexTuple tup, TupleDesc tupleDescriptor,
+				   Datum *values, bool *isnull)
+{
+	int			i;
+
+	/* Assert to protect callers who allocate fixed-size arrays */
+	Assert(tupleDescriptor->natts <= INDEX_MAX_KEYS);
+
+	for (i = 0; i < tupleDescriptor->natts; i++)
+	{
+		values[i] = index_getattr(tup, i + 1, tupleDescriptor, &isnull[i]);
+	}
+}
+
 /*
  * Create a palloc'd copy of an index tuple.
  */

src/backend/access/index/genam.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/index/genam.c,v 1.74 2009/06/11 14:48:54 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/index/genam.c,v 1.75 2009/08/01 19:59:41 tgl Exp $
  *
  * NOTES
  *	  many of the old access method routines have been turned into
@@ -24,6 +24,8 @@
 #include "miscadmin.h"
 #include "pgstat.h"
 #include "storage/bufmgr.h"
+#include "utils/builtins.h"
+#include "utils/lsyscache.h"
 #include "utils/rel.h"
 #include "utils/tqual.h"
 
@@ -130,6 +132,59 @@ IndexScanEnd(IndexScanDesc scan)
 	pfree(scan);
 }
 
+/*
+ * ReportUniqueViolation -- Report a unique-constraint violation.
+ *
+ * The index entry represented by values[]/isnull[] violates the unique
+ * constraint enforced by this index.  Throw a suitable error.
+ */
+void
+ReportUniqueViolation(Relation indexRelation, Datum *values, bool *isnull)
+{
+	/*
+	 * XXX for the moment we use the index's tupdesc as a guide to the
+	 * datatypes of the values.  This is okay for btree indexes but is in
+	 * fact the wrong thing in general.  This will have to be fixed if we
+	 * are ever to support non-btree unique indexes.
+	 */
+	TupleDesc	tupdesc = RelationGetDescr(indexRelation);
+	char	   *key_names;
+	StringInfoData key_values;
+	int			i;
+
+	key_names = pg_get_indexdef_columns(RelationGetRelid(indexRelation), true);
+
+	/* Get printable versions of the key values */
+	initStringInfo(&key_values);
+	for (i = 0; i < tupdesc->natts; i++)
+	{
+		char   *val;
+
+		if (isnull[i])
+			val = "null";
+		else
+		{
+			Oid		foutoid;
+			bool	typisvarlena;
+
+			getTypeOutputInfo(tupdesc->attrs[i]->atttypid,
+							  &foutoid, &typisvarlena);
+			val = OidOutputFunctionCall(foutoid, values[i]);
+		}
+
+		if (i > 0)
+			appendStringInfoString(&key_values, ", ");
+		appendStringInfoString(&key_values, val);
+	}
+
+	ereport(ERROR,
+			(errcode(ERRCODE_UNIQUE_VIOLATION),
+			 errmsg("duplicate key value violates unique constraint \"%s\"",
+					RelationGetRelationName(indexRelation)),
+			 errdetail("Key (%s)=(%s) already exists.",
+					   key_names, key_values.data)));
+}
+
 
 /* ----------------------------------------------------------------
  *		heap-or-index-scan access to system catalogs

src/backend/access/nbtree/nbtinsert.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/nbtree/nbtinsert.c,v 1.171 2009/07/29 20:56:18 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/nbtree/nbtinsert.c,v 1.172 2009/08/01 19:59:41 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -362,12 +362,25 @@ _bt_check_unique(Relation rel, IndexTuple itup, Relation heapRel,
 					}
 
 					/*
-					 * This is a definite conflict.
+					 * This is a definite conflict.  Break the tuple down
+					 * into datums and report the error.  But first, make
+					 * sure we release the buffer locks we're holding ---
+					 * the error reporting code could make catalog accesses,
+					 * which in the worst case might touch this same index
+					 * and cause deadlocks.
 					 */
-					ereport(ERROR,
-							(errcode(ERRCODE_UNIQUE_VIOLATION),
-							 errmsg("duplicate key value violates unique constraint \"%s\"",
-									RelationGetRelationName(rel))));
+					if (nbuf != InvalidBuffer)
+						_bt_relbuf(rel, nbuf);
+					_bt_relbuf(rel, buf);
+
+					{
+						Datum	values[INDEX_MAX_KEYS];
+						bool	isnull[INDEX_MAX_KEYS];
+
+						index_deform_tuple(itup, RelationGetDescr(rel),
+										   values, isnull);
+						ReportUniqueViolation(rel, values, isnull);
+					}
 				}
 				else if (all_dead)
 				{

src/backend/utils/adt/ri_triggers.c

@@ -15,7 +15,7 @@
  *
  * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
  *
- * $PostgreSQL: pgsql/src/backend/utils/adt/ri_triggers.c,v 1.113 2009/06/11 14:49:04 momjian Exp $
+ * $PostgreSQL: pgsql/src/backend/utils/adt/ri_triggers.c,v 1.114 2009/08/01 19:59:41 tgl Exp $
  *
  * ----------
  */
@@ -3413,11 +3413,8 @@ ri_ReportViolation(RI_QueryKey *qkey, const char *constrname,
 				   HeapTuple violator, TupleDesc tupdesc,
 				   bool spi_err)
 {
-#define BUFLENGTH	512
-	char		key_names[BUFLENGTH];
-	char		key_values[BUFLENGTH];
-	char	   *name_ptr = key_names;
-	char	   *val_ptr = key_values;
+	StringInfoData key_names;
+	StringInfoData key_values;
 	bool		onfk;
 	int			idx,
 				key_idx;
@@ -3465,6 +3462,8 @@ ri_ReportViolation(RI_QueryKey *qkey, const char *constrname,
 	}
 
 	/* Get printable versions of the keys involved */
+	initStringInfo(&key_names);
+	initStringInfo(&key_values);
 	for (idx = 0; idx < qkey->nkeypairs; idx++)
 	{
 		int			fnum = qkey->keypair[idx][key_idx];
@@ -3476,20 +3475,13 @@ ri_ReportViolation(RI_QueryKey *qkey, const char *constrname,
 		if (!val)
 			val = "null";
 
-		/*
-		 * Go to "..." if name or value doesn't fit in buffer.  We reserve 5
-		 * bytes to ensure we can add comma, "...", null.
-		 */
-		if (strlen(name) >= (key_names + BUFLENGTH - 5) - name_ptr ||
-			strlen(val) >= (key_values + BUFLENGTH - 5) - val_ptr)
+		if (idx > 0)
 		{
-			sprintf(name_ptr, "...");
-			sprintf(val_ptr, "...");
-			break;
+			appendStringInfoString(&key_names, ", ");
+			appendStringInfoString(&key_values, ", ");
 		}
-
-		name_ptr += sprintf(name_ptr, "%s%s", idx > 0 ? "," : "", name);
-		val_ptr += sprintf(val_ptr, "%s%s", idx > 0 ? "," : "", val);
+		appendStringInfoString(&key_names, name);
+		appendStringInfoString(&key_values, val);
 	}
 
 	if (onfk)
@@ -3498,7 +3490,7 @@ ri_ReportViolation(RI_QueryKey *qkey, const char *constrname,
 				 errmsg("insert or update on table \"%s\" violates foreign key constraint \"%s\"",
 						RelationGetRelationName(fk_rel), constrname),
 				 errdetail("Key (%s)=(%s) is not present in table \"%s\".",
-						   key_names, key_values,
+						   key_names.data, key_values.data,
 						   RelationGetRelationName(pk_rel))));
 	else
 		ereport(ERROR,
@@ -3507,7 +3499,7 @@ ri_ReportViolation(RI_QueryKey *qkey, const char *constrname,
 						RelationGetRelationName(pk_rel),
 						constrname, RelationGetRelationName(fk_rel)),
 			errdetail("Key (%s)=(%s) is still referenced from table \"%s\".",
-					  key_names, key_values,
+					  key_names.data, key_values.data,
 					  RelationGetRelationName(fk_rel))));
 }
 

src/backend/utils/adt/ruleutils.c

@@ -9,7 +9,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/adt/ruleutils.c,v 1.305 2009/07/29 20:56:19 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/adt/ruleutils.c,v 1.306 2009/08/01 19:59:41 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -142,7 +142,8 @@ static char *pg_get_viewdef_worker(Oid viewoid, int prettyFlags);
 static void decompile_column_index_array(Datum column_index_array, Oid relId,
 							 StringInfo buf);
 static char *pg_get_ruledef_worker(Oid ruleoid, int prettyFlags);
-static char *pg_get_indexdef_worker(Oid indexrelid, int colno, bool showTblSpc,
+static char *pg_get_indexdef_worker(Oid indexrelid, int colno,
+					   bool attrsOnly, bool showTblSpc,
 					   int prettyFlags);
 static char *pg_get_constraintdef_worker(Oid constraintId, bool fullCommand,
 							int prettyFlags);
@@ -613,7 +614,7 @@ pg_get_indexdef(PG_FUNCTION_ARGS)
 	Oid			indexrelid = PG_GETARG_OID(0);
 
 	PG_RETURN_TEXT_P(string_to_text(pg_get_indexdef_worker(indexrelid, 0,
-														   false, 0)));
+														   false, false, 0)));
 }
 
 Datum
@@ -626,18 +627,31 @@ pg_get_indexdef_ext(PG_FUNCTION_ARGS)
 
 	prettyFlags = pretty ? PRETTYFLAG_PAREN | PRETTYFLAG_INDENT : 0;
 	PG_RETURN_TEXT_P(string_to_text(pg_get_indexdef_worker(indexrelid, colno,
-													   false, prettyFlags)));
+														   colno != 0,
+														   false,
+														   prettyFlags)));
 }
 
 /* Internal version that returns a palloc'd C string */
 char *
 pg_get_indexdef_string(Oid indexrelid)
 {
-	return pg_get_indexdef_worker(indexrelid, 0, true, 0);
+	return pg_get_indexdef_worker(indexrelid, 0, false, true, 0);
+}
+
+/* Internal version that just reports the column definitions */
+char *
+pg_get_indexdef_columns(Oid indexrelid, bool pretty)
+{
+	int			prettyFlags;
+
+	prettyFlags = pretty ? PRETTYFLAG_PAREN | PRETTYFLAG_INDENT : 0;
+	return pg_get_indexdef_worker(indexrelid, 0, true, false, prettyFlags);
 }
 
 static char *
-pg_get_indexdef_worker(Oid indexrelid, int colno, bool showTblSpc,
+pg_get_indexdef_worker(Oid indexrelid, int colno,
+					   bool attrsOnly, bool showTblSpc,
 					   int prettyFlags)
 {
 	HeapTuple	ht_idx;
@@ -736,7 +750,7 @@ pg_get_indexdef_worker(Oid indexrelid, int colno, bool showTblSpc,
 	 */
 	initStringInfo(&buf);
 
-	if (!colno)
+	if (!attrsOnly)
 		appendStringInfo(&buf, "CREATE %sINDEX %s ON %s USING %s (",
 						 idxrec->indisunique ? "UNIQUE " : "",
 						 quote_identifier(NameStr(idxrelrec->relname)),
@@ -790,8 +804,7 @@ pg_get_indexdef_worker(Oid indexrelid, int colno, bool showTblSpc,
 			keycoltype = exprType(indexkey);
 		}
 
-		/* Provide decoration only in the colno=0 case */
-		if (!colno)
+		if (!attrsOnly && (!colno || colno == keyno + 1))
 		{
 			/* Add the operator class name, if not default */
 			get_opclass_name(indclass->values[keyno], keycoltype, &buf);
@@ -816,7 +829,7 @@ pg_get_indexdef_worker(Oid indexrelid, int colno, bool showTblSpc,
 		}
 	}
 
-	if (!colno)
+	if (!attrsOnly)
 	{
 		appendStringInfoChar(&buf, ')');
 

src/include/access/genam.h

@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/access/genam.h,v 1.79 2009/07/29 20:56:19 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/access/genam.h,v 1.80 2009/08/01 19:59:41 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -164,6 +164,8 @@ extern FmgrInfo *index_getprocinfo(Relation irel, AttrNumber attnum,
 extern IndexScanDesc RelationGetIndexScan(Relation indexRelation,
 					 int nkeys, ScanKey key);
 extern void IndexScanEnd(IndexScanDesc scan);
+extern void ReportUniqueViolation(Relation indexRelation,
+								  Datum *values, bool *isnull);
 
 /*
  * heap-or-index access to system catalogs (in genam.c)

src/include/access/itup.h

@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/access/itup.h,v 1.51 2009/01/01 17:23:56 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/access/itup.h,v 1.52 2009/08/01 19:59:41 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -143,6 +143,8 @@ extern IndexTuple index_form_tuple(TupleDesc tupleDescriptor,
 				 Datum *values, bool *isnull);
 extern Datum nocache_index_getattr(IndexTuple tup, int attnum,
 					  TupleDesc tupleDesc, bool *isnull);
+extern void index_deform_tuple(IndexTuple tup, TupleDesc tupleDescriptor,
+				 Datum *values, bool *isnull);
 extern IndexTuple CopyIndexTuple(IndexTuple source);
 
 #endif   /* ITUP_H */