Preliminary code review for anonymous-composite-types patch: fix breakage

of functions returning domain types, update documentation for typtype,
move get_typtype to lsyscache.c (actually, resurrect the old version),
add defense against creating pseudo-typed table columns, fix some
bogus list-parsing in grammar.  Issues remain with respect to alias
handling and type checking; Joe is on those.

doc/src/sgml/catalogs.sgml

 <!--
  Documentation of the system catalogs, directed toward PostgreSQL developers
- $Header: /cvsroot/pgsql/doc/src/sgml/catalogs.sgml,v 2.51 2002/08/02 18:15:04 tgl Exp $
+ $Header: /cvsroot/pgsql/doc/src/sgml/catalogs.sgml,v 2.52 2002/08/05 02:30:46 tgl Exp $
  -->
 
 <chapter id="catalogs">
@@ -3201,8 +3201,9 @@
       <entry>
        <structfield>typtype</structfield> is <literal>b</literal> for
        a base type, <literal>c</literal> for a complex type (i.e.,
-       a table's row type), or <literal>d</literal> for a derived type (i.e.,
-       a domain).  See also <structfield>typrelid</structfield> 
+       a table's row type), <literal>d</literal> for a derived type (i.e.,
+       a domain), or <literal>p</literal> for a pseudo-type.  See also
+       <structfield>typrelid</structfield>
        and <structfield>typbasetype</structfield>.
       </entry>
      </row>
@@ -3235,7 +3236,7 @@
        <structfield>typtype</structfield>), then this field points to
        the <structfield>pg_class</structfield> entry that defines the
        corresponding table. A table could theoretically be used as a
-       composite data type, but this is not fully functional.
+       composite data type, but this is only partly functional.
        Zero for non-complex types.
       </entry>
      </row>

src/backend/access/common/tupdesc.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/access/common/tupdesc.c,v 1.84 2002/08/04 19:48:09 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/access/common/tupdesc.c,v 1.85 2002/08/05 02:30:49 tgl Exp $
  *
  * NOTES
  *	  some of the executor utility code such as "ExecTypeFromTL" should be
@@ -24,9 +24,9 @@
 #include "catalog/namespace.h"
 #include "catalog/pg_type.h"
 #include "nodes/parsenodes.h"
-#include "parser/parse_relation.h"
 #include "parser/parse_type.h"
 #include "utils/builtins.h"
+#include "utils/lsyscache.h"
 #include "utils/syscache.h"
 
 
@@ -601,7 +601,7 @@ RelationNameGetTupleDesc(char *relname)
 TupleDesc
 TypeGetTupleDesc(Oid typeoid, List *colaliases)
 {
-	char		functyptype = typeid_get_typtype(typeoid);
+	char		functyptype = get_typtype(typeoid);
 	TupleDesc	tupdesc = NULL;
 
 	/*
@@ -639,15 +639,13 @@ TypeGetTupleDesc(Oid typeoid, List *colaliases)
 
 					if (label != NULL)
 						namestrcpy(&(tupdesc->attrs[varattno]->attname), label);
-					else
-						MemSet(NameStr(tupdesc->attrs[varattno]->attname), 0, NAMEDATALEN);
 				}
 			}
 		}
 		else
 			elog(ERROR, "Invalid return relation specified for function");
 	}
-	else if (functyptype == 'b')
+	else if (functyptype == 'b' || functyptype == 'd')
 	{
 		/* Must be a base data type, i.e. scalar */
 		char	   *attname;

src/backend/catalog/heap.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/catalog/heap.c,v 1.216 2002/08/02 21:54:34 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/catalog/heap.c,v 1.217 2002/08/05 02:30:50 tgl Exp $
  *
  *
  * INTERFACE ROUTINES
@@ -369,18 +369,6 @@ CheckAttributeNames(TupleDesc tupdesc, bool relhasoids, char relkind)
 		}
 	}
 
-	/*
-	 * also, warn user if attribute to be created has an unknown typid
-	 * (usually as a result of a 'retrieve into' - jolly
-	 */
-	for (i = 0; i < natts; i++)
-	{
-		if (tupdesc->attrs[i]->atttypid == UNKNOWNOID)
-			elog(WARNING, "Attribute '%s' has an unknown type"
-				 "\n\tProceeding with relation creation anyway",
-				 NameStr(tupdesc->attrs[i]->attname));
-	}
-
 	/*
 	 * next check for repeated attribute names
 	 */
@@ -394,6 +382,28 @@ CheckAttributeNames(TupleDesc tupdesc, bool relhasoids, char relkind)
 					 NameStr(tupdesc->attrs[j]->attname));
 		}
 	}
+
+	/*
+	 * We also do some checking of the attribute types here.
+	 *
+	 * Warn user, but don't fail, if column to be created has UNKNOWN type
+	 * (usually as a result of a 'retrieve into' - jolly)
+	 *
+	 * Refuse any attempt to create a pseudo-type column.
+	 */
+	for (i = 0; i < natts; i++)
+	{
+		Oid		att_type = tupdesc->attrs[i]->atttypid;
+
+		if (att_type == UNKNOWNOID)
+			elog(WARNING, "Attribute \"%s\" has an unknown type"
+				 "\n\tProceeding with relation creation anyway",
+				 NameStr(tupdesc->attrs[i]->attname));
+		if (get_typtype(att_type) == 'p')
+			elog(ERROR, "Attribute \"%s\" has pseudo-type %s",
+				 NameStr(tupdesc->attrs[i]->attname),
+				 format_type_be(att_type));
+	}
 }
 
 /* --------------------------------

src/backend/catalog/pg_proc.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/catalog/pg_proc.c,v 1.86 2002/08/05 00:21:27 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/catalog/pg_proc.c,v 1.87 2002/08/05 02:30:50 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -25,7 +25,6 @@
 #include "miscadmin.h"
 #include "parser/parse_coerce.h"
 #include "parser/parse_expr.h"
-#include "parser/parse_relation.h"
 #include "parser/parse_type.h"
 #include "tcop/tcopprot.h"
 #include "utils/builtins.h"
@@ -370,7 +369,7 @@ checkretval(Oid rettype, char fn_typtype, List *queryTreeList)
 
 	typerelid = typeidTypeRelid(rettype);
 
-	if (fn_typtype == 'b')
+	if (fn_typtype == 'b' || fn_typtype == 'd')
 	{
 		/* Shouldn't have a typerelid */
 		Assert(typerelid == InvalidOid);
@@ -592,7 +591,7 @@ fmgr_sql_validator(PG_FUNCTION_ARGS)
 	prosrc = DatumGetCString(DirectFunctionCall1(textout, tmp));
 
 	/* check typtype to see if we have a predetermined return type */
-	functyptype = typeid_get_typtype(proc->prorettype);
+	functyptype = get_typtype(proc->prorettype);
 
 	querytree_list = pg_parse_and_rewrite(prosrc, proc->proargtypes, proc->pronargs);
 	checkretval(proc->prorettype, functyptype, querytree_list);

src/backend/executor/functions.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/executor/functions.c,v 1.53 2002/08/04 19:48:09 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/executor/functions.c,v 1.54 2002/08/05 02:30:50 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -195,7 +195,7 @@ init_sql_fcache(FmgrInfo *finfo)
 	 */
 	fcache->typlen = typeStruct->typlen;
 
-	if (typeStruct->typtype == 'b')
+	if (typeStruct->typtype == 'b' || typeStruct->typtype == 'd')
 	{
 		/* The return type is not a relation, so just use byval */
 		fcache->typbyval = typeStruct->typbyval;

src/backend/executor/nodeFunctionscan.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/executor/nodeFunctionscan.c,v 1.4 2002/08/04 19:48:09 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/executor/nodeFunctionscan.c,v 1.5 2002/08/05 02:30:50 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -31,7 +31,6 @@
 #include "executor/nodeFunctionscan.h"
 #include "parser/parsetree.h"
 #include "parser/parse_expr.h"
-#include "parser/parse_relation.h"
 #include "parser/parse_type.h"
 #include "storage/lmgr.h"
 #include "tcop/pquery.h"
@@ -204,7 +203,7 @@ ExecInitFunctionScan(FunctionScan *node, EState *estate, Plan *parent)
 	 * Now determine if the function returns a simple or composite type,
 	 * and check/add column aliases.
 	 */
-	functyptype = typeid_get_typtype(funcrettype);
+	functyptype = get_typtype(funcrettype);
 
 	/*
 	 * Build a suitable tupledesc representing the output rows
@@ -228,7 +227,7 @@ ExecInitFunctionScan(FunctionScan *node, EState *estate, Plan *parent)
 		else
 			elog(ERROR, "Invalid return relation specified for function");
 	}
-	else if (functyptype == 'b')
+	else if (functyptype == 'b' || functyptype == 'd')
 	{
 		/*
 		 * Must be a base data type, i.e. scalar
@@ -462,7 +461,7 @@ function_getonetuple(FunctionScanState *scanstate,
 			 */
 			if (fn_typtype == 'p' && fn_typeid == RECORDOID)
 				if (tupledesc_mismatch(tupdesc, slot->ttc_tupleDescriptor))
-					elog(ERROR, "Query specified return tuple and actual"
+					elog(ERROR, "Query-specified return tuple and actual"
 									" function return tuple do not match");
 		}
 		else

src/backend/parser/gram.y

@@ -11,7 +11,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/parser/gram.y,v 2.355 2002/08/04 19:48:09 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/parser/gram.y,v 2.356 2002/08/05 02:30:50 tgl Exp $
  *
  * HISTORY
  *	  AUTHOR			DATE			MAJOR EVENT
@@ -203,7 +203,7 @@ static void doNegateFloat(Value *v);
 %type <chr> 	TriggerOneEvent
 
 %type <list>	stmtblock, stmtmulti,
-				OptTableElementList, OptInherit, definition,
+				OptTableElementList, TableElementList, OptInherit, definition,
 				opt_distinct, opt_definition, func_args,
 				func_args_list, func_as, createfunc_opt_list
 				oper_argtypes, RuleActionList, RuleActionMulti,
@@ -216,7 +216,7 @@ static void doNegateFloat(Value *v);
 				insert_target_list, def_list, opt_indirection,
 				group_clause, TriggerFuncArgs, select_limit,
 				opt_select_limit, opclass_item_list, trans_options,
-				tableFuncElementList
+				TableFuncElementList, OptTableFuncElementList
 
 %type <range>	into_clause, OptTempTableName
 
@@ -257,8 +257,8 @@ static void doNegateFloat(Value *v);
 
 %type <vsetstmt> set_rest
 
-%type <node>	OptTableElement, ConstraintElem, tableFuncElement
-%type <node>	columnDef, tableFuncColumnDef
+%type <node>	TableElement, ConstraintElem, TableFuncElement
+%type <node>	columnDef
 %type <defelt>	def_elem
 %type <node>	def_arg, columnElem, where_clause, insert_column_item,
 				a_expr, b_expr, c_expr, r_expr, AexprConst,
@@ -1428,24 +1428,22 @@ OptTemp:	TEMPORARY						{ $$ = TRUE; }
 		;
 
 OptTableElementList:
-			OptTableElementList ',' OptTableElement
+			TableElementList					{ $$ = $1; }
+			| /*EMPTY*/							{ $$ = NIL; }
+		;
+
+TableElementList:
+			TableElementList ',' TableElement
 				{
-					if ($3 != NULL)
 					$$ = lappend($1, $3);
-					else
-						$$ = $1;
 				}
-			| OptTableElement
+			| TableElement
 				{
-					if ($1 != NULL)
 					$$ = makeList1($1);
-					else
-						$$ = NIL;
 				}
-			| /*EMPTY*/							{ $$ = NIL; }
 		;
 
-OptTableElement:
+TableElement:
 			columnDef							{ $$ = $1; }
 			| TableLikeClause					{ $$ = $1; }
 			| TableConstraint					{ $$ = $1; }
@@ -1877,7 +1875,7 @@ CreateSeqStmt:
 		;
 
 OptSeqList: OptSeqList OptSeqElem					{ $$ = lappend($1, $2); }
-			|										{ $$ = NIL; }
+			| /*EMPTY*/								{ $$ = NIL; }
 		;
 
 OptSeqElem: CACHE NumericOnly
@@ -4452,14 +4450,14 @@ table_ref:	relation_expr
 					n->coldeflist = NIL;
 					$$ = (Node *) n;
 				}
-			| func_table AS '(' tableFuncElementList ')'
+			| func_table AS '(' OptTableFuncElementList ')'
 				{
 					RangeFunction *n = makeNode(RangeFunction);
 					n->funccallnode = $1;
 					n->coldeflist = $4;
 					$$ = (Node *) n;
 				}
-			| func_table AS ColId '(' tableFuncElementList ')'
+			| func_table AS ColId '(' OptTableFuncElementList ')'
 				{
 					RangeFunction *n = makeNode(RangeFunction);
 					Alias *a = makeNode(Alias);
@@ -4469,7 +4467,7 @@ table_ref:	relation_expr
 					n->coldeflist = $5;
 					$$ = (Node *) n;
 				}
-			| func_table ColId '(' tableFuncElementList ')'
+			| func_table ColId '(' OptTableFuncElementList ')'
 				{
 					RangeFunction *n = makeNode(RangeFunction);
 					Alias *a = makeNode(Alias);
@@ -4733,29 +4731,23 @@ where_clause:
 		;
 
 
-tableFuncElementList:
-			tableFuncElementList ',' tableFuncElement
+OptTableFuncElementList:
+			TableFuncElementList				{ $$ = $1; }
+			| /*EMPTY*/							{ $$ = NIL; }
+		;
+
+TableFuncElementList:
+			TableFuncElementList ',' TableFuncElement
 				{
-					if ($3 != NULL)
 					$$ = lappend($1, $3);
-					else
-						$$ = $1;
 				}
-			| tableFuncElement
+			| TableFuncElement
 				{
-					if ($1 != NULL)
 					$$ = makeList1($1);
-					else
-						$$ = NIL;
 				}
-			| /*EMPTY*/							{ $$ = NIL; }
-		;
-
-tableFuncElement:
-			tableFuncColumnDef					{ $$ = $1; }
 		;
 
-tableFuncColumnDef:	ColId Typename
+TableFuncElement:	ColId Typename
 				{
 					ColumnDef *n = makeNode(ColumnDef);
 					n->colname = $1;

src/backend/parser/parse_relation.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/parser/parse_relation.c,v 1.72 2002/08/04 19:48:10 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/parser/parse_relation.c,v 1.73 2002/08/05 02:30:50 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -727,7 +727,7 @@ addRangeTableEntryForFunction(ParseState *pstate,
 	 * Now determine if the function returns a simple or composite type,
 	 * and check/add column aliases.
 	 */
-	functyptype = typeid_get_typtype(funcrettype);
+	functyptype = get_typtype(funcrettype);
 
 	if (functyptype == 'c')
 	{
@@ -776,7 +776,7 @@ addRangeTableEntryForFunction(ParseState *pstate,
 			elog(ERROR, "Invalid return relation specified for function %s",
 				 funcname);
 	}
-	else if (functyptype == 'b')
+	else if (functyptype == 'b' || functyptype == 'd')
 	{
 		/*
 		 * Must be a base data type, i.e. scalar.
@@ -1080,7 +1080,7 @@ expandRTE(ParseState *pstate, RangeTblEntry *rte,
 			{
 				/* Function RTE */
 				Oid	funcrettype = exprType(rte->funcexpr);
-				char functyptype = typeid_get_typtype(funcrettype);
+				char functyptype = get_typtype(funcrettype);
 				List *coldeflist = rte->coldeflist;
 
 				/*
@@ -1091,7 +1091,6 @@ expandRTE(ParseState *pstate, RangeTblEntry *rte,
 					Oid	funcrelid = typeidTypeRelid(funcrettype);
 					if (OidIsValid(funcrelid))
 					{
-
 						/*
 						 * Composite data type, i.e. a table's row type
 						 * Same as ordinary relation RTE
@@ -1142,7 +1141,7 @@ expandRTE(ParseState *pstate, RangeTblEntry *rte,
 						elog(ERROR, "Invalid return relation specified"
 									" for function");
 				}
-				else if (functyptype == 'b')
+				else if (functyptype == 'b' || functyptype == 'd')
 				{
 					/*
 					 * Must be a base data type, i.e. scalar
@@ -1392,7 +1391,7 @@ get_rte_attribute_type(RangeTblEntry *rte, AttrNumber attnum,
 			{
 				/* Function RTE */
 				Oid funcrettype = exprType(rte->funcexpr);
-				char functyptype = typeid_get_typtype(funcrettype);
+				char functyptype = get_typtype(funcrettype);
 				List *coldeflist = rte->coldeflist;
 
 				/*
@@ -1436,7 +1435,7 @@ get_rte_attribute_type(RangeTblEntry *rte, AttrNumber attnum,
 						elog(ERROR, "Invalid return relation specified"
 									" for function");
 				}
-				else if (functyptype == 'b')
+				else if (functyptype == 'b' || functyptype == 'd')
 				{
 					/*
 					 * Must be a base data type, i.e. scalar
@@ -1687,28 +1686,3 @@ warnAutoRange(ParseState *pstate, RangeVar *relation)
 			 pstate->parentParseState != NULL ? " in subquery" : "",
 			 relation->relname);
 }
-
-char
-typeid_get_typtype(Oid typeid)
-{
-	HeapTuple		typeTuple;
-	Form_pg_type	typeStruct;
-	char			result;
-
-	/*
-	 * determine if the function returns a simple, named composite,
-	 * or anonymous composite type
-	 */
- 	typeTuple = SearchSysCache(TYPEOID,
- 							   ObjectIdGetDatum(typeid),
- 							   0, 0, 0);
- 	if (!HeapTupleIsValid(typeTuple))
- 		elog(ERROR, "cache lookup for type %u failed", typeid);
- 	typeStruct = (Form_pg_type) GETSTRUCT(typeTuple);
- 
-	result = typeStruct->typtype;
-
- 	ReleaseSysCache(typeTuple);
-
-	return result;
-}

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.77 2002/08/02 18:15:08 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/utils/cache/lsyscache.c,v 1.78 2002/08/05 02:30:50 tgl Exp $
  *
  * NOTES
  *	  Eventually, the index information should go through here, too.
@@ -1142,11 +1142,9 @@ get_typavgwidth(Oid typid, int32 typmod)
 /*
  * get_typtype
  *
- *		Given the type OID, find if it is a basic type, a named relation
- *		or the generic type 'relation'.
+ *		Given the type OID, find if it is a basic type, a complex type, etc.
  *		It returns the null char if the cache lookup fails...
  */
-#ifdef NOT_USED
 char
 get_typtype(Oid typid)
 {
@@ -1167,7 +1165,7 @@ get_typtype(Oid typid)
 	else
 		return '\0';
 }
-#endif
+
 
 /*				---------- STATISTICS CACHE ----------					 */
 

src/include/catalog/pg_type.h

@@ -8,7 +8,7 @@
  * Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: pg_type.h,v 1.126 2002/08/04 19:48:10 momjian Exp $
+ * $Id: pg_type.h,v 1.127 2002/08/05 02:30:50 tgl Exp $
  *
  * NOTES
  *	  the genbki.sh script reads this file and generates .bki
@@ -60,10 +60,10 @@ CATALOG(pg_type) BOOTSTRAP
 	bool		typbyval;
 
 	/*
-	 * typtype is 'b' for a basic type, 'c' for a catalog type (ie a
-	 * class), or 'p' for a pseudo type. If typtype is 'c', typrelid is the
-	 * OID of the class' entry in pg_class. (Why do we need an entry in
-	 * pg_type for classes, anyway?)
+	 * typtype is 'b' for a basic type, 'c' for a complex type (ie a
+	 * table's rowtype), 'd' for a domain type, or 'p' for a pseudo type.
+	 *
+	 * If typtype is 'c', typrelid is the OID of the class' entry in pg_class.
 	 */
 	char		typtype;
 
@@ -75,7 +75,8 @@ CATALOG(pg_type) BOOTSTRAP
 	bool		typisdefined;
 
 	char		typdelim;		/* delimiter for arrays of this type */
-	Oid			typrelid;		/* 0 if not a class type */
+
+	Oid			typrelid;		/* 0 if not a complex type */
 
 	/*
 	 * If typelem is not 0 then it identifies another row in pg_type. The

src/include/parser/parse_relation.h

@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: parse_relation.h,v 1.36 2002/08/04 19:48:11 momjian Exp $
+ * $Id: parse_relation.h,v 1.37 2002/08/05 02:30:50 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -61,6 +61,5 @@ extern List *expandRelAttrs(ParseState *pstate, RangeTblEntry *rte);
 extern int	attnameAttNum(Relation rd, const char *attname, bool sysColOK);
 extern Name attnumAttName(Relation rd, int attid);
 extern Oid	attnumTypeId(Relation rd, int attid);
-extern char typeid_get_typtype(Oid typeid);
 
 #endif   /* PARSE_RELATION_H */

src/include/utils/lsyscache.h

@@ -6,7 +6,7 @@
  * Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: lsyscache.h,v 1.56 2002/08/02 18:15:09 tgl Exp $
+ * $Id: lsyscache.h,v 1.57 2002/08/05 02:30:50 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -51,6 +51,7 @@ extern bool get_typbyval(Oid typid);
 extern void get_typlenbyval(Oid typid, int16 *typlen, bool *typbyval);
 extern char get_typstorage(Oid typid);
 extern Node *get_typdefault(Oid typid);
+extern char get_typtype(Oid typid);
 extern Oid getBaseType(Oid typid);
 extern Oid getBaseTypeTypeMod(Oid typid, int32 *typmod);
 extern int32 get_typavgwidth(Oid typid, int32 typmod);