Fix handling of pg_type.typdefault per bug report from Dave Blasby.

If there's anyone out there who's actually using datatype-defined
default values, this will be an incompatible change in behavior ...
but the old behavior was so broken that I doubt anyone was using it.

5 files changed, 105 insertions, 130 deletions

diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 4f7b04a36a..b68488ae00 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/catalog/heap.c,v 1.175 2001/08/25 18:52:41 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/catalog/heap.c,v 1.176 2001/09/06 02:07:42 tgl Exp $
  *
  *
  * INTERFACE ROUTINES
@@ -657,7 +657,6 @@ AddNewRelationTuple(Relation pg_class_desc,
 static void
 AddNewRelationType(char *typeName, Oid new_rel_oid, Oid new_type_oid)
 {
-
 	/*
 	 * The sizes are set to oid size because it makes implementing sets
 	 * MUCH easier, and no one (we hope) uses these fields to figure out
@@ -666,24 +665,22 @@ AddNewRelationType(char *typeName, Oid new_rel_oid, Oid new_type_oid)
 	 * actually get is the oid of a tuple in the pg_proc catalog, so the
 	 * size of the "set" is the size of an oid. Similarly, byval being
 	 * true makes sets much easier, and it isn't used by anything else.
-	 *
-	 * XXX Note the assumption that OIDs are the same size as int4s.
 	 */
 	TypeCreate(typeName,		/* type name */
 			   new_type_oid,	/* preassigned oid for type */
 			   new_rel_oid,		/* relation oid */
 			   sizeof(Oid),		/* internal size */
-			   sizeof(Oid),		/* external size */
+			   -1,				/* external size */
 			   'c',				/* type-type (catalog) */
 			   ',',				/* default array delimiter */
-			   "int4in",		/* input procedure */
-			   "int4out",		/* output procedure */
-			   "int4in",		/* receive procedure */
-			   "int4out",		/* send procedure */
+			   "oidin",			/* input procedure */
+			   "oidout",		/* output procedure */
+			   "oidin",			/* receive procedure */
+			   "oidout",		/* send procedure */
 			   NULL,			/* array element type - irrelevant */
-			   "-",				/* default type value */
+			   NULL,			/* default type value - none */
 			   true,			/* passed by value */
-			   'i',				/* default alignment */
+			   'i',				/* default alignment - same as for OID */
 			   'p');			/* Not TOASTable */
 }
 
diff --git a/src/backend/catalog/pg_type.c b/src/backend/catalog/pg_type.c
index 5429c537a6..b31892860a 100644
--- a/src/backend/catalog/pg_type.c
+++ b/src/backend/catalog/pg_type.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/catalog/pg_type.c,v 1.62 2001/08/10 15:49:39 petere Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/catalog/pg_type.c,v 1.63 2001/09/06 02:07:42 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -51,7 +51,7 @@ TypeGetWithOpenRelation(Relation pg_type_desc,
 	/*
 	 * initialize the scan key and begin a scan of pg_type
 	 */
-	ScanKeyEntryInitialize(typeKey,
+	ScanKeyEntryInitialize(&typeKey[0],
 						   0,
 						   Anum_pg_type_typname,
 						   F_NAMEEQ,
@@ -318,10 +318,18 @@ TypeCreate(char *typeName,
 	}
 
 	/*
-	 * XXX comment me
+	 * validate size specifications: either positive (fixed-length) or
+	 * -1 (variable-length).
 	 */
-	if (externalSize == 0)
-		externalSize = -1;		/* variable length */
+	if (! (internalSize > 0 || internalSize == -1))
+		elog(ERROR, "TypeCreate: invalid type internal size %d",
+			 internalSize);
+	if (! (externalSize > 0 || externalSize == -1))
+		elog(ERROR, "TypeCreate: invalid type external size %d",
+			 externalSize);
+
+	if (internalSize != -1 && storage != 'p')
+		elog(ERROR, "TypeCreate: fixed size types must have storage PLAIN");
 
 	/*
 	 * initialize arrays needed by FormHeapTuple
@@ -330,21 +338,10 @@ TypeCreate(char *typeName,
 	{
 		nulls[i] = ' ';
 		replaces[i] = 'r';
-		values[i] = (Datum) NULL;		/* redundant, but nice */
+		values[i] = (Datum) 0;
 	}
 
 	/*
-	 * XXX
-	 *
-	 * Do this so that user-defined types have size -1 instead of zero if
-	 * they are variable-length - this is so that everything else in the
-	 * backend works.
-	 */
-
-	if (internalSize == 0)
-		internalSize = -1;
-
-	/*
 	 * initialize the *values information
 	 */
 	i = 0;
@@ -435,15 +432,19 @@ TypeCreate(char *typeName,
 	/*
 	 * initialize the default value for this type.
 	 */
-	values[i] = DirectFunctionCall1(textin,		/* 17 */
-			 CStringGetDatum(defaultTypeValue ? defaultTypeValue : "-"));
+	if (defaultTypeValue)
+		values[i] = DirectFunctionCall1(textin,
+										CStringGetDatum(defaultTypeValue));
+	else
+		nulls[i] = 'n';
+	i++;						/* 17 */
 
 	/*
 	 * open pg_type and begin a scan for the type name.
 	 */
 	pg_type_desc = heap_openr(TypeRelationName, RowExclusiveLock);
 
-	ScanKeyEntryInitialize(typeKey,
+	ScanKeyEntryInitialize(&typeKey[0],
 						   0,
 						   Anum_pg_type_typname,
 						   F_NAMEEQ,
diff --git a/src/backend/commands/define.c b/src/backend/commands/define.c
index f88343fa91..947257b5e1 100644
--- a/src/backend/commands/define.c
+++ b/src/backend/commands/define.c
@@ -10,7 +10,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/commands/define.c,v 1.58 2001/08/03 20:47:40 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/commands/define.c,v 1.59 2001/09/06 02:07:42 tgl Exp $
  *
  * DESCRIPTION
  *	  The "DefineFoo" routines take the parse tree and pick out the
@@ -535,20 +535,20 @@ DefineAggregate(char *aggName, List *parameters)
 void
 DefineType(char *typeName, List *parameters)
 {
-	int16		internalLength = 0;		/* int2 */
-	int16		externalLength = 0;		/* int2 */
+	int16		internalLength = -1;		/* int2 */
+	int16		externalLength = -1;		/* int2 */
 	char	   *elemName = NULL;
 	char	   *inputName = NULL;
 	char	   *outputName = NULL;
 	char	   *sendName = NULL;
 	char	   *receiveName = NULL;
-	char	   *defaultValue = NULL;	/* Datum */
+	char	   *defaultValue = NULL;
 	bool		byValue = false;
 	char		delimiter = DEFAULT_TYPDELIM;
 	char	   *shadow_type;
 	List	   *pl;
 	char		alignment = 'i'; /* default alignment */
-	char		storage = 'p';	/* default storage in TOAST */
+	char		storage = 'p';	/* default TOAST storage method */
 
 	/*
 	 * Type names must be one character shorter than other names, allowing
@@ -556,10 +556,8 @@ DefineType(char *typeName, List *parameters)
 	 * "_".
 	 */
 	if (strlen(typeName) > (NAMEDATALEN - 2))
-	{
 		elog(ERROR, "DefineType: type names must be %d characters or less",
 			 NAMEDATALEN - 2);
-	}
 
 	foreach(pl, parameters)
 	{
@@ -645,9 +643,6 @@ DefineType(char *typeName, List *parameters)
 	if (outputName == NULL)
 		elog(ERROR, "Define: \"output\" unspecified");
 
-	if (internalLength != -1 && storage != 'p')
-		elog(ERROR, "Define: fixed size types must have storage PLAIN");
-
 	/*
 	 * now have TypeCreate do all the real work.
 	 */
@@ -674,6 +669,9 @@ DefineType(char *typeName, List *parameters)
 	 */
 	shadow_type = makeArrayTypeName(typeName);
 
+	/* alignment must be 'i' or 'd' for arrays */
+	alignment = (alignment == 'd') ? 'd' : 'i';
+
 	TypeCreate(shadow_type,		/* type name */
 			   InvalidOid,		/* preassigned type oid (not done here) */
 			   InvalidOid,		/* relation oid (n/a here) */
@@ -688,7 +686,7 @@ DefineType(char *typeName, List *parameters)
 			   typeName,		/* element type name */
 			   NULL,			/* never a default type value */
 			   false,			/* never passed by value */
-			   alignment,		/* NB: must be 'i' or 'd' for arrays... */
+			   alignment,		/* see above */
 			   'x');			/* ARRAY is always toastable */
 
 	pfree(shadow_type);
diff --git a/src/backend/optimizer/prep/preptlist.c b/src/backend/optimizer/prep/preptlist.c
index da51a76d3f..737f624b5a 100644
--- a/src/backend/optimizer/prep/preptlist.c
+++ b/src/backend/optimizer/prep/preptlist.c
@@ -15,7 +15,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/optimizer/prep/preptlist.c,v 1.42 2001/03/22 03:59:38 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/optimizer/prep/preptlist.c,v 1.43 2001/09/06 02:07:42 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -192,45 +192,54 @@ expand_targetlist(List *tlist, int command_type,
 			{
 				case CMD_INSERT:
 					{
-						Datum		typedefault = get_typdefault(atttype);
-						int			typlen;
-						Const	   *temp_const;
+						bool		hasdefault;
+						Datum		typedefault;
+						int16		typlen;
+						bool		typbyval;
+						Const	   *def_const;
 
-#ifdef	_DROP_COLUMN_HACK__
-						if (COLUMN_IS_DROPPED(att_tup))
-							typedefault = PointerGetDatum(NULL);
-#endif	 /* _DROP_COLUMN_HACK__ */
-
-						if (typedefault == PointerGetDatum(NULL))
-							typlen = 0;
-						else
+						if (att_tup->attisset)
 						{
-
 							/*
-							 * Since this is an append or replace, the
-							 * size of any set attribute is the size of
-							 * the OID used to represent it.
+							 * Set attributes are represented as OIDs no
+							 * matter what the set element type is, and
+							 * the element type's default is irrelevant too.
 							 */
-							if (att_tup->attisset)
-								typlen = get_typlen(OIDOID);
+							hasdefault = false;
+							typedefault = (Datum) 0;
+							typlen = sizeof(Oid);
+							typbyval = true;
+						}
+						else
+						{
+#ifdef	_DROP_COLUMN_HACK__
+							if (COLUMN_IS_DROPPED(att_tup))
+							{
+								hasdefault = false;
+								typedefault = (Datum) 0;
+							}
 							else
-								typlen = get_typlen(atttype);
+#endif	 /* _DROP_COLUMN_HACK__ */
+								hasdefault = get_typdefault(atttype,
+															&typedefault);
+
+							get_typlenbyval(atttype, &typlen, &typbyval);
 						}
 
-						temp_const = makeConst(atttype,
-											   typlen,
-											   typedefault,
-								  (typedefault == PointerGetDatum(NULL)),
-											   false,
-											   false,	/* not a set */
-											   false);
+						def_const = makeConst(atttype,
+											  typlen,
+											  typedefault,
+											  !hasdefault,
+											  typbyval,
+											  false,	/* not a set */
+											  false);
 
 						new_tle = makeTargetEntry(makeResdom(attrno,
 															 atttype,
 															 -1,
 													   pstrdup(attrname),
 															 false),
-												  (Node *) temp_const);
+												  (Node *) def_const);
 						break;
 					}
 				case CMD_UPDATE:
diff --git a/src/backend/utils/cache/lsyscache.c b/src/backend/utils/cache/lsyscache.c
index 1637abe999..b788383ba5 100644
--- a/src/backend/utils/cache/lsyscache.c
+++ b/src/backend/utils/cache/lsyscache.c
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/utils/cache/lsyscache.c,v 1.57 2001/08/21 16:36:05 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/utils/cache/lsyscache.c,v 1.58 2001/09/06 02:07:42 tgl Exp $
  *
  * NOTES
  *	  Eventually, the index information should go through here, too.
@@ -750,23 +750,20 @@ get_typstorage(Oid typid)
 /*
  * get_typdefault
  *
- *	  Given a type OID, return the typdefault field associated with that
- *	  type, or Datum(NULL) if there is no typdefault.  (This implies
- *	  that pass-by-value types can't have a default value that has
- *	  a representation of zero.  Not worth fixing now.)
- *	  The result points to palloc'd storage for non-pass-by-value types.
+ *	  Given a type OID, return the type's default value, if any.
+ *	  Returns FALSE if there is no default (effectively, default is NULL).
+ *	  The result points to palloc'd storage for pass-by-reference types.
  */
-Datum
-get_typdefault(Oid typid)
+bool
+get_typdefault(Oid typid, Datum *defaultValue)
 {
 	HeapTuple	typeTuple;
 	Form_pg_type type;
-	struct varlena *typDefault;
+	Oid			typinput,
+				typelem;
+	Datum		textDefaultVal;
 	bool		isNull;
-	int32		dataSize;
-	int32		typLen;
-	bool		typByVal;
-	Datum		returnValue;
+	char	   *strDefaultVal;
 
 	typeTuple = SearchSysCache(TYPEOID,
 							   ObjectIdGetDatum(typid),
@@ -777,66 +774,39 @@ get_typdefault(Oid typid)
 
 	type = (Form_pg_type) GETSTRUCT(typeTuple);
 
+	typinput = type->typinput;
+	typelem = type->typelem;
+
 	/*
-	 * First, see if there is a non-null typdefault field (usually there
-	 * isn't)
+	 * typdefault is potentially null, so don't try to access it as a struct
+	 * field. Must do it the hard way with SysCacheGetAttr.
 	 */
-	typDefault = (struct varlena *)
-		DatumGetPointer(SysCacheGetAttr(TYPEOID,
-										typeTuple,
-										Anum_pg_type_typdefault,
-										&isNull));
+	textDefaultVal = SysCacheGetAttr(TYPEOID,
+									 typeTuple,
+									 Anum_pg_type_typdefault,
+									 &isNull);
 
 	if (isNull)
 	{
 		ReleaseSysCache(typeTuple);
-		return PointerGetDatum(NULL);
+		*defaultValue = (Datum) 0;
+		return false;
 	}
 
-	/*
-	 * Otherwise, extract/copy the value.
-	 */
-	dataSize = VARSIZE(typDefault) - VARHDRSZ;
-	typLen = type->typlen;
-	typByVal = type->typbyval;
+	/* Convert text datum to C string */
+	strDefaultVal = DatumGetCString(DirectFunctionCall1(textout,
+														textDefaultVal));
 
-	if (typByVal)
-	{
-		if (dataSize == typLen)
-			returnValue = fetch_att(VARDATA(typDefault), typByVal, typLen);
-		else
-			returnValue = PointerGetDatum(NULL);
-	}
-	else if (typLen < 0)
-	{
-		/* variable-size type */
-		if (dataSize < 0)
-			returnValue = PointerGetDatum(NULL);
-		else
-		{
-			returnValue = PointerGetDatum(palloc(VARSIZE(typDefault)));
-			memcpy((char *) DatumGetPointer(returnValue),
-				   (char *) typDefault,
-				   (int) VARSIZE(typDefault));
-		}
-	}
-	else
-	{
-		/* fixed-size pass-by-ref type */
-		if (dataSize != typLen)
-			returnValue = PointerGetDatum(NULL);
-		else
-		{
-			returnValue = PointerGetDatum(palloc(dataSize));
-			memcpy((char *) DatumGetPointer(returnValue),
-				   VARDATA(typDefault),
-				   (int) dataSize);
-		}
-	}
+	/* Convert C string to a value of the given type */
+	*defaultValue = OidFunctionCall3(typinput,
+									 CStringGetDatum(strDefaultVal),
+									 ObjectIdGetDatum(typelem),
+									 Int32GetDatum(-1));
 
+	pfree(strDefaultVal);
 	ReleaseSysCache(typeTuple);
 
-	return returnValue;
+	return true;
 }
 
 /*