From 07f9682de43ce53fcd6d86744f610cacfabc60bb Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 5 Aug 2002 02:30:50 +0000 Subject: [PATCH] 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 | 9 +-- src/backend/access/common/tupdesc.c | 10 ++-- src/backend/catalog/heap.c | 36 +++++++----- src/backend/catalog/pg_proc.c | 7 +-- src/backend/executor/functions.c | 4 +- src/backend/executor/nodeFunctionscan.c | 9 ++- src/backend/parser/gram.y | 76 +++++++++++-------------- src/backend/parser/parse_relation.c | 40 +++---------- src/backend/utils/cache/lsyscache.c | 8 +-- src/include/catalog/pg_type.h | 13 +++-- src/include/parser/parse_relation.h | 3 +- src/include/utils/lsyscache.h | 3 +- 12 files changed, 95 insertions(+), 123 deletions(-) diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml index 573815388fe..38dbe32aad7 100644 --- a/doc/src/sgml/catalogs.sgml +++ b/doc/src/sgml/catalogs.sgml @@ -1,6 +1,6 @@ @@ -3201,8 +3201,9 @@ typtype is b for a base type, c for a complex type (i.e., - a table's row type), or d for a derived type (i.e., - a domain). See also typrelid + a table's row type), d for a derived type (i.e., + a domain), or p for a pseudo-type. See also + typrelid and typbasetype. @@ -3235,7 +3236,7 @@ typtype), then this field points to the pg_class 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. diff --git a/src/backend/access/common/tupdesc.c b/src/backend/access/common/tupdesc.c index e97f77ce700..0e1765fc09e 100644 --- a/src/backend/access/common/tupdesc.c +++ b/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; diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index cb1248caaa0..65f7dc097a0 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.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)); + } } /* -------------------------------- diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c index c8f769db306..5cc863249b4 100644 --- a/src/backend/catalog/pg_proc.c +++ b/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); diff --git a/src/backend/executor/functions.c b/src/backend/executor/functions.c index ab414e19581..784bd94b0a7 100644 --- a/src/backend/executor/functions.c +++ b/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; diff --git a/src/backend/executor/nodeFunctionscan.c b/src/backend/executor/nodeFunctionscan.c index 66c418c8ba6..b8aecc2b825 100644 --- a/src/backend/executor/nodeFunctionscan.c +++ b/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 diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 53759b80296..5c77aebfe62 100644 --- a/src/backend/parser/gram.y +++ b/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 TriggerOneEvent %type 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 into_clause, OptTempTableName @@ -257,8 +257,8 @@ static void doNegateFloat(Value *v); %type set_rest -%type OptTableElement, ConstraintElem, tableFuncElement -%type columnDef, tableFuncColumnDef +%type TableElement, ConstraintElem, TableFuncElement +%type columnDef %type def_elem %type 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 - { - if ($3 != NULL) - $$ = lappend($1, $3); - else - $$ = $1; - } - | OptTableElement - { - if ($1 != NULL) - $$ = makeList1($1); - else - $$ = NIL; - } + TableElementList { $$ = $1; } | /*EMPTY*/ { $$ = NIL; } ; -OptTableElement: +TableElementList: + TableElementList ',' TableElement + { + $$ = lappend($1, $3); + } + | TableElement + { + $$ = makeList1($1); + } + ; + +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 - { - if ($3 != NULL) - $$ = lappend($1, $3); - else - $$ = $1; - } - | tableFuncElement - { - if ($1 != NULL) - $$ = makeList1($1); - else - $$ = NIL; - } +OptTableFuncElementList: + TableFuncElementList { $$ = $1; } | /*EMPTY*/ { $$ = NIL; } ; -tableFuncElement: - tableFuncColumnDef { $$ = $1; } +TableFuncElementList: + TableFuncElementList ',' TableFuncElement + { + $$ = lappend($1, $3); + } + | TableFuncElement + { + $$ = makeList1($1); + } ; -tableFuncColumnDef: ColId Typename +TableFuncElement: ColId Typename { ColumnDef *n = makeNode(ColumnDef); n->colname = $1; diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c index 6a4bda5d62b..872c03ca9f5 100644 --- a/src/backend/parser/parse_relation.c +++ b/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; -} diff --git a/src/backend/utils/cache/lsyscache.c b/src/backend/utils/cache/lsyscache.c index 9b5b4cd6f13..4c2838e7af7 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.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 ---------- */ diff --git a/src/include/catalog/pg_type.h b/src/include/catalog/pg_type.h index 6bc4a2a92ae..3d026acc083 100644 --- a/src/include/catalog/pg_type.h +++ b/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 diff --git a/src/include/parser/parse_relation.h b/src/include/parser/parse_relation.h index 7abfd047865..38729a81a64 100644 --- a/src/include/parser/parse_relation.h +++ b/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 */ diff --git a/src/include/utils/lsyscache.h b/src/include/utils/lsyscache.h index ab06031b2c6..c03e1a241e2 100644 --- a/src/include/utils/lsyscache.h +++ b/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);