Clean up generation of default names for constraints, indexes, and serial

sequences, as per recent discussion.  All these names are now of the
form table_column_type, with digits added if needed to make them unique.
Default constraint names are chosen to be unique across their whole schema,
not just within the parent object, so as to be more SQL-spec-compatible
and make the information schema views more useful.

6 files changed, 334 insertions, 348 deletions

diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 718c3b90910..a4cc33d4c9d 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/catalog/heap.c,v 1.269 2004/06/06 20:30:07 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/catalog/heap.c,v 1.270 2004/06/10 17:55:53 tgl Exp $
  *
  *
  * INTERFACE ROUTINES
@@ -1488,7 +1488,7 @@ AddRelationRawConstraints(Relation rel,
 	ParseState *pstate;
 	RangeTblEntry *rte;
 	int			numchecks;
-	int			constr_name_ctr = 0;
+	List	   *checknames;
 	ListCell   *cell;
 	Node	   *expr;
 	CookedConstraint *cooked;
@@ -1547,6 +1547,7 @@ AddRelationRawConstraints(Relation rel,
 	 * Process constraint expressions.
 	 */
 	numchecks = numoldchecks;
+	checknames = NIL;
 	foreach(cell, rawConstraints)
 	{
 		Constraint *cdef = (Constraint *) lfirst(cell);
@@ -1556,81 +1557,6 @@ AddRelationRawConstraints(Relation rel,
 			continue;
 		Assert(cdef->cooked_expr == NULL);
 
-		/* Check name uniqueness, or generate a new name */
-		if (cdef->name != NULL)
-		{
-			ListCell   *cell2;
-
-			ccname = cdef->name;
-			/* Check against pre-existing constraints */
-			if (ConstraintNameIsUsed(CONSTRAINT_RELATION,
-									 RelationGetRelid(rel),
-									 RelationGetNamespace(rel),
-									 ccname))
-				ereport(ERROR,
-						(errcode(ERRCODE_DUPLICATE_OBJECT),
-						 errmsg("constraint \"%s\" for relation \"%s\" already exists",
-								ccname, RelationGetRelationName(rel))));
-			/* Check against other new constraints */
-			/* Needed because we don't do CommandCounterIncrement in loop */
-			foreach(cell2, rawConstraints)
-			{
-				Constraint *cdef2 = (Constraint *) lfirst(cell2);
-
-				if (cdef2 == cdef ||
-					cdef2->contype != CONSTR_CHECK ||
-					cdef2->raw_expr == NULL ||
-					cdef2->name == NULL)
-					continue;
-				if (strcmp(cdef2->name, ccname) == 0)
-					ereport(ERROR,
-							(errcode(ERRCODE_DUPLICATE_OBJECT),
-						 errmsg("check constraint \"%s\" already exists",
-								ccname)));
-			}
-		}
-		else
-		{
-			bool		success;
-
-			do
-			{
-				ListCell   *cell2;
-
-				/*
-				 * Generate a name that does not conflict with
-				 * pre-existing constraints, nor with any auto-generated
-				 * names so far.
-				 */
-				ccname = GenerateConstraintName(CONSTRAINT_RELATION,
-												RelationGetRelid(rel),
-												RelationGetNamespace(rel),
-												&constr_name_ctr);
-
-				/*
-				 * Check against other new constraints, in case the user
-				 * has specified a name that looks like an auto-generated
-				 * name.
-				 */
-				success = true;
-				foreach(cell2, rawConstraints)
-				{
-					Constraint *cdef2 = (Constraint *) lfirst(cell2);
-
-					if (cdef2 == cdef ||
-						cdef2->contype != CONSTR_CHECK ||
-						cdef2->raw_expr == NULL ||
-						cdef2->name == NULL)
-						continue;
-					if (strcmp(cdef2->name, ccname) == 0)
-					{
-						success = false;
-						break;
-					}
-				}
-			} while (!success);
-		}
-
 		/*
 		 * Transform raw parsetree to executable expression.
 		 */
@@ -1663,6 +1589,73 @@ AddRelationRawConstraints(Relation rel,
 					 errmsg("cannot use aggregate function in check constraint")));
 
 		/*
+		 * Check name uniqueness, or generate a name if none was given.
+		 */
+		if (cdef->name != NULL)
+		{
+			ListCell   *cell2;
+
+			ccname = cdef->name;
+			/* Check against pre-existing constraints */
+			if (ConstraintNameIsUsed(CONSTRAINT_RELATION,
+									 RelationGetRelid(rel),
+									 RelationGetNamespace(rel),
+									 ccname))
+				ereport(ERROR,
+						(errcode(ERRCODE_DUPLICATE_OBJECT),
+						 errmsg("constraint \"%s\" for relation \"%s\" already exists",
+								ccname, RelationGetRelationName(rel))));
+			/* Check against other new constraints */
+			/* Needed because we don't do CommandCounterIncrement in loop */
+			foreach(cell2, checknames)
+			{
+				if (strcmp((char *) lfirst(cell2), ccname) == 0)
+					ereport(ERROR,
+							(errcode(ERRCODE_DUPLICATE_OBJECT),
+							 errmsg("check constraint \"%s\" already exists",
+									ccname)));
+			}
+		}
+		else
+		{
+			/*
+			 * When generating a name, we want to create "tab_col_check"
+			 * for a column constraint and "tab_check" for a table
+			 * constraint.  We no longer have any info about the
+			 * syntactic positioning of the constraint phrase, so we
+			 * approximate this by seeing whether the expression references
+			 * more than one column.  (If the user played by the rules,
+			 * the result is the same...)
+			 *
+			 * Note: pull_var_clause() doesn't descend into sublinks,
+			 * but we eliminated those above; and anyway this only needs
+			 * to be an approximate answer.
+			 */
+			List *vars;
+			char *colname;
+
+			vars = pull_var_clause(expr, false);
+
+			/* eliminate duplicates */
+			vars = list_union(NIL, vars);
+
+			if (list_length(vars) == 1)
+				colname = get_attname(RelationGetRelid(rel),
+									  ((Var *) linitial(vars))->varattno);
+			else
+				colname = NULL;
+
+			ccname = ChooseConstraintName(RelationGetRelationName(rel),
+										  colname,
+										  "check",
+										  RelationGetNamespace(rel),
+										  checknames);
+		}
+
+		/* save name for future checks */
+		checknames = lappend(checknames, ccname);
+
+		/*
 		 * OK, store it.
 		 */
 		StoreRelCheck(rel, ccname, nodeToString(expr));
diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c
index 09f3acd5f39..4bd9409b8de 100644
--- a/src/backend/catalog/pg_constraint.c
+++ b/src/backend/catalog/pg_constraint.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/catalog/pg_constraint.c,v 1.19 2003/11/29 19:51:46 pgsql Exp $
+ *	  $PostgreSQL: pgsql/src/backend/catalog/pg_constraint.c,v 1.20 2004/06/10 17:55:53 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -22,6 +22,7 @@
 #include "catalog/indexing.h"
 #include "catalog/pg_constraint.h"
 #include "catalog/pg_type.h"
+#include "commands/defrem.h"
 #include "miscadmin.h"
 #include "utils/array.h"
 #include "utils/builtins.h"
@@ -263,13 +264,19 @@ CreateConstraintEntry(const char *constraintName,
 
 /*
  * Test whether given name is currently used as a constraint name
- * for the given relation.
+ * for the given object (relation or domain).
  *
- * NB: Caller should hold exclusive lock on the given relation, else
- * this test is not very meaningful.
+ * This is used to decide whether to accept a user-specified constraint name.
+ * It is deliberately not the same test as ChooseConstraintName uses to decide
+ * whether an auto-generated name is OK: here, we will allow it unless there
+ * is an identical constraint name in use *on the same object*.
+ *
+ * NB: Caller should hold exclusive lock on the given object, else
+ * this test can be fooled by concurrent additions.
  */
 bool
-ConstraintNameIsUsed(CONSTRAINTCATEGORY conCat, Oid objId, Oid objNamespace, const char *cname)
+ConstraintNameIsUsed(ConstraintCategory conCat, Oid objId,
+					 Oid objNamespace, const char *conname)
 {
 	bool		found;
 	Relation	conDesc;
@@ -277,14 +284,14 @@ ConstraintNameIsUsed(CONSTRAINTCATEGORY conCat, Oid objId, Oid objNamespace, con
 	ScanKeyData skey[2];
 	HeapTuple	tup;
 
-	conDesc = heap_openr(ConstraintRelationName, RowExclusiveLock);
+	conDesc = heap_openr(ConstraintRelationName, AccessShareLock);
 
 	found = false;
 
 	ScanKeyInit(&skey[0],
 				Anum_pg_constraint_conname,
 				BTEqualStrategyNumber, F_NAMEEQ,
-				CStringGetDatum(cname));
+				CStringGetDatum(conname));
 
 	ScanKeyInit(&skey[1],
 				Anum_pg_constraint_connamespace,
@@ -311,101 +318,99 @@ ConstraintNameIsUsed(CONSTRAINTCATEGORY conCat, Oid objId, Oid objNamespace, con
 	}
 
 	systable_endscan(conscan);
-	heap_close(conDesc, RowExclusiveLock);
+	heap_close(conDesc, AccessShareLock);
 
 	return found;
 }
 
 /*
- * Generate a currently-unused constraint name for the given relation.
+ * Select a nonconflicting name for a new constraint.
+ *
+ * The objective here is to choose a name that is unique within the
+ * specified namespace.  Postgres does not require this, but the SQL
+ * spec does, and some apps depend on it.  Therefore we avoid choosing
+ * default names that so conflict.
+ *
+ * name1, name2, and label are used the same way as for makeObjectName(),
+ * except that the label can't be NULL; digits will be appended to the label
+ * if needed to create a name that is unique within the specified namespace.
  *
- * The passed counter should be initialized to 0 the first time through.
- * If multiple constraint names are to be generated in a single command,
- * pass the new counter value to each successive call, else the same
- * name will be generated each time.
+ * 'others' can be a list of string names already chosen within the current
+ * command (but not yet reflected into the catalogs); we will not choose
+ * a duplicate of one of these either.
  *
- * NB: Caller should hold exclusive lock on the given relation, else
- * someone else might choose the same name concurrently!
+ * Note: it is theoretically possible to get a collision anyway, if someone
+ * else chooses the same name concurrently.  This is fairly unlikely to be
+ * a problem in practice, especially if one is holding an exclusive lock on
+ * the relation identified by name1.
+ *
+ * Returns a palloc'd string.
  */
 char *
-GenerateConstraintName(CONSTRAINTCATEGORY conCat, Oid objId, Oid objNamespace, int *counter)
+ChooseConstraintName(const char *name1, const char *name2,
+					 const char *label, Oid namespace,
+					 List *others)
 {
-	bool		found;
+	int			pass = 0;
+	char	   *conname = NULL;
+	char		modlabel[NAMEDATALEN];
 	Relation	conDesc;
-	char	   *cname;
+	SysScanDesc conscan;
+	ScanKeyData skey[2];
+	bool		found;
+	ListCell   *l;
 
-	cname = (char *) palloc(NAMEDATALEN * sizeof(char));
+	conDesc = heap_openr(ConstraintRelationName, AccessShareLock);
 
-	conDesc = heap_openr(ConstraintRelationName, RowExclusiveLock);
+	/* try the unmodified label first */
+	StrNCpy(modlabel, label, sizeof(modlabel));
 
-	/* Loop until we find a non-conflicting constraint name */
-	/* We assume there will be one eventually ... */
-	do
+	for (;;)
 	{
-		SysScanDesc conscan;
-		ScanKeyData skey[2];
-		HeapTuple	tup;
-
-		++(*counter);
-		snprintf(cname, NAMEDATALEN, "$%d", *counter);
+		conname = makeObjectName(name1, name2, modlabel);
 
-		/*
-		 * This duplicates ConstraintNameIsUsed() so that we can avoid
-		 * re-opening pg_constraint for each iteration.
-		 */
 		found = false;
 
-		ScanKeyInit(&skey[0],
-					Anum_pg_constraint_conname,
-					BTEqualStrategyNumber, F_NAMEEQ,
-					CStringGetDatum(cname));
-
-		ScanKeyInit(&skey[1],
-					Anum_pg_constraint_connamespace,
-					BTEqualStrategyNumber, F_OIDEQ,
-					ObjectIdGetDatum(objNamespace));
-
-		conscan = systable_beginscan(conDesc, ConstraintNameNspIndex, true,
-									 SnapshotNow, 2, skey);
-
-		while (HeapTupleIsValid(tup = systable_getnext(conscan)))
+		foreach(l, others)
 		{
-			Form_pg_constraint con = (Form_pg_constraint) GETSTRUCT(tup);
-
-			if (conCat == CONSTRAINT_RELATION && con->conrelid == objId)
-			{
-				found = true;
-				break;
-			}
-			else if (conCat == CONSTRAINT_DOMAIN && con->contypid == objId)
+			if (strcmp((char *) lfirst(l), conname) == 0)
 			{
 				found = true;
 				break;
 			}
 		}
 
-		systable_endscan(conscan);
-	} while (found);
+		if (!found)
+		{
+			ScanKeyInit(&skey[0],
+						Anum_pg_constraint_conname,
+						BTEqualStrategyNumber, F_NAMEEQ,
+						CStringGetDatum(conname));
 
-	heap_close(conDesc, RowExclusiveLock);
+			ScanKeyInit(&skey[1],
+						Anum_pg_constraint_connamespace,
+						BTEqualStrategyNumber, F_OIDEQ,
+						ObjectIdGetDatum(namespace));
 
-	return cname;
-}
+			conscan = systable_beginscan(conDesc, ConstraintNameNspIndex, true,
+										 SnapshotNow, 2, skey);
 
-/*
- * Does the given name look like a generated constraint name?
- *
- * This is a test on the form of the name, *not* on whether it has
- * actually been assigned.
- */
-bool
-ConstraintNameIsGenerated(const char *cname)
-{
-	if (cname[0] != '$')
-		return false;
-	if (strspn(cname + 1, "0123456789") != strlen(cname + 1))
-		return false;
-	return true;
+			found = (HeapTupleIsValid(systable_getnext(conscan)));
+
+			systable_endscan(conscan);
+		}
+
+		if (!found)
+			break;
+
+		/* found a conflict, so try a new name component */
+		pfree(conname);
+		snprintf(modlabel, sizeof(modlabel), "%s%d", label, ++pass);
+	}
+
+	heap_close(conDesc, AccessShareLock);
+
+	return conname;
 }
 
 /*
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 1fe8e58e585..d141c83c1bf 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/commands/indexcmds.c,v 1.120 2004/05/26 04:41:11 neilc Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/indexcmds.c,v 1.121 2004/06/10 17:55:56 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -28,10 +28,10 @@
 #include "commands/defrem.h"
 #include "commands/tablecmds.h"
 #include "executor/executor.h"
+#include "mb/pg_wchar.h"
 #include "miscadmin.h"
 #include "optimizer/clauses.h"
 #include "optimizer/prep.h"
-#include "parser/analyze.h"
 #include "parser/parsetree.h"
 #include "parser/parse_coerce.h"
 #include "parser/parse_expr.h"
@@ -53,8 +53,6 @@ static void ComputeIndexAttrs(IndexInfo *indexInfo, Oid *classOidP,
 static Oid GetIndexOpClass(List *opclass, Oid attrType,
 				char *accessMethodName, Oid accessMethodId);
 static Oid	GetDefaultOpClass(Oid attrType, Oid accessMethodId);
-static char *CreateIndexName(const char *table_name, const char *column_name,
-							 const char *label, Oid inamespace);
 static bool relationHasPrimaryKey(Relation rel);
 
 
@@ -159,18 +157,18 @@ DefineIndex(RangeVar *heapRelation,
 	if (indexRelationName == NULL)
 	{
 		if (primary)
-			indexRelationName = CreateIndexName(RelationGetRelationName(rel),
-												NULL,
-												"pkey",
-												namespaceId);
+			indexRelationName = ChooseRelationName(RelationGetRelationName(rel),
+												   NULL,
+												   "pkey",
+												   namespaceId);
 		else
 		{
 			IndexElem  *iparam = (IndexElem *) linitial(attributeList);
 
-			indexRelationName = CreateIndexName(RelationGetRelationName(rel),
-												iparam->name,
-												"key",
-												namespaceId);
+			indexRelationName = ChooseRelationName(RelationGetRelationName(rel),
+												   iparam->name,
+												   "key",
+												   namespaceId);
 		}
 	}
 
@@ -657,35 +655,131 @@ GetDefaultOpClass(Oid attrType, Oid accessMethodId)
 }
 
 /*
- * Select a nonconflicting name for an index.
+ *	makeObjectName()
+ *
+ *	Create a name for an implicitly created index, sequence, constraint, etc.
+ *
+ *	The parameters are typically: the original table name, the original field
+ *	name, and a "type" string (such as "seq" or "pkey").	The field name
+ *	and/or type can be NULL if not relevant.
+ *
+ *	The result is a palloc'd string.
+ *
+ *	The basic result we want is "name1_name2_label", omitting "_name2" or
+ *	"_label" when those parameters are NULL.  However, we must generate
+ *	a name with less than NAMEDATALEN characters!  So, we truncate one or
+ *	both names if necessary to make a short-enough string.	The label part
+ *	is never truncated (so it had better be reasonably short).
+ *
+ *	The caller is responsible for checking uniqueness of the generated
+ *	name and retrying as needed; retrying will be done by altering the
+ *	"label" string (which is why we never truncate that part).
  */
-static char *
-CreateIndexName(const char *table_name, const char *column_name,
-				const char *label, Oid inamespace)
+char *
+makeObjectName(const char *name1, const char *name2, const char *label)
 {
-	int			pass = 0;
-	char	   *iname = NULL;
-	char		typename[NAMEDATALEN];
+	char	   *name;
+	int			overhead = 0;	/* chars needed for label and underscores */
+	int			availchars;		/* chars available for name(s) */
+	int			name1chars;		/* chars allocated to name1 */
+	int			name2chars;		/* chars allocated to name2 */
+	int			ndx;
+
+	name1chars = strlen(name1);
+	if (name2)
+	{
+		name2chars = strlen(name2);
+		overhead++;				/* allow for separating underscore */
+	}
+	else
+		name2chars = 0;
+	if (label)
+		overhead += strlen(label) + 1;
+
+	availchars = NAMEDATALEN - 1 - overhead;
+	Assert(availchars > 0);		/* else caller chose a bad label */
 
 	/*
-	 * The type name for makeObjectName is label, or labelN if that's
-	 * necessary to prevent collision with existing indexes.
+	 * If we must truncate,  preferentially truncate the longer name. This
+	 * logic could be expressed without a loop, but it's simple and
+	 * obvious as a loop.
 	 */
-	strncpy(typename, label, sizeof(typename));
+	while (name1chars + name2chars > availchars)
+	{
+		if (name1chars > name2chars)
+			name1chars--;
+		else
+			name2chars--;
+	}
+
+	if (name1)
+		name1chars = pg_mbcliplen(name1, name1chars, name1chars);
+	if (name2)
+		name2chars = pg_mbcliplen(name2, name2chars, name2chars);
+
+	/* Now construct the string using the chosen lengths */
+	name = palloc(name1chars + name2chars + overhead + 1);
+	memcpy(name, name1, name1chars);
+	ndx = name1chars;
+	if (name2)
+	{
+		name[ndx++] = '_';
+		memcpy(name + ndx, name2, name2chars);
+		ndx += name2chars;
+	}
+	if (label)
+	{
+		name[ndx++] = '_';
+		strcpy(name + ndx, label);
+	}
+	else
+		name[ndx] = '\0';
+
+	return name;
+}
+
+/*
+ * Select a nonconflicting name for a new relation.  This is ordinarily
+ * used to choose index names (which is why it's here) but it can also
+ * be used for sequences, or any autogenerated relation kind.
+ *
+ * name1, name2, and label are used the same way as for makeObjectName(),
+ * except that the label can't be NULL; digits will be appended to the label
+ * if needed to create a name that is unique within the specified namespace.
+ *
+ * Note: it is theoretically possible to get a collision anyway, if someone
+ * else chooses the same name concurrently.  This is fairly unlikely to be
+ * a problem in practice, especially if one is holding an exclusive lock on
+ * the relation identified by name1.  However, if choosing multiple names
+ * within a single command, you'd better create the new object and do
+ * CommandCounterIncrement before choosing the next one!
+ *
+ * Returns a palloc'd string.
+ */
+char *
+ChooseRelationName(const char *name1, const char *name2,
+				   const char *label, Oid namespace)
+{
+	int			pass = 0;
+	char	   *relname = NULL;
+	char		modlabel[NAMEDATALEN];
+
+	/* try the unmodified label first */
+	StrNCpy(modlabel, label, sizeof(modlabel));
 
 	for (;;)
 	{
-		iname = makeObjectName(table_name, column_name, typename);
+		relname = makeObjectName(name1, name2, modlabel);
 
-		if (!OidIsValid(get_relname_relid(iname, inamespace)))
+		if (!OidIsValid(get_relname_relid(relname, namespace)))
 			break;
 
 		/* found a conflict, so try a new name component */
-		pfree(iname);
-		snprintf(typename, sizeof(typename), "%s%d", label, ++pass);
+		pfree(relname);
+		snprintf(modlabel, sizeof(modlabel), "%s%d", label, ++pass);
 	}
 
-	return iname;
+	return relname;
 }
 
 /*
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 3a95064442c..7b0b35840ee 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.112 2004/06/06 20:30:07 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.113 2004/06/10 17:55:56 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -124,8 +124,6 @@ typedef struct AlteredTableInfo
 	List	   *changedConstraintDefs;	/* string definitions of same */
 	List	   *changedIndexOids;	/* OIDs of indexes to rebuild */
 	List	   *changedIndexDefs;	/* string definitions of same */
-	/* Workspace for ATExecAddConstraint */
-	int			constr_name_ctr;
 } AlteredTableInfo;
 
 /* Struct describing one new constraint to check in Phase 3 scan */
@@ -323,46 +321,45 @@ DefineRelation(CreateStmt *stmt, char relkind)
 
 	if (old_constraints != NIL)
 	{
-		ConstrCheck *check = (ConstrCheck *) palloc(list_length(old_constraints) *
-													sizeof(ConstrCheck));
+		ConstrCheck *check = (ConstrCheck *)
+			palloc0(list_length(old_constraints) * sizeof(ConstrCheck));
 		int			ncheck = 0;
-		int			constr_name_ctr = 0;
 
 		foreach(listptr, old_constraints)
 		{
 			Constraint *cdef = (Constraint *) lfirst(listptr);
+			bool dup = false;
 
 			if (cdef->contype != CONSTR_CHECK)
 				continue;
-
-			if (cdef->name != NULL)
+			Assert(cdef->name != NULL);
+			Assert(cdef->raw_expr == NULL && cdef->cooked_expr != NULL);
+			/*
+			 * In multiple-inheritance situations, it's possible to inherit
+			 * the same grandparent constraint through multiple parents.
+			 * Hence, discard inherited constraints that match as to both
+			 * name and expression.  Otherwise, gripe if the names conflict.
+			 */
+			for (i = 0; i < ncheck; i++)
 			{
-				for (i = 0; i < ncheck; i++)
+				if (strcmp(check[i].ccname, cdef->name) != 0)
+					continue;
+				if (strcmp(check[i].ccbin, cdef->cooked_expr) == 0)
 				{
-					if (strcmp(check[i].ccname, cdef->name) == 0)
-						ereport(ERROR,
-								(errcode(ERRCODE_DUPLICATE_OBJECT),
+					dup = true;
+					break;
+				}
+				ereport(ERROR,
+						(errcode(ERRCODE_DUPLICATE_OBJECT),
 						 errmsg("duplicate check constraint name \"%s\"",
 								cdef->name)));
-				}
-				check[ncheck].ccname = cdef->name;
 			}
-			else
+			if (!dup)
 			{
-				/*
-				 * Generate a constraint name.	NB: this should match the
-				 * form of names that GenerateConstraintName() may produce
-				 * for names added later.  We are assured that there is no
-				 * name conflict, because MergeAttributes() did not pass
-				 * back any names of this form.
-				 */
-				check[ncheck].ccname = (char *) palloc(NAMEDATALEN);
-				snprintf(check[ncheck].ccname, NAMEDATALEN, "$%d",
-						 ++constr_name_ctr);
+				check[ncheck].ccname = cdef->name;
+				check[ncheck].ccbin = pstrdup(cdef->cooked_expr);
+				ncheck++;
 			}
-			Assert(cdef->raw_expr == NULL && cdef->cooked_expr != NULL);
-			check[ncheck].ccbin = pstrdup(cdef->cooked_expr);
-			ncheck++;
 		}
 		if (ncheck > 0)
 		{
@@ -867,17 +864,7 @@ MergeAttributes(List *schema, List *supers, bool istemp,
 				Node	   *expr;
 
 				cdef->contype = CONSTR_CHECK;
-
-				/*
-				 * Do not inherit generated constraint names, since they
-				 * might conflict across multiple inheritance parents.
-				 * (But conflicts between user-assigned names will cause
-				 * an error.)
-				 */
-				if (ConstraintNameIsGenerated(check[i].ccname))
-					cdef->name = NULL;
-				else
-					cdef->name = pstrdup(check[i].ccname);
+				cdef->name = pstrdup(check[i].ccname);
 				cdef->raw_expr = NULL;
 				/* adjust varattnos of ccbin here */
 				expr = stringToNode(check[i].ccbin);
@@ -3610,10 +3597,11 @@ ATExecAddConstraint(AlteredTableInfo *tab, Relation rel, Node *newConstraint)
 			}
 			else
 				fkconstraint->constr_name =
-					GenerateConstraintName(CONSTRAINT_RELATION,
-										   RelationGetRelid(rel),
-										   RelationGetNamespace(rel),
-										   &tab->constr_name_ctr);
+					ChooseConstraintName(RelationGetRelationName(rel),
+										 strVal(linitial(fkconstraint->fk_attrs)),
+										 "fkey",
+										 RelationGetNamespace(rel),
+										 NIL);
 
 			ATAddForeignKeyConstraint(tab, rel, fkconstraint);
 
diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c
index 439ad91cc37..a64617d08d4 100644
--- a/src/backend/commands/typecmds.c
+++ b/src/backend/commands/typecmds.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/commands/typecmds.c,v 1.58 2004/06/04 20:35:21 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/typecmds.c,v 1.59 2004/06/10 17:55:56 tgl Exp $
  *
  * DESCRIPTION
  *	  The "DefineFoo" routines take the parse tree and pick out the
@@ -83,7 +83,7 @@ static void domainOwnerCheck(HeapTuple tup, TypeName *typename);
 static char *domainAddConstraint(Oid domainOid, Oid domainNamespace,
 					Oid baseTypeOid,
 					int typMod, Constraint *constr,
-					int *counter, char *domainName);
+					char *domainName);
 
 
 /*
@@ -509,7 +509,6 @@ DefineDomain(CreateDomainStmt *stmt)
 	Oid			basetypeoid;
 	Oid			domainoid;
 	Form_pg_type baseType;
-	int			counter = 0;
 
 	/* Convert list of names to a name and namespace */
 	domainNamespace = QualifiedNameGetCreationNamespace(stmt->domainname,
@@ -760,7 +759,7 @@ DefineDomain(CreateDomainStmt *stmt)
 			case CONSTR_CHECK:
 				domainAddConstraint(domainoid, domainNamespace,
 									basetypeoid, stmt->typename->typmod,
-									constr, &counter, domainName);
+									constr, domainName);
 				break;
 
 				/* Other constraint types were fully processed above */
@@ -768,6 +767,9 @@ DefineDomain(CreateDomainStmt *stmt)
 			default:
 				break;
 		}
+
+		/* CCI so we can detect duplicate constraint names */
+		CommandCounterIncrement();
 	}
 
 	/*
@@ -1463,7 +1465,6 @@ AlterDomainAddConstraint(List *names, Node *newConstraint)
 	char	   *ccbin;
 	Expr	   *expr;
 	ExprState  *exprstate;
-	int			counter = 0;
 	Constraint *constr;
 
 	/* Make a TypeName so we can use standard type lookup machinery */
@@ -1547,7 +1548,7 @@ AlterDomainAddConstraint(List *names, Node *newConstraint)
 
 	ccbin = domainAddConstraint(HeapTupleGetOid(tup), typTup->typnamespace,
 								typTup->typbasetype, typTup->typtypmod,
-							 constr, &counter, NameStr(typTup->typname));
+								constr, NameStr(typTup->typname));
 
 	/*
 	 * Test all values stored in the attributes based on the domain the
@@ -1788,7 +1789,7 @@ domainOwnerCheck(HeapTuple tup, TypeName *typename)
 static char *
 domainAddConstraint(Oid domainOid, Oid domainNamespace, Oid baseTypeOid,
 					int typMod, Constraint *constr,
-					int *counter, char *domainName)
+					char *domainName)
 {
 	Node	   *expr;
 	char	   *ccsrc;
@@ -1811,10 +1812,11 @@ domainAddConstraint(Oid domainOid, Oid domainNamespace, Oid baseTypeOid,
 					constr->name, domainName)));
 	}
 	else
-		constr->name = GenerateConstraintName(CONSTRAINT_DOMAIN,
-											  domainOid,
-											  domainNamespace,
-											  counter);
+		constr->name = ChooseConstraintName(domainName,
+											NULL,
+											"check",
+											domainNamespace,
+											NIL);
 
 	/*
 	 * Convert the A_EXPR in raw_expr into an EXPR
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index f6cdf96e296..e5537e26ae9 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -6,7 +6,7 @@
  * Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- *	$PostgreSQL: pgsql/src/backend/parser/analyze.c,v 1.304 2004/06/09 19:08:16 tgl Exp $
+ *	$PostgreSQL: pgsql/src/backend/parser/analyze.c,v 1.305 2004/06/10 17:55:58 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -20,6 +20,7 @@
 #include "catalog/namespace.h"
 #include "catalog/pg_index.h"
 #include "catalog/pg_type.h"
+#include "commands/defrem.h"
 #include "commands/prepare.h"
 #include "miscadmin.h"
 #include "nodes/makefuncs.h"
@@ -45,7 +46,6 @@
 #include "utils/lsyscache.h"
 #include "utils/relcache.h"
 #include "utils/syscache.h"
-#include "mb/pg_wchar.h"
 
 
 /* State shared by transformCreateSchemaStmt and its subroutines */
@@ -704,90 +704,6 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt,
 }
 
 /*
- *	makeObjectName()
- *
- *	Create a name for an implicitly created index, sequence, constraint, etc.
- *
- *	The parameters are: the original table name, the original field name, and
- *	a "type" string (such as "seq" or "pkey").	The field name and/or type
- *	can be NULL if not relevant.
- *
- *	The result is a palloc'd string.
- *
- *	The basic result we want is "name1_name2_type", omitting "_name2" or
- *	"_type" when those parameters are NULL.  However, we must generate
- *	a name with less than NAMEDATALEN characters!  So, we truncate one or
- *	both names if necessary to make a short-enough string.	The type part
- *	is never truncated (so it had better be reasonably short).
- *
- *	To reduce the probability of collisions, we might someday add more
- *	smarts to this routine, like including some "hash" characters computed
- *	from the truncated characters.	Currently it seems best to keep it simple,
- *	so that the generated names are easily predictable by a person.
- */
-char *
-makeObjectName(const char *name1, const char *name2, const char *typename)
-{
-	char	   *name;
-	int			overhead = 0;	/* chars needed for type and underscores */
-	int			availchars;		/* chars available for name(s) */
-	int			name1chars;		/* chars allocated to name1 */
-	int			name2chars;		/* chars allocated to name2 */
-	int			ndx;
-
-	name1chars = strlen(name1);
-	if (name2)
-	{
-		name2chars = strlen(name2);
-		overhead++;				/* allow for separating underscore */
-	}
-	else
-		name2chars = 0;
-	if (typename)
-		overhead += strlen(typename) + 1;
-
-	availchars = NAMEDATALEN - 1 - overhead;
-
-	/*
-	 * If we must truncate,  preferentially truncate the longer name. This
-	 * logic could be expressed without a loop, but it's simple and
-	 * obvious as a loop.
-	 */
-	while (name1chars + name2chars > availchars)
-	{
-		if (name1chars > name2chars)
-			name1chars--;
-		else
-			name2chars--;
-	}
-
-	if (name1)
-		name1chars = pg_mbcliplen(name1, name1chars, name1chars);
-	if (name2)
-		name2chars = pg_mbcliplen(name2, name2chars, name2chars);
-
-	/* Now construct the string using the chosen lengths */
-	name = palloc(name1chars + name2chars + overhead + 1);
-	strncpy(name, name1, name1chars);
-	ndx = name1chars;
-	if (name2)
-	{
-		name[ndx++] = '_';
-		strncpy(name + ndx, name2, name2chars);
-		ndx += name2chars;
-	}
-	if (typename)
-	{
-		name[ndx++] = '_';
-		strcpy(name + ndx, typename);
-	}
-	else
-		name[ndx] = '\0';
-
-	return name;
-}
-
-/*
  * transformCreateStmt -
  *	  transforms the "create table" statement
  *	  SQL92 allows constraints to be scattered all over, so thumb through
@@ -919,21 +835,34 @@ transformColumnDefinition(ParseState *pstate, CreateStmtContext *cxt,
 	/* Special actions for SERIAL pseudo-types */
 	if (is_serial)
 	{
-		char	   *sname;
+		Oid			snamespaceid;
 		char	   *snamespace;
+		char	   *sname;
 		char	   *qstring;
 		A_Const    *snamenode;
 		FuncCall   *funccallnode;
 		CreateSeqStmt *seqstmt;
 
 		/*
-		 * Determine name and namespace to use for the sequence.
+		 * Determine namespace and name to use for the sequence.
+		 *
+		 * Although we use ChooseRelationName, it's not guaranteed that
+		 * the selected sequence name won't conflict; given sufficiently
+		 * long field names, two different serial columns in the same table
+		 * could be assigned the same sequence name, and we'd not notice
+		 * since we aren't creating the sequence quite yet.  In practice
+		 * this seems quite unlikely to be a problem, especially since
+		 * few people would need two serial columns in one table.
 		 */
-		sname = makeObjectName(cxt->relation->relname, column->colname, "seq");
-		snamespace = get_namespace_name(RangeVarGetCreationNamespace(cxt->relation));
+		snamespaceid = RangeVarGetCreationNamespace(cxt->relation);
+		snamespace = get_namespace_name(snamespaceid);
+		sname = ChooseRelationName(cxt->relation->relname,
+								   column->colname,
+								   "seq",
+								   snamespaceid);
 
 		ereport(NOTICE,
-				(errmsg("%s will create implicit sequence \"%s\" for \"serial\" column \"%s.%s\"",
+				(errmsg("%s will create implicit sequence \"%s\" for serial column \"%s.%s\"",
 						cxt->stmtType, sname,
 						cxt->relation->relname, column->colname)));
 
@@ -975,7 +904,6 @@ transformColumnDefinition(ParseState *pstate, CreateStmtContext *cxt,
 
 		constraint = makeNode(Constraint);
 		constraint->contype = CONSTR_DEFAULT;
-		constraint->name = sname;
 		constraint->raw_expr = (Node *) funccallnode;
 		constraint->cooked_expr = NULL;
 		constraint->keys = NIL;
@@ -1044,30 +972,13 @@ transformColumnDefinition(ParseState *pstate, CreateStmtContext *cxt,
 				break;
 
 			case CONSTR_PRIMARY:
-				if (constraint->name == NULL)
-					constraint->name = makeObjectName(cxt->relation->relname,
-													  NULL,
-													  "pkey");
-				if (constraint->keys == NIL)
-					constraint->keys = list_make1(makeString(column->colname));
-				cxt->ixconstraints = lappend(cxt->ixconstraints, constraint);
-				break;
-
 			case CONSTR_UNIQUE:
-				if (constraint->name == NULL)
-					constraint->name = makeObjectName(cxt->relation->relname,
-													  column->colname,
-													  "key");
 				if (constraint->keys == NIL)
 					constraint->keys = list_make1(makeString(column->colname));
 				cxt->ixconstraints = lappend(cxt->ixconstraints, constraint);
 				break;
 
 			case CONSTR_CHECK:
-				if (constraint->name == NULL)
-					constraint->name = makeObjectName(cxt->relation->relname,
-													  column->colname,
-													  NULL);
 				cxt->ckconstraints = lappend(cxt->ckconstraints, constraint);
 				break;
 
@@ -1093,13 +1004,6 @@ transformTableConstraint(ParseState *pstate, CreateStmtContext *cxt,
 	switch (constraint->contype)
 	{
 		case CONSTR_PRIMARY:
-			if (constraint->name == NULL)
-				constraint->name = makeObjectName(cxt->relation->relname,
-												  NULL,
-												  "pkey");
-			cxt->ixconstraints = lappend(cxt->ixconstraints, constraint);
-			break;
-
 		case CONSTR_UNIQUE:
 			cxt->ixconstraints = lappend(cxt->ixconstraints, constraint);
 			break;