From 6c91eef7b73b53d3fa7023f487b1ae842f4f9e4d Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 6 Sep 2001 02:07:42 +0000 Subject: [PATCH] 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. --- doc/src/sgml/catalogs.sgml | 12 ++- src/backend/catalog/heap.c | 19 ++--- src/backend/catalog/pg_type.c | 41 +++++----- src/backend/commands/define.c | 20 +++-- src/backend/optimizer/prep/preptlist.c | 63 +++++++++------- src/backend/utils/cache/lsyscache.c | 92 ++++++++--------------- src/bin/pg_dump/pg_dump.c | 22 ++++-- src/include/catalog/pg_type.h | 31 ++++++-- src/include/utils/lsyscache.h | 4 +- src/test/regress/expected/create_type.out | 25 ++++++ src/test/regress/sql/create_type.sql | 26 +++++++ 11 files changed, 209 insertions(+), 146 deletions(-) diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml index 39c05e24bd..0b1ab7e287 100644 --- a/doc/src/sgml/catalogs.sgml +++ b/doc/src/sgml/catalogs.sgml @@ -1,6 +1,6 @@ @@ -243,7 +243,9 @@ The initial value of the transition state. This is a text - field which will be cast to the type of aggtranstype. + field containing the initial value in its external string + representation. If the field is NULL, the transition state + value starts out NULL. @@ -2071,7 +2073,11 @@ typdefault text - ??? + + typdefault is NULL for types without a + default value. If it's not NULL, it contains the external string + representation of the type's default value. + 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,20 +338,9 @@ 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 */ @@ -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; } /* diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index abed81cf82..3395026354 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -22,7 +22,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/bin/pg_dump/pg_dump.c,v 1.227 2001/08/27 20:33:07 tgl Exp $ + * $Header: /cvsroot/pgsql/src/bin/pg_dump/pg_dump.c,v 1.228 2001/09/06 02:07:42 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -1400,7 +1400,10 @@ getTypes(int *numTypes) tinfo[i].typsend = strdup(PQgetvalue(res, i, i_typsend)); tinfo[i].typelem = strdup(PQgetvalue(res, i, i_typelem)); tinfo[i].typdelim = strdup(PQgetvalue(res, i, i_typdelim)); - tinfo[i].typdefault = strdup(PQgetvalue(res, i, i_typdefault)); + if (PQgetisnull(res, i, i_typdefault)) + tinfo[i].typdefault = NULL; + else + tinfo[i].typdefault = strdup(PQgetvalue(res, i, i_typdefault)); tinfo[i].typrelid = strdup(PQgetvalue(res, i, i_typrelid)); tinfo[i].typalign = strdup(PQgetvalue(res, i, i_typalign)); tinfo[i].typstorage = strdup(PQgetvalue(res, i, i_typstorage)); @@ -3167,8 +3170,10 @@ dumpTypes(Archive *fout, FuncInfo *finfo, int numFuncs, "CREATE TYPE %s " "( internallength = %s, externallength = %s,", fmtId(tinfo[i].typname, force_quotes), - tinfo[i].typlen, - tinfo[i].typprtlen); + (strcmp(tinfo[i].typlen, "-1") == 0) ? + "variable" : tinfo[i].typlen, + (strcmp(tinfo[i].typprtlen, "-1") == 0) ? + "variable" : tinfo[i].typprtlen); /* cannot combine these because fmtId uses static result area */ appendPQExpBuffer(q, " input = %s,", fmtId(tinfo[i].typinput, force_quotes)); @@ -3176,9 +3181,14 @@ dumpTypes(Archive *fout, FuncInfo *finfo, int numFuncs, fmtId(tinfo[i].typoutput, force_quotes)); appendPQExpBuffer(q, " send = %s,", fmtId(tinfo[i].typsend, force_quotes)); - appendPQExpBuffer(q, " receive = %s, default = ", + appendPQExpBuffer(q, " receive = %s", fmtId(tinfo[i].typreceive, force_quotes)); - formatStringLiteral(q, tinfo[i].typdefault, CONV_ALL); + + if (tinfo[i].typdefault != NULL) + { + appendPQExpBuffer(q, ", default = "); + formatStringLiteral(q, tinfo[i].typdefault, CONV_ALL); + } if (tinfo[i].isArray) { diff --git a/src/include/catalog/pg_type.h b/src/include/catalog/pg_type.h index d5e0202029..912a47a96c 100644 --- a/src/include/catalog/pg_type.h +++ b/src/include/catalog/pg_type.h @@ -8,7 +8,7 @@ * Portions Copyright (c) 1996-2001, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $Id: pg_type.h,v 1.110 2001/08/25 18:52:43 tgl Exp $ + * $Id: pg_type.h,v 1.111 2001/09/06 02:07:42 tgl Exp $ * * NOTES * the genbki.sh script reads this file and generates .bki @@ -42,11 +42,16 @@ CATALOG(pg_type) BOOTSTRAP int4 typowner; /* - * typlen is the number of bytes we use to represent a value of this - * type, e.g. 4 for an int4. But for a variable length type, typlen - * is -1. + * For a fixed-size type, typlen is the number of bytes we use to + * represent a value of this type, e.g. 4 for an int4. But for a + * variable-length type, typlen is -1. */ int2 typlen; + /* + * typprtlen was once intended to be the length of the external + * representation of a datatype, with the same interpretation as for + * typlen. But it's currently unused. + */ int2 typprtlen; /* @@ -66,8 +71,14 @@ CATALOG(pg_type) BOOTSTRAP * anyway?) */ char typtype; + + /* + * If typisdefined is false, the entry is only a placeholder (forward + * reference). We know the type name, but not yet anything else about it. + */ bool typisdefined; - char typdelim; + + char typdelim; /* delimiter for arrays of this type */ Oid typrelid; /* 0 if not a class type */ /* @@ -82,6 +93,10 @@ CATALOG(pg_type) BOOTSTRAP * typelem != 0 and typlen < 0. */ Oid typelem; + + /* + * I/O conversion procedures for the datatype. + */ regproc typinput; regproc typoutput; regproc typreceive; @@ -123,6 +138,12 @@ CATALOG(pg_type) BOOTSTRAP */ char typstorage; + /* + * typdefault is NULL if the type has no associated default value. + * If it's not NULL, it contains the external representation of the + * type's default value --- this default is used whenever no per-column + * default is specified for a column of the datatype. + */ text typdefault; /* VARIABLE LENGTH FIELD */ } FormData_pg_type; diff --git a/src/include/utils/lsyscache.h b/src/include/utils/lsyscache.h index ac7d9ba100..6b3f8a52ea 100644 --- a/src/include/utils/lsyscache.h +++ b/src/include/utils/lsyscache.h @@ -6,7 +6,7 @@ * Portions Copyright (c) 1996-2001, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $Id: lsyscache.h,v 1.35 2001/08/21 16:36:06 tgl Exp $ + * $Id: lsyscache.h,v 1.36 2001/09/06 02:07:42 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -42,7 +42,7 @@ extern int16 get_typlen(Oid typid); extern bool get_typbyval(Oid typid); extern void get_typlenbyval(Oid typid, int16 *typlen, bool *typbyval); extern char get_typstorage(Oid typid); -extern Datum get_typdefault(Oid typid); +extern bool get_typdefault(Oid typid, Datum *defaultValue); extern int32 get_typavgwidth(Oid typid, int32 typmod); extern int32 get_attavgwidth(Oid relid, AttrNumber attnum); extern bool get_attstatsslot(HeapTuple statstuple, diff --git a/src/test/regress/expected/create_type.out b/src/test/regress/expected/create_type.out index 985c8f7569..06b992280b 100644 --- a/src/test/regress/expected/create_type.out +++ b/src/test/regress/expected/create_type.out @@ -13,3 +13,28 @@ CREATE TYPE city_budget ( output = int44out, element = int4 ); +-- Test type-related default values (broken in releases before PG 7.2) +CREATE TYPE int42 ( + internallength = 4, + input = int4in, + output = int4out, + alignment = int4, + default = 42, + passedbyvalue +); +CREATE TYPE text_w_default ( + internallength = variable, + input = textin, + output = textout, + alignment = int4, + default = 'zippo' +); +CREATE TABLE default_test (f1 text_w_default, f2 int42); +INSERT INTO default_test DEFAULT VALUES; +SELECT * FROM default_test; + f1 | f2 +-------+---- + zippo | 42 +(1 row) + +DROP TABLE default_test; diff --git a/src/test/regress/sql/create_type.sql b/src/test/regress/sql/create_type.sql index 96655eed63..a728bbd8b9 100644 --- a/src/test/regress/sql/create_type.sql +++ b/src/test/regress/sql/create_type.sql @@ -16,3 +16,29 @@ CREATE TYPE city_budget ( element = int4 ); +-- Test type-related default values (broken in releases before PG 7.2) + +CREATE TYPE int42 ( + internallength = 4, + input = int4in, + output = int4out, + alignment = int4, + default = 42, + passedbyvalue +); + +CREATE TYPE text_w_default ( + internallength = variable, + input = textin, + output = textout, + alignment = int4, + default = 'zippo' +); + +CREATE TABLE default_test (f1 text_w_default, f2 int42); + +INSERT INTO default_test DEFAULT VALUES; + +SELECT * FROM default_test; + +DROP TABLE default_test; -- 2.49.0