Обсуждение: Allowing ALTER TYPE to change storage strategy
Hi, From time to time I run into the limitation that ALTER TYPE does not allow changing storage strategy. I've written a bunch of data types over the years - in some cases I simply forgot to specify storage in CREATE TYPE (so it defaulted to PLAIN) or I expected PLAIN to be sufficient and then later wished I could enable TOAST. Obviously, without ALTER TYPE supporting that it's rather tricky. You either need to do dump/restore, or tweak the pg_type catalog directly. So here is an initial patch extending ALTER TYPE to support this. The question is why this was not implemented before - my assumption is this is not simply because no one wanted that. We track the storage in pg_attribute too, and ALTER TABLE allows changing that ... My understanding is that pg_type.typstorage essentially specifies two things: (a) default storage strategy for the attributes with this type, and (b) whether the type implementation is prepared to handle TOAST-ed values or not. And pg_attribute.attstorage has to respect this - when the type says PLAIN then the attribute can't simply use strategy that would enable TOAST. Luckily, this is only a problem when switching typstorage to PLAIN (i.e. when disabling TOAST for the type). The attached v1 patch checks if there are attributes with non-PLAIN storage for this type, and errors out if it finds one. But unfortunately that's not entirely correct, because ALTER TABLE only sets storage for new data. A table may be created with e.g. EXTENDED storage for an attribute, a bunch of rows may be loaded and then the storage for the attribute may be changed to PLAIN. That would pass the check as it's currently in the patch, yet there may be TOAST-ed values for the type with PLAIN storage :-( I'm not entirely sure what to do about this, but I think it's OK to just reject changes in this direction (from non-PLAIN to PLAIN storage). I've never needed it, and it seems pretty useless - it seems fine to just instruct the user to do a dump/restore. In the opposite direction - when switching from PLAIN to a TOAST-enabled storage, or enabling/disabling compression, this is not an issue at all. It's legal for type to specify e.g. EXTENDED but attribute to use PLAIN, for example. So the attached v1 patch simply allows this direction. regards -- Tomas Vondra http://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Вложения
Tomas Vondra <tomas.vondra@2ndquadrant.com> writes:
> My understanding is that pg_type.typstorage essentially specifies two
> things: (a) default storage strategy for the attributes with this type,
> and (b) whether the type implementation is prepared to handle TOAST-ed
> values or not. And pg_attribute.attstorage has to respect this - when
> the type says PLAIN then the attribute can't simply use strategy that
> would enable TOAST.
Check.
> Luckily, this is only a problem when switching typstorage to PLAIN (i.e.
> when disabling TOAST for the type). The attached v1 patch checks if
> there are attributes with non-PLAIN storage for this type, and errors
> out if it finds one. But unfortunately that's not entirely correct,
> because ALTER TABLE only sets storage for new data. A table may be
> created with e.g. EXTENDED storage for an attribute, a bunch of rows may
> be loaded and then the storage for the attribute may be changed to
> PLAIN. That would pass the check as it's currently in the patch, yet
> there may be TOAST-ed values for the type with PLAIN storage :-(
> I'm not entirely sure what to do about this, but I think it's OK to just
> reject changes in this direction (from non-PLAIN to PLAIN storage).
Yeah, I think you should just reject that: once toast-capable, always
toast-capable. Scanning pg_attribute is entirely insufficient because
of race conditions --- and while we accept such races in some other
places, this seems like a place where the risk is too high and the
value too low.
Anybody who really needs to go in that direction still has the alternative
of manually frobbing the catalogs (and taking the responsibility for
races and un-toasting whatever's stored already).
regards, tom lane
On Fri, Feb 28, 2020 at 01:59:49PM -0500, Tom Lane wrote: >Tomas Vondra <tomas.vondra@2ndquadrant.com> writes: >> My understanding is that pg_type.typstorage essentially specifies two >> things: (a) default storage strategy for the attributes with this type, >> and (b) whether the type implementation is prepared to handle TOAST-ed >> values or not. And pg_attribute.attstorage has to respect this - when >> the type says PLAIN then the attribute can't simply use strategy that >> would enable TOAST. > >Check. > >> Luckily, this is only a problem when switching typstorage to PLAIN (i.e. >> when disabling TOAST for the type). The attached v1 patch checks if >> there are attributes with non-PLAIN storage for this type, and errors >> out if it finds one. But unfortunately that's not entirely correct, >> because ALTER TABLE only sets storage for new data. A table may be >> created with e.g. EXTENDED storage for an attribute, a bunch of rows may >> be loaded and then the storage for the attribute may be changed to >> PLAIN. That would pass the check as it's currently in the patch, yet >> there may be TOAST-ed values for the type with PLAIN storage :-( > >> I'm not entirely sure what to do about this, but I think it's OK to just >> reject changes in this direction (from non-PLAIN to PLAIN storage). > >Yeah, I think you should just reject that: once toast-capable, always >toast-capable. Scanning pg_attribute is entirely insufficient because >of race conditions --- and while we accept such races in some other >places, this seems like a place where the risk is too high and the >value too low. > >Anybody who really needs to go in that direction still has the alternative >of manually frobbing the catalogs (and taking the responsibility for >races and un-toasting whatever's stored already). > Yeah. Attached is v2 of the patch, simply rejecting such changes. I think we might check if there are any attributes with the given data type, and allow the change if there are none. That would still allow the change when the type is used only for things like function parameters etc. But we'd also have to check for domains (recursively). One thing I haven't mentioned in the original message is CASCADE. It seems useful to optionally change storage for all attributes with the given data type. But I'm not sure it's actually a good idea, and the amount of code seems non-trivial (it'd have to copy quite a bit of code from ALTER TABLE). I'm also not sure what to do about domains and attributes using those. It's more time/code than what I'm willing spend now, so I'll laeve this as a possible future improvement. regards -- Tomas Vondra http://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Вложения
Tomas Vondra <tomas.vondra@2ndquadrant.com> writes:
> I think we might check if there are any attributes with the given data
> type, and allow the change if there are none. That would still allow the
> change when the type is used only for things like function parameters
> etc. But we'd also have to check for domains (recursively).
Still has race conditions.
> One thing I haven't mentioned in the original message is CASCADE. It
> seems useful to optionally change storage for all attributes with the
> given data type. But I'm not sure it's actually a good idea, and the
> amount of code seems non-trivial (it'd have to copy quite a bit of code
> from ALTER TABLE).
You'd need a moderately strong lock on each such table, which means
there'd be serious deadlock hazards. I'm dubious that it's worth
troubling with.
regards, tom lane
On Fri, Feb 28, 2020 at 08:35:33PM -0500, Tom Lane wrote: >Tomas Vondra <tomas.vondra@2ndquadrant.com> writes: >> I think we might check if there are any attributes with the given data >> type, and allow the change if there are none. That would still allow the >> change when the type is used only for things like function parameters >> etc. But we'd also have to check for domains (recursively). > >Still has race conditions. > Yeah, I have no problem believing that. >> One thing I haven't mentioned in the original message is CASCADE. It >> seems useful to optionally change storage for all attributes with the >> given data type. But I'm not sure it's actually a good idea, and the >> amount of code seems non-trivial (it'd have to copy quite a bit of code >> from ALTER TABLE). > >You'd need a moderately strong lock on each such table, which means >there'd be serious deadlock hazards. I'm dubious that it's worth >troubling with. > Yeah, I don't plan to do this in v1 (and I have no immediate plan to work on it after that). But I wonder how is the deadlock risk any different compared e.g. to DROP TYPE ... CASCADE? regards -- Tomas Vondra http://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Tomas Vondra <tomas.vondra@2ndquadrant.com> writes:
> On Fri, Feb 28, 2020 at 08:35:33PM -0500, Tom Lane wrote:
>> You'd need a moderately strong lock on each such table, which means
>> there'd be serious deadlock hazards. I'm dubious that it's worth
>> troubling with.
> Yeah, I don't plan to do this in v1 (and I have no immediate plan to
> work on it after that). But I wonder how is the deadlock risk any
> different compared e.g. to DROP TYPE ... CASCADE?
True, but dropping a type you're actively using seems pretty improbable;
whereas the whole point of the patch you're proposing is that people
*would* want to use it in production.
regards, tom lane
I started to look at this patch with fresh eyes after reading the patch for adding binary I/O for ltree, https://www.postgresql.org/message-id/flat/CANmj9Vxx50jOo1L7iSRxd142NyTz6Bdcgg7u9P3Z8o0=HGkYyQ@mail.gmail.com and realizing that the only reasonable way to tackle that problem is to provide an ALTER TYPE command that can set the binary I/O functions for an existing type. (One might think that it'd be acceptable to UPDATE the pg_type row directly; but that wouldn't take care of dependencies properly, and it also wouldn't handle the domain issues I discuss below.) There are other properties too that can be set in CREATE TYPE but we have no convenient way to adjust later, though it'd be reasonable to want to do so. I do not think that we want to invent bespoke syntax for each property. The more such stuff we cram into ALTER TYPE, the bigger the risk of conflicting with some future SQL extension. Moreover, since CREATE TYPE for base types already uses the "( keyword = value, ... )" syntax for these properties, and we have a similar precedent in CREATE/ALTER OPERATOR, it seems to me that the syntax we want here is ALTER TYPE typename SET ( keyword = value [, ... ] ) Attached is a stab at doing it that way, and implementing setting of the binary I/O functions for good measure. (It'd be reasonable to add more stuff, like setting the other support functions, but this is enough for the immediate discussion.) The main thing I'm not too happy about is what to do about domains. Your v2 patch supposed that it's okay to allow ALTER TYPE on domains, but I'm not sure we want to go that way, and if we do there's certainly a bunch more work that has to be done. Up to now the system has supposed that domains inherit all these properties from their base types. I'm not certain exactly how far that assumption has propagated, but there's at least one place that implicitly assumes it: pg_dump has no logic for adjusting a domain to have different storage or support functions than the base type had. So as v2 stands, a custom storage option on a domain would be lost in dump/reload. Another issue that would become a big problem if we allow domains to have custom I/O functions is that the wire protocol transmits the base type's OID, not the domain's OID, for an output column that is of a domain type. A client that expected a particular output format on the strength of what it was told the column type was would be in for a big surprise. Certainly we could fix pg_dump if we had a mind to, but changing the wire protocol for this would have unpleasant ramifications. And I'm worried about whether there are other places in the system that are also making this sort of assumption. I'm also not very convinced that we *want* to allow domains to vary from their base types in this way. The primary use-case I can think of for ALTER TYPE SET is in extension update scripts, and an extension would almost surely wish for any domains over its type to automatically absorb whatever changes of this sort it wants to make. So I think there are two distinct paths we could take here: * Decide that it's okay to allow domains to vary from their base type in these properties. Teach pg_dump to cope with that, and stand ready to fix any other bugs we find, and have some story to tell the people whose clients we break. Possibly add a CASCADE option to ALTER TYPE SET, with the semantics of adjusting dependent domains to match. (This is slightly less scary than the CASCADE semantics you had in mind, because it would only affect pg_type entries not tables.) * Decide that we'll continue to require domains to match their base type in all these properties. That means refusing to allow ALTER on a domain per se, and automatically cascading these changes to dependent domains. In the v3 patch below, I've ripped out the ALTER DOMAIN syntax on the assumption that we'd do the latter; but I've not written the cascade recursion logic, because that seemed like a lot of work to do in advance of having consensus on it being a good idea. I've also restricted the code to work just on base types, because it's far from apparent to me that it makes any sense to allow any of these operations on derived types such as composites or ranges. Again, there's a fair amount of code that is not going to be prepared for such a type to have properties that it could not have at creation, and I don't see a use-case that really justifies breaking those expectations. Thoughts? regards, tom lane diff --git a/doc/src/sgml/ref/alter_type.sgml b/doc/src/sgml/ref/alter_type.sgml index 67be1dd..3465d3f 100644 --- a/doc/src/sgml/ref/alter_type.sgml +++ b/doc/src/sgml/ref/alter_type.sgml @@ -30,6 +30,7 @@ ALTER TYPE <replaceable class="parameter">name</replaceable> RENAME TO <replacea ALTER TYPE <replaceable class="parameter">name</replaceable> SET SCHEMA <replaceable class="parameter">new_schema</replaceable> ALTER TYPE <replaceable class="parameter">name</replaceable> ADD VALUE [ IF NOT EXISTS ] <replaceable class="parameter">new_enum_value</replaceable>[ { BEFORE | AFTER } <replaceable class="parameter">neighbor_enum_value</replaceable>] ALTER TYPE <replaceable class="parameter">name</replaceable> RENAME VALUE <replaceable class="parameter">existing_enum_value</replaceable>TO <replaceable class="parameter">new_enum_value</replaceable> +ALTER TYPE <replaceable class="parameter">name</replaceable> SET ( <replaceable class="parameter">property</replaceable>= <replaceable class="parameter">value</replaceable> [, ... ] ) <phrase>where <replaceable class="parameter">action</replaceable> is one of:</phrase> @@ -70,7 +71,7 @@ ALTER TYPE <replaceable class="parameter">name</replaceable> RENAME VALUE <repla </varlistentry> <varlistentry> - <term><literal>SET DATA TYPE</literal></term> + <term><literal>ALTER ATTRIBUTE ... SET DATA TYPE</literal></term> <listitem> <para> This form changes the type of an attribute of a composite type. @@ -135,6 +136,58 @@ ALTER TYPE <replaceable class="parameter">name</replaceable> RENAME VALUE <repla </para> </listitem> </varlistentry> + + <varlistentry> + <term> + <literal>SET ( <replaceable class="parameter">property</replaceable> = <replaceable class="parameter">value</replaceable>[, ... ] )</literal> + </term> + <listitem> + <para> + This form is only applicable to base types. It allows adjustment of a + subset of the base-type properties that can be set in <command>CREATE + TYPE</command>. Specifically, these properties can be changed: + <itemizedlist> + <listitem> + <para> + <literal>RECEIVE</literal> can be set to the name of a binary input + function, or <literal>NONE</literal> to remove the type's binary + input function. Using this option requires superuser privilege. + </para> + </listitem> + <listitem> + <para> + <literal>SEND</literal> can be set to the name of a binary output + function, or <literal>NONE</literal> to remove the type's binary + output function. Using this option requires superuser privilege. + </para> + </listitem> + <listitem> + <para> + <literal>STORAGE</literal><indexterm> + <primary>TOAST</primary> + <secondary>per-type storage settings</secondary> + </indexterm> + can be set to <literal>plain</literal>, + <literal>extended</literal>, <literal>external</literal>, + or <literal>main</literal> (see <xref linkend="storage-toast"/> for + more information about what these mean). However, changing + from <literal>plain</literal> to another setting requires superuser + privilege (because it requires that the type's C functions all be + TOAST-ready), and changing to <literal>plain</literal> from another + setting is not allowed at all (since the type may already have + TOASTed values present in the database). Note that changing this + option doesn't by itself change any stored data, it just sets the + default TOAST strategy to be used for table columns created in the + future. See <xref linkend="sql-altertable"/> to change the TOAST + strategy for existing table columns. + </para> + </listitem> + </itemizedlist> + See <xref linkend="sql-createtype"/> for more details about these + type properties. + </para> + </listitem> + </varlistentry> </variablelist> </para> @@ -156,7 +209,7 @@ ALTER TYPE <replaceable class="parameter">name</replaceable> RENAME VALUE <repla doesn't do anything you couldn't do by dropping and recreating the type. However, a superuser can alter ownership of any type anyway.) To add an attribute or alter an attribute type, you must also - have <literal>USAGE</literal> privilege on the data type. + have <literal>USAGE</literal> privilege on the attribute's data type. </para> </refsect1> @@ -353,7 +406,20 @@ ALTER TYPE colors ADD VALUE 'orange' AFTER 'red'; To rename an enum value: <programlisting> ALTER TYPE colors RENAME VALUE 'purple' TO 'mauve'; -</programlisting></para> +</programlisting> + </para> + + <para> + To create binary I/O functions for an existing base type: +<programlisting> +CREATE FUNCTION mytypesend(mytype) RETURNS bytea ...; +CREATE FUNCTION mytyperecv(internal, oid, integer) RETURNS mytype ...; +ALTER TYPE mytype SET ( + send = mytypesend, + receive = mytyperecv +); +</programlisting> + </para> </refsect1> <refsect1> diff --git a/src/backend/catalog/pg_type.c b/src/backend/catalog/pg_type.c index 8d7572d..99398d2 100644 --- a/src/backend/catalog/pg_type.c +++ b/src/backend/catalog/pg_type.c @@ -155,8 +155,8 @@ TypeShellMake(const char *typeName, Oid typeNamespace, Oid ownerId) * Create dependencies. We can/must skip this in bootstrap mode. */ if (!IsBootstrapProcessingMode()) - GenerateTypeDependencies(typoid, - (Form_pg_type) GETSTRUCT(tup), + GenerateTypeDependencies(tup, + pg_type_desc, NULL, NULL, 0, @@ -487,8 +487,8 @@ TypeCreate(Oid newTypeOid, * Create dependencies. We can/must skip this in bootstrap mode. */ if (!IsBootstrapProcessingMode()) - GenerateTypeDependencies(typeObjectId, - (Form_pg_type) GETSTRUCT(tup), + GenerateTypeDependencies(tup, + pg_type_desc, (defaultTypeBin ? stringToNode(defaultTypeBin) : NULL), @@ -515,12 +515,16 @@ TypeCreate(Oid newTypeOid, * GenerateTypeDependencies: build the dependencies needed for a type * * Most of what this function needs to know about the type is passed as the - * new pg_type row, typeForm. But we can't get at the varlena fields through - * that, so defaultExpr and typacl are passed separately. (typacl is really + * new pg_type row, typeTuple. We make callers pass the pg_type Relation + * as well, so that we have easy access to a tuple descriptor for the row. + * + * While this is able to extract the defaultExpr and typacl from the tuple, + * doing so is relatively expensive, and callers may have those values at + * hand already. Pass those if handy, otherwise pass NULL. (typacl is really * "Acl *", but we declare it "void *" to avoid including acl.h in pg_type.h.) * - * relationKind and isImplicitArray aren't visible in the pg_type row either, - * so they're also passed separately. + * relationKind and isImplicitArray are likewise somewhat expensive to deduce + * from the tuple, so we make callers pass those (they're not optional). * * isDependentType is true if this is an implicit array or relation rowtype; * that means it doesn't need its own dependencies on owner etc. @@ -534,8 +538,8 @@ TypeCreate(Oid newTypeOid, * that type will become a member of the extension.) */ void -GenerateTypeDependencies(Oid typeObjectId, - Form_pg_type typeForm, +GenerateTypeDependencies(HeapTuple typeTuple, + Relation typeCatalog, Node *defaultExpr, void *typacl, char relationKind, /* only for relation rowtypes */ @@ -543,9 +547,30 @@ GenerateTypeDependencies(Oid typeObjectId, bool isDependentType, bool rebuild) { + Form_pg_type typeForm = (Form_pg_type) GETSTRUCT(typeTuple); + Oid typeObjectId = typeForm->oid; + Datum datum; + bool isNull; ObjectAddress myself, referenced; + /* Extract defaultExpr if caller didn't pass it */ + if (defaultExpr == NULL) + { + datum = heap_getattr(typeTuple, Anum_pg_type_typdefaultbin, + RelationGetDescr(typeCatalog), &isNull); + if (!isNull) + defaultExpr = stringToNode(TextDatumGetCString(datum)); + } + /* Extract typacl if caller didn't pass it */ + if (typacl == NULL) + { + datum = heap_getattr(typeTuple, Anum_pg_type_typacl, + RelationGetDescr(typeCatalog), &isNull); + if (!isNull) + typacl = DatumGetAclPCopy(datum); + } + /* If rebuild, first flush old dependencies, except extension deps */ if (rebuild) { diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c index 5209736..65f4b49 100644 --- a/src/backend/commands/typecmds.c +++ b/src/backend/commands/typecmds.c @@ -482,24 +482,6 @@ DefineType(ParseState *pstate, List *names, List *parameters) errmsg("type output function %s must return type %s", NameListToString(outputName), "cstring"))); } - if (receiveOid) - { - resulttype = get_func_rettype(receiveOid); - if (resulttype != typoid) - ereport(ERROR, - (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), - errmsg("type receive function %s must return type %s", - NameListToString(receiveName), typeName))); - } - if (sendOid) - { - resulttype = get_func_rettype(sendOid); - if (resulttype != BYTEAOID) - ereport(ERROR, - (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), - errmsg("type send function %s must return type %s", - NameListToString(sendName), "bytea"))); - } /* * Convert typmodin/out function proc names to OIDs. @@ -523,9 +505,12 @@ DefineType(ParseState *pstate, List *names, List *parameters) * minimum sane check would be for execute-with-grant-option. But we * don't have a way to make the type go away if the grant option is * revoked, so ownership seems better. + * + * XXX For now, this is all unnecessary given the superuser check above. + * If we ever relax that, these calls likely should be moved into + * findTypeInputFunction et al, where they could be shared by AlterType. */ #ifdef NOT_USED - /* XXX this is unnecessary given the superuser check above */ if (inputOid && !pg_proc_ownercheck(inputOid, GetUserId())) aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_FUNCTION, NameListToString(inputName)); @@ -558,6 +543,8 @@ DefineType(ParseState *pstate, List *names, List *parameters) * volatility marking might be just an error-of-omission and not a true * indication of how the function behaves, we'll let it pass as a warning * for now. + * + * XXX move these into findTypeInputFunction et al? */ if (inputOid && func_volatile(inputOid) == PROVOLATILE_VOLATILE) ereport(WARNING, @@ -1813,27 +1800,33 @@ findTypeReceiveFunction(List *procname, Oid typeOid) /* * Receive functions can take a single argument of type INTERNAL, or three - * arguments (internal, typioparam OID, typmod). + * arguments (internal, typioparam OID, typmod). They must return the + * expected type. */ argList[0] = INTERNALOID; procOid = LookupFuncName(procname, 1, argList, true); - if (OidIsValid(procOid)) - return procOid; + if (!OidIsValid(procOid)) + { + argList[1] = OIDOID; + argList[2] = INT4OID; - argList[1] = OIDOID; - argList[2] = INT4OID; + procOid = LookupFuncName(procname, 3, argList, true); + } - procOid = LookupFuncName(procname, 3, argList, true); - if (OidIsValid(procOid)) - return procOid; + if (!OidIsValid(procOid)) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_FUNCTION), + errmsg("function %s does not exist", + func_signature_string(procname, 1, NIL, argList)))); - ereport(ERROR, - (errcode(ERRCODE_UNDEFINED_FUNCTION), - errmsg("function %s does not exist", - func_signature_string(procname, 1, NIL, argList)))); + if (get_func_rettype(procOid) != typeOid) + ereport(ERROR, + (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("type receive function %s must return type %s", + NameListToString(procname), format_type_be(typeOid)))); - return InvalidOid; /* keep compiler quiet */ + return procOid; } static Oid @@ -1843,20 +1836,24 @@ findTypeSendFunction(List *procname, Oid typeOid) Oid procOid; /* - * Send functions can take a single argument of the type. + * Send functions take a single argument of the type and return bytea. */ argList[0] = typeOid; procOid = LookupFuncName(procname, 1, argList, true); - if (OidIsValid(procOid)) - return procOid; + if (!OidIsValid(procOid)) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_FUNCTION), + errmsg("function %s does not exist", + func_signature_string(procname, 1, NIL, argList)))); - ereport(ERROR, - (errcode(ERRCODE_UNDEFINED_FUNCTION), - errmsg("function %s does not exist", - func_signature_string(procname, 1, NIL, argList)))); + if (get_func_rettype(procOid) != BYTEAOID) + ereport(ERROR, + (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("type send function %s must return type %s", + NameListToString(procname), "bytea"))); - return InvalidOid; /* keep compiler quiet */ + return procOid; } static Oid @@ -2186,9 +2183,6 @@ AlterDomainDefault(List *names, Node *defaultRaw) Relation rel; char *defaultValue; Node *defaultExpr = NULL; /* NULL if no default specified */ - Acl *typacl; - Datum aclDatum; - bool isNull; Datum new_record[Natts_pg_type]; bool new_record_nulls[Natts_pg_type]; bool new_record_repl[Natts_pg_type]; @@ -2241,6 +2235,7 @@ AlterDomainDefault(List *names, Node *defaultRaw) (IsA(defaultExpr, Const) &&((Const *) defaultExpr)->constisnull)) { /* Default is NULL, drop it */ + defaultExpr = NULL; new_record_nulls[Anum_pg_type_typdefaultbin - 1] = true; new_record_repl[Anum_pg_type_typdefaultbin - 1] = true; new_record_nulls[Anum_pg_type_typdefault - 1] = true; @@ -2281,19 +2276,11 @@ AlterDomainDefault(List *names, Node *defaultRaw) CatalogTupleUpdate(rel, &tup->t_self, newtuple); - /* Must extract ACL for use of GenerateTypeDependencies */ - aclDatum = heap_getattr(newtuple, Anum_pg_type_typacl, - RelationGetDescr(rel), &isNull); - if (isNull) - typacl = NULL; - else - typacl = DatumGetAclPCopy(aclDatum); - /* Rebuild dependencies */ - GenerateTypeDependencies(domainoid, - (Form_pg_type) GETSTRUCT(newtuple), + GenerateTypeDependencies(newtuple, + rel, defaultExpr, - typacl, + NULL, /* don't have typacl handy */ 0, /* relation kind is n/a */ false, /* a domain isn't an implicit array */ false, /* nor is it any kind of dependent type */ @@ -3709,3 +3696,249 @@ AlterTypeNamespaceInternal(Oid typeOid, Oid nspOid, return oldNspOid; } + +/* + * AlterType + * routine implementing ALTER TYPE <type> SET (option = ...). + * + * Currently, only STORAGE option and binary I/O functions can be changed. + */ +ObjectAddress +AlterType(AlterTypeStmt *stmt) +{ + ObjectAddress address; + Relation catalog; + TypeName *typename; + HeapTuple tup; + HeapTuple newtup; + Oid typeOid; + Form_pg_type typForm; + Datum values[Natts_pg_type]; + bool nulls[Natts_pg_type]; + bool replaces[Natts_pg_type]; + bool requireSuper = false; + char storage = '\0'; + bool updateStorage = false; + List *receiveName = NIL; + bool updateReceive = false; + Oid receiveOid; + List *sendName = NIL; + bool updateSend = false; + Oid sendOid; + ListCell *pl; + + catalog = table_open(TypeRelationId, RowExclusiveLock); + + /* Make a TypeName so we can use standard type lookup machinery */ + typename = makeTypeNameFromNameList(stmt->typeName); + tup = typenameType(NULL, typename, NULL); + + typeOid = typeTypeId(tup); + typForm = (Form_pg_type) GETSTRUCT(tup); + + /* Process options */ + foreach(pl, stmt->options) + { + DefElem *defel = (DefElem *) lfirst(pl); + + if (strcmp(defel->defname, "storage") == 0) + { + char *a = defGetString(defel); + + if (pg_strcasecmp(a, "plain") == 0) + storage = 'p'; + else if (pg_strcasecmp(a, "external") == 0) + storage = 'e'; + else if (pg_strcasecmp(a, "extended") == 0) + storage = 'x'; + else if (pg_strcasecmp(a, "main") == 0) + storage = 'm'; + else + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("storage \"%s\" not recognized", a))); + + /* + * Validate the storage request. If the type isn't varlena, it + * certainly doesn't support non-PLAIN storage. + */ + if (storage != 'p' && typForm->typlen != -1) + ereport(ERROR, + (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("fixed-size types must have storage PLAIN"))); + + /* + * Switching from PLAIN to non-PLAIN is allowed, but it requires + * superuser, since we can't validate that the type's C functions + * will support it. Switching from non-PLAIN to PLAIN is + * disallowed outright, because it's not practical to ensure that + * no tables have toasted values of the type. Switching among + * different non-PLAIN settings is OK, since it just constitutes a + * change in the strategy requested for columns created in the + * future. + */ + if (storage != 'p' && typForm->typstorage == 'p') + requireSuper = true; + else if (storage == 'p' && typForm->typstorage != 'p') + ereport(ERROR, + (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("cannot change type's storage to PLAIN"))); + + updateStorage = true; + } + else if (strcmp(defel->defname, "receive") == 0) + { + if (defel->arg == NULL) + receiveName = NIL; /* NONE, removes the function */ + else + receiveName = defGetQualifiedName(defel); + updateReceive = true; + /* Replacing an I/O function requires superuser. */ + requireSuper = true; + } + else if (strcmp(defel->defname, "send") == 0) + { + if (defel->arg == NULL) + sendName = NIL; /* NONE, removes the function */ + else + sendName = defGetQualifiedName(defel); + updateSend = true; + /* Replacing an I/O function requires superuser. */ + requireSuper = true; + } + + /* + * The rest of the options that CREATE accepts cannot be changed. + * Check for them so that we can give a meaningful error message. + */ + else if (strcmp(defel->defname, "input") == 0 || + strcmp(defel->defname, "output") == 0 || + strcmp(defel->defname, "typmod_in") == 0 || + strcmp(defel->defname, "typmod_out") == 0 || + strcmp(defel->defname, "analyze") == 0 || + strcmp(defel->defname, "internallength") == 0 || + strcmp(defel->defname, "passedbyvalue") == 0 || + strcmp(defel->defname, "alignment") == 0 || + strcmp(defel->defname, "like") == 0 || + strcmp(defel->defname, "category") == 0 || + strcmp(defel->defname, "preferred") == 0 || + strcmp(defel->defname, "default") == 0 || + strcmp(defel->defname, "element") == 0 || + strcmp(defel->defname, "delimiter") == 0 || + strcmp(defel->defname, "collatable") == 0) + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("type attribute \"%s\" cannot be changed", + defel->defname))); + else + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("type attribute \"%s\" not recognized", + defel->defname))); + } + + /* + * Permissions check. Require superuser if we decided the command + * requires that, else must own the type. + */ + if (requireSuper) + { + if (!superuser()) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("must be superuser to alter a type"))); + } + else + { + if (!pg_type_ownercheck(typeOid, GetUserId())) + aclcheck_error_type(ACLCHECK_NOT_OWNER, typeOid); + } + + /* + * We disallow all forms of ALTER TYPE SET on types that aren't plain base + * types. It would for example be highly unsafe, not to mention + * pointless, to change the send/receive functions for a composite type. + * Moreover, pg_dump has no support for changing these properties on + * non-base types. We might weaken this someday, but not now. + * + * Note: if you weaken this enough to allow composite types, be sure to + * adjust the GenerateTypeDependencies call below. + */ + if (typForm->typtype != TYPTYPE_BASE) + ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("%s is not a base type", + format_type_be(typeOid)))); + + /* + * For the same reasons, don't allow direct alteration of array types. + */ + if (OidIsValid(typForm->typelem) && + get_array_type(typForm->typelem) == typeOid) + ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("%s is not a base type", + format_type_be(typeOid)))); + + /* + * Look up receive and send functions if specified. + * + * As in DefineType, we needn't do any permissions check on the I/O + * functions, since we already insisted on superuser. + */ + if (receiveName) + receiveOid = findTypeReceiveFunction(receiveName, typeOid); + else + receiveOid = InvalidOid; + if (sendName) + sendOid = findTypeSendFunction(sendName, typeOid); + else + sendOid = InvalidOid; + + /* Update the tuple */ + memset(values, 0, sizeof(values)); + memset(nulls, 0, sizeof(nulls)); + memset(replaces, 0, sizeof(replaces)); + + if (updateStorage) + { + replaces[Anum_pg_type_typstorage - 1] = true; + values[Anum_pg_type_typstorage - 1] = CharGetDatum(storage); + } + if (updateReceive) + { + replaces[Anum_pg_type_typreceive - 1] = true; + values[Anum_pg_type_typreceive - 1] = ObjectIdGetDatum(receiveOid); + } + if (updateSend) + { + replaces[Anum_pg_type_typsend - 1] = true; + values[Anum_pg_type_typsend - 1] = ObjectIdGetDatum(sendOid); + } + + newtup = heap_modify_tuple(tup, RelationGetDescr(catalog), + values, nulls, replaces); + + CatalogTupleUpdate(catalog, &newtup->t_self, newtup); + + /* Rebuild dependencies */ + GenerateTypeDependencies(newtup, + catalog, + NULL, /* don't have defaultExpr handy */ + NULL, /* don't have typacl handy */ + 0, /* we rejected composite types above */ + false, /* we rejected implicit arrays above */ + false, /* so it can't be a dependent type */ + true); + + InvokeObjectPostAlterHook(TypeRelationId, typeOid, 0); + + ObjectAddressSet(address, TypeRelationId, typeOid); + + /* Clean up */ + ReleaseSysCache(tup); + + table_close(catalog, RowExclusiveLock); + + return address; +} diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index e04c33e..eaab97f 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -3636,6 +3636,17 @@ _copyAlterOperatorStmt(const AlterOperatorStmt *from) return newnode; } +static AlterTypeStmt * +_copyAlterTypeStmt(const AlterTypeStmt *from) +{ + AlterTypeStmt *newnode = makeNode(AlterTypeStmt); + + COPY_NODE_FIELD(typeName); + COPY_NODE_FIELD(options); + + return newnode; +} + static RuleStmt * _copyRuleStmt(const RuleStmt *from) { @@ -5263,6 +5274,9 @@ copyObjectImpl(const void *from) case T_AlterOperatorStmt: retval = _copyAlterOperatorStmt(from); break; + case T_AlterTypeStmt: + retval = _copyAlterTypeStmt(from); + break; case T_RuleStmt: retval = _copyRuleStmt(from); break; diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index 5b1ba14..88b9129 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -1482,6 +1482,15 @@ _equalAlterOperatorStmt(const AlterOperatorStmt *a, const AlterOperatorStmt *b) } static bool +_equalAlterTypeStmt(const AlterTypeStmt *a, const AlterTypeStmt *b) +{ + COMPARE_NODE_FIELD(typeName); + COMPARE_NODE_FIELD(options); + + return true; +} + +static bool _equalRuleStmt(const RuleStmt *a, const RuleStmt *b) { COMPARE_NODE_FIELD(relation); @@ -3359,6 +3368,9 @@ equal(const void *a, const void *b) case T_AlterOperatorStmt: retval = _equalAlterOperatorStmt(a, b); break; + case T_AlterTypeStmt: + retval = _equalAlterTypeStmt(a, b); + break; case T_RuleStmt: retval = _equalRuleStmt(a, b); break; diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 96e7fdb..7e384f9 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -249,7 +249,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); AlterDatabaseStmt AlterDatabaseSetStmt AlterDomainStmt AlterEnumStmt AlterFdwStmt AlterForeignServerStmt AlterGroupStmt AlterObjectDependsStmt AlterObjectSchemaStmt AlterOwnerStmt - AlterOperatorStmt AlterSeqStmt AlterSystemStmt AlterTableStmt + AlterOperatorStmt AlterTypeStmt AlterSeqStmt AlterSystemStmt AlterTableStmt AlterTblSpcStmt AlterExtensionStmt AlterExtensionContentsStmt AlterForeignTableStmt AlterCompositeTypeStmt AlterUserMappingStmt AlterRoleStmt AlterRoleSetStmt AlterPolicyStmt AlterStatsStmt @@ -847,6 +847,7 @@ stmt : | AlterObjectSchemaStmt | AlterOwnerStmt | AlterOperatorStmt + | AlterTypeStmt | AlterPolicyStmt | AlterSeqStmt | AlterSystemStmt @@ -9367,6 +9368,24 @@ operator_def_arg: /***************************************************************************** * + * ALTER TYPE name SET define + * + * We repurpose ALTER OPERATOR's version of "definition" here + * + *****************************************************************************/ + +AlterTypeStmt: + ALTER TYPE_P any_name SET '(' operator_def_list ')' + { + AlterTypeStmt *n = makeNode(AlterTypeStmt); + n->typeName = $3; + n->options = $6; + $$ = (Node *)n; + } + ; + +/***************************************************************************** + * * ALTER THING name OWNER TO newname * *****************************************************************************/ diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index bb85b5e..b9389c2 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -163,6 +163,7 @@ ClassifyUtilityCommandAsReadOnly(Node *parsetree) case T_AlterTableMoveAllStmt: case T_AlterTableSpaceOptionsStmt: case T_AlterTableStmt: + case T_AlterTypeStmt: case T_AlterUserMappingStmt: case T_CommentStmt: case T_CompositeTypeStmt: @@ -1723,6 +1724,10 @@ ProcessUtilitySlow(ParseState *pstate, address = AlterOperator((AlterOperatorStmt *) parsetree); break; + case T_AlterTypeStmt: + address = AlterType((AlterTypeStmt *) parsetree); + break; + case T_CommentStmt: address = CommentObject((CommentStmt *) parsetree); break; @@ -2908,6 +2913,10 @@ CreateCommandTag(Node *parsetree) tag = "ALTER OPERATOR"; break; + case T_AlterTypeStmt: + tag = "ALTER TYPE"; + break; + case T_AlterTSDictionaryStmt: tag = "ALTER TEXT SEARCH DICTIONARY"; break; @@ -3264,6 +3273,10 @@ GetCommandLogLevel(Node *parsetree) lev = LOGSTMT_DDL; break; + case T_AlterTypeStmt: + lev = LOGSTMT_DDL; + break; + case T_AlterTableMoveAllStmt: case T_AlterTableStmt: lev = LOGSTMT_DDL; diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index b6b08d0..54d0317 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -2150,7 +2150,7 @@ psql_completion(const char *text, int start, int end) else if (Matches("ALTER", "TYPE", MatchAny)) COMPLETE_WITH("ADD ATTRIBUTE", "ADD VALUE", "ALTER ATTRIBUTE", "DROP ATTRIBUTE", - "OWNER TO", "RENAME", "SET SCHEMA"); + "OWNER TO", "RENAME", "SET SCHEMA", "SET ("); /* complete ALTER TYPE <foo> ADD with actions */ else if (Matches("ALTER", "TYPE", MatchAny, "ADD")) COMPLETE_WITH("ATTRIBUTE", "VALUE"); diff --git a/src/include/catalog/pg_type.h b/src/include/catalog/pg_type.h index e1a5ab3..0c73555 100644 --- a/src/include/catalog/pg_type.h +++ b/src/include/catalog/pg_type.h @@ -325,8 +325,8 @@ extern ObjectAddress TypeCreate(Oid newTypeOid, bool typeNotNull, Oid typeCollation); -extern void GenerateTypeDependencies(Oid typeObjectId, - Form_pg_type typeForm, +extern void GenerateTypeDependencies(HeapTuple typeTuple, + Relation typeCatalog, Node *defaultExpr, void *typacl, char relationKind, /* only for relation diff --git a/src/include/commands/typecmds.h b/src/include/commands/typecmds.h index fc18d64..0162bc2 100644 --- a/src/include/commands/typecmds.h +++ b/src/include/commands/typecmds.h @@ -54,4 +54,6 @@ extern Oid AlterTypeNamespaceInternal(Oid typeOid, Oid nspOid, bool errorOnTableType, ObjectAddresses *objsMoved); +extern ObjectAddress AlterType(AlterTypeStmt *stmt); + #endif /* TYPECMDS_H */ diff --git a/src/include/nodes/nodes.h b/src/include/nodes/nodes.h index baced7e..8a76afe 100644 --- a/src/include/nodes/nodes.h +++ b/src/include/nodes/nodes.h @@ -380,6 +380,7 @@ typedef enum NodeTag T_AlterObjectSchemaStmt, T_AlterOwnerStmt, T_AlterOperatorStmt, + T_AlterTypeStmt, T_DropOwnedStmt, T_ReassignOwnedStmt, T_CompositeTypeStmt, diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index da0706a..2039b42 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -2959,9 +2959,8 @@ typedef struct AlterOwnerStmt RoleSpec *newowner; /* the new owner */ } AlterOwnerStmt; - /* ---------------------- - * Alter Operator Set Restrict, Join + * Alter Operator Set ( this-n-that ) * ---------------------- */ typedef struct AlterOperatorStmt @@ -2971,6 +2970,16 @@ typedef struct AlterOperatorStmt List *options; /* List of DefElem nodes */ } AlterOperatorStmt; +/* ------------------------ + * Alter Type Set ( this-n-that ) + * ------------------------ + */ +typedef struct AlterTypeStmt +{ + NodeTag type; + List *typeName; /* type name (possibly qualified) */ + List *options; /* List of DefElem nodes */ +} AlterTypeStmt; /* ---------------------- * Create Rule Statement diff --git a/src/test/regress/expected/create_type.out b/src/test/regress/expected/create_type.out index 8309756..99e6bad 100644 --- a/src/test/regress/expected/create_type.out +++ b/src/test/regress/expected/create_type.out @@ -211,3 +211,64 @@ select format_type('bpchar'::regtype, -1); bpchar (1 row) +-- +-- Test CREATE/ALTER TYPE using a type that's compatible with varchar, +-- so we can re-use those support functions +-- +CREATE TYPE myvarchar; +CREATE FUNCTION myvarcharin(cstring, oid, integer) RETURNS myvarchar +LANGUAGE internal IMMUTABLE PARALLEL SAFE STRICT AS 'varcharin'; +NOTICE: return type myvarchar is only a shell +CREATE FUNCTION myvarcharout(myvarchar) RETURNS cstring +LANGUAGE internal IMMUTABLE PARALLEL SAFE STRICT AS 'varcharout'; +NOTICE: argument type myvarchar is only a shell +CREATE FUNCTION myvarcharsend(myvarchar) RETURNS bytea +LANGUAGE internal STABLE PARALLEL SAFE STRICT AS 'varcharsend'; +NOTICE: argument type myvarchar is only a shell +CREATE FUNCTION myvarcharrecv(internal, oid, integer) RETURNS myvarchar +LANGUAGE internal STABLE PARALLEL SAFE STRICT AS 'varcharrecv'; +NOTICE: return type myvarchar is only a shell +-- fail, it's still a shell: +ALTER TYPE myvarchar SET (storage = extended); +ERROR: type "myvarchar" is only a shell +CREATE TYPE myvarchar ( + input = myvarcharin, + output = myvarcharout, + alignment = integer, + storage = main +); +ALTER TYPE myvarchar SET (storage = plain); -- not allowed +ERROR: cannot change type's storage to PLAIN +ALTER TYPE myvarchar SET (storage = extended); +ALTER TYPE myvarchar SET ( + send = myvarcharsend, + receive = myvarcharrecv +); +SELECT typinput, typoutput, typreceive, typsend, typstorage +FROM pg_type WHERE typname = 'myvarchar'; + typinput | typoutput | typreceive | typsend | typstorage +-------------+--------------+---------------+---------------+------------ + myvarcharin | myvarcharout | myvarcharrecv | myvarcharsend | x +(1 row) + +-- ensure dependencies are straight +DROP FUNCTION myvarcharsend(myvarchar); -- fail +ERROR: cannot drop function myvarcharsend(myvarchar) because other objects depend on it +DETAIL: type myvarchar depends on function myvarcharsend(myvarchar) +function myvarcharin(cstring,oid,integer) depends on type myvarchar +function myvarcharout(myvarchar) depends on type myvarchar +function myvarcharrecv(internal,oid,integer) depends on type myvarchar +HINT: Use DROP ... CASCADE to drop the dependent objects too. +DROP TYPE myvarchar; -- fail +ERROR: cannot drop type myvarchar because other objects depend on it +DETAIL: function myvarcharin(cstring,oid,integer) depends on type myvarchar +function myvarcharout(myvarchar) depends on type myvarchar +function myvarcharsend(myvarchar) depends on type myvarchar +function myvarcharrecv(internal,oid,integer) depends on type myvarchar +HINT: Use DROP ... CASCADE to drop the dependent objects too. +DROP TYPE myvarchar CASCADE; +NOTICE: drop cascades to 4 other objects +DETAIL: drop cascades to function myvarcharin(cstring,oid,integer) +drop cascades to function myvarcharout(myvarchar) +drop cascades to function myvarcharsend(myvarchar) +drop cascades to function myvarcharrecv(internal,oid,integer) diff --git a/src/test/regress/sql/create_type.sql b/src/test/regress/sql/create_type.sql index 3d1deba..f00b848 100644 --- a/src/test/regress/sql/create_type.sql +++ b/src/test/regress/sql/create_type.sql @@ -154,3 +154,49 @@ select format_type('varchar'::regtype, 42); select format_type('bpchar'::regtype, null); -- this behavior difference is intentional select format_type('bpchar'::regtype, -1); + +-- +-- Test CREATE/ALTER TYPE using a type that's compatible with varchar, +-- so we can re-use those support functions +-- +CREATE TYPE myvarchar; + +CREATE FUNCTION myvarcharin(cstring, oid, integer) RETURNS myvarchar +LANGUAGE internal IMMUTABLE PARALLEL SAFE STRICT AS 'varcharin'; + +CREATE FUNCTION myvarcharout(myvarchar) RETURNS cstring +LANGUAGE internal IMMUTABLE PARALLEL SAFE STRICT AS 'varcharout'; + +CREATE FUNCTION myvarcharsend(myvarchar) RETURNS bytea +LANGUAGE internal STABLE PARALLEL SAFE STRICT AS 'varcharsend'; + +CREATE FUNCTION myvarcharrecv(internal, oid, integer) RETURNS myvarchar +LANGUAGE internal STABLE PARALLEL SAFE STRICT AS 'varcharrecv'; + +-- fail, it's still a shell: +ALTER TYPE myvarchar SET (storage = extended); + +CREATE TYPE myvarchar ( + input = myvarcharin, + output = myvarcharout, + alignment = integer, + storage = main +); + +ALTER TYPE myvarchar SET (storage = plain); -- not allowed + +ALTER TYPE myvarchar SET (storage = extended); + +ALTER TYPE myvarchar SET ( + send = myvarcharsend, + receive = myvarcharrecv +); + +SELECT typinput, typoutput, typreceive, typsend, typstorage +FROM pg_type WHERE typname = 'myvarchar'; + +-- ensure dependencies are straight +DROP FUNCTION myvarcharsend(myvarchar); -- fail +DROP TYPE myvarchar; -- fail + +DROP TYPE myvarchar CASCADE;
On Mon, Mar 02, 2020 at 02:11:10PM -0500, Tom Lane wrote: >I started to look at this patch with fresh eyes after reading the patch >for adding binary I/O for ltree, > >https://www.postgresql.org/message-id/flat/CANmj9Vxx50jOo1L7iSRxd142NyTz6Bdcgg7u9P3Z8o0=HGkYyQ@mail.gmail.com > >and realizing that the only reasonable way to tackle that problem is to >provide an ALTER TYPE command that can set the binary I/O functions for >an existing type. (One might think that it'd be acceptable to UPDATE >the pg_type row directly; but that wouldn't take care of dependencies >properly, and it also wouldn't handle the domain issues I discuss below.) >There are other properties too that can be set in CREATE TYPE but we >have no convenient way to adjust later, though it'd be reasonable to >want to do so. > >I do not think that we want to invent bespoke syntax for each property. >The more such stuff we cram into ALTER TYPE, the bigger the risk of >conflicting with some future SQL extension. Moreover, since CREATE TYPE >for base types already uses the "( keyword = value, ... )" syntax for >these properties, and we have a similar precedent in CREATE/ALTER >OPERATOR, it seems to me that the syntax we want here is > > ALTER TYPE typename SET ( keyword = value [, ... ] ) > Agreed, it seems reasonable to use the ALTER OPRATOR precedent. >Attached is a stab at doing it that way, and implementing setting of >the binary I/O functions for good measure. (It'd be reasonable to >add more stuff, like setting the other support functions, but this >is enough for the immediate discussion.) > >The main thing I'm not too happy about is what to do about domains. >Your v2 patch supposed that it's okay to allow ALTER TYPE on domains, >but I'm not sure we want to go that way, and if we do there's certainly >a bunch more work that has to be done. Up to now the system has >supposed that domains inherit all these properties from their base >types. I'm not certain exactly how far that assumption has propagated, >but there's at least one place that implicitly assumes it: pg_dump has >no logic for adjusting a domain to have different storage or support >functions than the base type had. So as v2 stands, a custom storage >option on a domain would be lost in dump/reload. > >Another issue that would become a big problem if we allow domains to >have custom I/O functions is that the wire protocol transmits the >base type's OID, not the domain's OID, for an output column that >is of a domain type. A client that expected a particular output >format on the strength of what it was told the column type was >would be in for a big surprise. > >Certainly we could fix pg_dump if we had a mind to, but changing >the wire protocol for this would have unpleasant ramifications. >And I'm worried about whether there are other places in the system >that are also making this sort of assumption. > >I'm also not very convinced that we *want* to allow domains to vary from >their base types in this way. The primary use-case I can think of for >ALTER TYPE SET is in extension update scripts, and an extension would >almost surely wish for any domains over its type to automatically absorb >whatever changes of this sort it wants to make. > >So I think there are two distinct paths we could take here: > >* Decide that it's okay to allow domains to vary from their base type >in these properties. Teach pg_dump to cope with that, and stand ready >to fix any other bugs we find, and have some story to tell the people >whose clients we break. Possibly add a CASCADE option to >ALTER TYPE SET, with the semantics of adjusting dependent domains >to match. (This is slightly less scary than the CASCADE semantics >you had in mind, because it would only affect pg_type entries not >tables.) > >* Decide that we'll continue to require domains to match their base >type in all these properties. That means refusing to allow ALTER >on a domain per se, and automatically cascading these changes to >dependent domains. > >In the v3 patch below, I've ripped out the ALTER DOMAIN syntax on >the assumption that we'd do the latter; but I've not written the >cascade recursion logic, because that seemed like a lot of work >to do in advance of having consensus on it being a good idea. > I do agree we should do the latter, i.e. maintain the assumption that domains have the same properties as their base type. I can't think of a use case for allowing them to differ, it just didn't occur to me there is this implicit assumption when writing the patch. >I've also restricted the code to work just on base types, because >it's far from apparent to me that it makes any sense to allow any >of these operations on derived types such as composites or ranges. >Again, there's a fair amount of code that is not going to be >prepared for such a type to have properties that it could not >have at creation, and I don't see a use-case that really justifies >breaking those expectations. > Yeah, that makes sense too, I think. regards -- Tomas Vondra http://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Tomas Vondra <tomas.vondra@2ndquadrant.com> writes:
> On Mon, Mar 02, 2020 at 02:11:10PM -0500, Tom Lane wrote:
>> In the v3 patch below, I've ripped out the ALTER DOMAIN syntax on
>> the assumption that we'd do the latter; but I've not written the
>> cascade recursion logic, because that seemed like a lot of work
>> to do in advance of having consensus on it being a good idea.
> I do agree we should do the latter, i.e. maintain the assumption that
> domains have the same properties as their base type. I can't think of a
> use case for allowing them to differ, it just didn't occur to me there
> is this implicit assumption when writing the patch.
Here's a v4 that is rebased over HEAD + the OPAQUE-ectomy that I
proposed at <4110.1583255415@sss.pgh.pa.us>, plus it adds recursion
to domains, and I also added the ability to set typmod I/O and
analyze functions, which seems like functionality that somebody
could possibly wish to add to a type after-the-fact much like
binary I/O.
I thought about allowing the basic I/O functions to be replaced as
well, but I couldn't really convince myself that there's a use-case
for that. In practice you'd probably always just change the
behavior of the existing I/O functions, not want to sub in new ones.
(I kind of wonder, actually, whether there's a use-case for the
NONE options here at all. When would you remove a support function?)
Of the remaining CREATE TYPE options, "category" and "preferred"
could perhaps be changeable but I couldn't get excited about them.
All the others seem like there are gotchas --- for example,
changing a type's collatable property is much harder than it
looks because it'd affect stored views. So this seems like a
reasonable stopping point.
I think this is committable --- how about you?
I've included the OPAQUE-ectomy patches below so that the cfbot
can test this, but they're the same as in the other thread.
regards, tom lane
diff --git a/doc/src/sgml/ref/create_type.sgml b/doc/src/sgml/ref/create_type.sgml
index 175315f..111f8e6 100644
--- a/doc/src/sgml/ref/create_type.sgml
+++ b/doc/src/sgml/ref/create_type.sgml
@@ -823,18 +823,6 @@ CREATE TYPE <replaceable class="parameter">name</replaceable>
function is written in C.
</para>
- <para>
- In <productname>PostgreSQL</productname> versions before 7.3, it
- was customary to avoid creating a shell type at all, by replacing the
- functions' forward references to the type name with the placeholder
- pseudo-type <type>opaque</type>. The <type>cstring</type> arguments and
- results also had to be declared as <type>opaque</type> before 7.3. To
- support loading of old dump files, <command>CREATE TYPE</command> will
- accept I/O functions declared using <type>opaque</type>, but it will issue
- a notice and change the function declarations to use the correct
- types.
- </para>
-
</refsect1>
<refsect1>
diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c
index 540044b..9279c05 100644
--- a/src/backend/commands/functioncmds.c
+++ b/src/backend/commands/functioncmds.c
@@ -1444,50 +1444,6 @@ SetFunctionReturnType(Oid funcOid, Oid newRetType)
/*
- * SetFunctionArgType - change declared argument type of a function
- *
- * As above, but change an argument's type.
- */
-void
-SetFunctionArgType(Oid funcOid, int argIndex, Oid newArgType)
-{
- Relation pg_proc_rel;
- HeapTuple tup;
- Form_pg_proc procForm;
- ObjectAddress func_address;
- ObjectAddress type_address;
-
- pg_proc_rel = table_open(ProcedureRelationId, RowExclusiveLock);
-
- tup = SearchSysCacheCopy1(PROCOID, ObjectIdGetDatum(funcOid));
- if (!HeapTupleIsValid(tup)) /* should not happen */
- elog(ERROR, "cache lookup failed for function %u", funcOid);
- procForm = (Form_pg_proc) GETSTRUCT(tup);
-
- if (argIndex < 0 || argIndex >= procForm->pronargs ||
- procForm->proargtypes.values[argIndex] != OPAQUEOID)
- elog(ERROR, "function %u doesn't take OPAQUE", funcOid);
-
- /* okay to overwrite copied tuple */
- procForm->proargtypes.values[argIndex] = newArgType;
-
- /* update the catalog and its indexes */
- CatalogTupleUpdate(pg_proc_rel, &tup->t_self, tup);
-
- table_close(pg_proc_rel, RowExclusiveLock);
-
- /*
- * Also update the dependency to the new type. Opaque is a pinned type, so
- * there is no old dependency record for it that we would need to remove.
- */
- ObjectAddressSet(type_address, TypeRelationId, newArgType);
- ObjectAddressSet(func_address, ProcedureRelationId, funcOid);
- recordDependencyOn(&func_address, &type_address, DEPENDENCY_NORMAL);
-}
-
-
-
-/*
* CREATE CAST
*/
ObjectAddress
diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c
index 5209736..d6e694e 100644
--- a/src/backend/commands/typecmds.c
+++ b/src/backend/commands/typecmds.c
@@ -163,7 +163,6 @@ DefineType(ParseState *pstate, List *names, List *parameters)
char *array_type;
Oid array_oid;
Oid typoid;
- Oid resulttype;
ListCell *pl;
ObjectAddress address;
@@ -196,8 +195,7 @@ DefineType(ParseState *pstate, List *names, List *parameters)
#endif
/*
- * Look to see if type already exists (presumably as a shell; if not,
- * TypeCreate will complain).
+ * Look to see if type already exists.
*/
typoid = GetSysCacheOid2(TYPENAMENSP, Anum_pg_type_oid,
CStringGetDatum(typeName),
@@ -211,35 +209,37 @@ DefineType(ParseState *pstate, List *names, List *parameters)
{
if (moveArrayTypeName(typoid, typeName, typeNamespace))
typoid = InvalidOid;
+ else
+ ereport(ERROR,
+ (errcode(ERRCODE_DUPLICATE_OBJECT),
+ errmsg("type \"%s\" already exists", typeName)));
}
/*
- * If it doesn't exist, create it as a shell, so that the OID is known for
- * use in the I/O function definitions.
+ * If this command is a parameterless CREATE TYPE, then we're just here to
+ * make a shell type, so do that (or fail if there already is a shell).
*/
- if (!OidIsValid(typoid))
+ if (parameters == NIL)
{
- address = TypeShellMake(typeName, typeNamespace, GetUserId());
- typoid = address.objectId;
- /* Make new shell type visible for modification below */
- CommandCounterIncrement();
-
- /*
- * If the command was a parameterless CREATE TYPE, we're done ---
- * creating the shell type was all we're supposed to do.
- */
- if (parameters == NIL)
- return address;
- }
- else
- {
- /* Complain if dummy CREATE TYPE and entry already exists */
- if (parameters == NIL)
+ if (OidIsValid(typoid))
ereport(ERROR,
(errcode(ERRCODE_DUPLICATE_OBJECT),
errmsg("type \"%s\" already exists", typeName)));
+
+ address = TypeShellMake(typeName, typeNamespace, GetUserId());
+ return address;
}
+ /*
+ * Otherwise, we must already have a shell type, since there is no other
+ * way that the I/O functions could have been created.
+ */
+ if (!OidIsValid(typoid))
+ ereport(ERROR,
+ (errcode(ERRCODE_DUPLICATE_OBJECT),
+ errmsg("type \"%s\" does not exist", typeName),
+ errhint("Create the type as a shell type, then create its I/O functions, then do a full CREATE
TYPE.")));
+
/* Extract the parameters from the parameter list */
foreach(pl, parameters)
{
@@ -445,63 +445,6 @@ DefineType(ParseState *pstate, List *names, List *parameters)
sendOid = findTypeSendFunction(sendName, typoid);
/*
- * Verify that I/O procs return the expected thing. If we see OPAQUE,
- * complain and change it to the correct type-safe choice.
- */
- resulttype = get_func_rettype(inputOid);
- if (resulttype != typoid)
- {
- if (resulttype == OPAQUEOID)
- {
- /* backwards-compatibility hack */
- ereport(WARNING,
- (errmsg("changing return type of function %s from %s to %s",
- NameListToString(inputName), "opaque", typeName)));
- SetFunctionReturnType(inputOid, typoid);
- }
- else
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
- errmsg("type input function %s must return type %s",
- NameListToString(inputName), typeName)));
- }
- resulttype = get_func_rettype(outputOid);
- if (resulttype != CSTRINGOID)
- {
- if (resulttype == OPAQUEOID)
- {
- /* backwards-compatibility hack */
- ereport(WARNING,
- (errmsg("changing return type of function %s from %s to %s",
- NameListToString(outputName), "opaque", "cstring")));
- SetFunctionReturnType(outputOid, CSTRINGOID);
- }
- else
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
- errmsg("type output function %s must return type %s",
- NameListToString(outputName), "cstring")));
- }
- if (receiveOid)
- {
- resulttype = get_func_rettype(receiveOid);
- if (resulttype != typoid)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
- errmsg("type receive function %s must return type %s",
- NameListToString(receiveName), typeName)));
- }
- if (sendOid)
- {
- resulttype = get_func_rettype(sendOid);
- if (resulttype != BYTEAOID)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
- errmsg("type send function %s must return type %s",
- NameListToString(sendName), "bytea")));
- }
-
- /*
* Convert typmodin/out function proc names to OIDs.
*/
if (typmodinName)
@@ -1404,16 +1347,9 @@ DefineRange(CreateRangeStmt *stmt)
}
/*
- * If it doesn't exist, create it as a shell, so that the OID is known for
- * use in the range function definitions.
+ * Unlike DefineType(), we don't insist on a shell type existing first, as
+ * it's only needed if the user wants to specify a canonical function.
*/
- if (!OidIsValid(typoid))
- {
- address = TypeShellMake(typeName, typeNamespace, GetUserId());
- typoid = address.objectId;
- /* Make new shell type visible for modification below */
- CommandCounterIncrement();
- }
/* Extract the parameters from the parameter list */
foreach(lc, stmt->params)
@@ -1502,8 +1438,15 @@ DefineRange(CreateRangeStmt *stmt)
/* Identify support functions, if provided */
if (rangeCanonicalName != NIL)
+ {
+ if (!OidIsValid(typoid))
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("cannot specify a canonical function without a pre-created shell type"),
+ errhint("Create the type as a shell type, then create its canonicalization function, then do a
fullCREATE TYPE.")));
rangeCanonical = findRangeCanonicalFunction(rangeCanonicalName,
typoid);
+ }
else
rangeCanonical = InvalidOid;
@@ -1555,7 +1498,8 @@ DefineRange(CreateRangeStmt *stmt)
0, /* Array dimensions of typbasetype */
false, /* Type NOT NULL */
InvalidOid); /* type's collation (ranges never have one) */
- Assert(typoid == address.objectId);
+ Assert(typoid == InvalidOid || typoid == address.objectId);
+ typoid = address.objectId;
/* Create the entry in pg_range */
RangeCreate(typoid, rangeSubtype, rangeCollation, rangeSubOpclass,
@@ -1695,63 +1639,32 @@ findTypeInputFunction(List *procname, Oid typeOid)
/*
* Input functions can take a single argument of type CSTRING, or three
- * arguments (string, typioparam OID, typmod).
- *
- * For backwards compatibility we allow OPAQUE in place of CSTRING; if we
- * see this, we issue a warning and fix up the pg_proc entry.
+ * arguments (string, typioparam OID, typmod). They must return the
+ * target type.
*/
argList[0] = CSTRINGOID;
procOid = LookupFuncName(procname, 1, argList, true);
- if (OidIsValid(procOid))
- return procOid;
-
- argList[1] = OIDOID;
- argList[2] = INT4OID;
-
- procOid = LookupFuncName(procname, 3, argList, true);
- if (OidIsValid(procOid))
- return procOid;
-
- /* No luck, try it with OPAQUE */
- argList[0] = OPAQUEOID;
-
- procOid = LookupFuncName(procname, 1, argList, true);
-
if (!OidIsValid(procOid))
{
argList[1] = OIDOID;
argList[2] = INT4OID;
procOid = LookupFuncName(procname, 3, argList, true);
+ if (!OidIsValid(procOid))
+ ereport(ERROR,
+ (errcode(ERRCODE_UNDEFINED_FUNCTION),
+ errmsg("function %s does not exist",
+ func_signature_string(procname, 1, NIL, argList))));
}
- if (OidIsValid(procOid))
- {
- /* Found, but must complain and fix the pg_proc entry */
- ereport(WARNING,
- (errmsg("changing argument type of function %s from \"opaque\" to \"cstring\"",
- NameListToString(procname))));
- SetFunctionArgType(procOid, 0, CSTRINGOID);
-
- /*
- * Need CommandCounterIncrement since DefineType will likely try to
- * alter the pg_proc tuple again.
- */
- CommandCounterIncrement();
-
- return procOid;
- }
-
- /* Use CSTRING (preferred) in the error message */
- argList[0] = CSTRINGOID;
-
- ereport(ERROR,
- (errcode(ERRCODE_UNDEFINED_FUNCTION),
- errmsg("function %s does not exist",
- func_signature_string(procname, 1, NIL, argList))));
+ if (get_func_rettype(procOid) != typeOid)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("type input function %s must return type %s",
+ NameListToString(procname), format_type_be(typeOid))));
- return InvalidOid; /* keep compiler quiet */
+ return procOid;
}
static Oid
@@ -1761,48 +1674,25 @@ findTypeOutputFunction(List *procname, Oid typeOid)
Oid procOid;
/*
- * Output functions can take a single argument of the type.
- *
- * For backwards compatibility we allow OPAQUE in place of the actual type
- * name; if we see this, we issue a warning and fix up the pg_proc entry.
+ * Output functions always take a single argument of the type and return
+ * cstring.
*/
argList[0] = typeOid;
procOid = LookupFuncName(procname, 1, argList, true);
- if (OidIsValid(procOid))
- return procOid;
-
- /* No luck, try it with OPAQUE */
- argList[0] = OPAQUEOID;
-
- procOid = LookupFuncName(procname, 1, argList, true);
-
- if (OidIsValid(procOid))
- {
- /* Found, but must complain and fix the pg_proc entry */
- ereport(WARNING,
- (errmsg("changing argument type of function %s from \"opaque\" to %s",
- NameListToString(procname), format_type_be(typeOid))));
- SetFunctionArgType(procOid, 0, typeOid);
-
- /*
- * Need CommandCounterIncrement since DefineType will likely try to
- * alter the pg_proc tuple again.
- */
- CommandCounterIncrement();
-
- return procOid;
- }
-
- /* Use type name, not OPAQUE, in the failure message. */
- argList[0] = typeOid;
+ if (!OidIsValid(procOid))
+ ereport(ERROR,
+ (errcode(ERRCODE_UNDEFINED_FUNCTION),
+ errmsg("function %s does not exist",
+ func_signature_string(procname, 1, NIL, argList))));
- ereport(ERROR,
- (errcode(ERRCODE_UNDEFINED_FUNCTION),
- errmsg("function %s does not exist",
- func_signature_string(procname, 1, NIL, argList))));
+ if (get_func_rettype(procOid) != CSTRINGOID)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("type output function %s must return type %s",
+ NameListToString(procname), "cstring")));
- return InvalidOid; /* keep compiler quiet */
+ return procOid;
}
static Oid
@@ -1813,27 +1703,32 @@ findTypeReceiveFunction(List *procname, Oid typeOid)
/*
* Receive functions can take a single argument of type INTERNAL, or three
- * arguments (internal, typioparam OID, typmod).
+ * arguments (internal, typioparam OID, typmod). They must return the
+ * target type.
*/
argList[0] = INTERNALOID;
procOid = LookupFuncName(procname, 1, argList, true);
- if (OidIsValid(procOid))
- return procOid;
-
- argList[1] = OIDOID;
- argList[2] = INT4OID;
+ if (!OidIsValid(procOid))
+ {
+ argList[1] = OIDOID;
+ argList[2] = INT4OID;
- procOid = LookupFuncName(procname, 3, argList, true);
- if (OidIsValid(procOid))
- return procOid;
+ procOid = LookupFuncName(procname, 3, argList, true);
+ if (!OidIsValid(procOid))
+ ereport(ERROR,
+ (errcode(ERRCODE_UNDEFINED_FUNCTION),
+ errmsg("function %s does not exist",
+ func_signature_string(procname, 1, NIL, argList))));
+ }
- ereport(ERROR,
- (errcode(ERRCODE_UNDEFINED_FUNCTION),
- errmsg("function %s does not exist",
- func_signature_string(procname, 1, NIL, argList))));
+ if (get_func_rettype(procOid) != typeOid)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("type receive function %s must return type %s",
+ NameListToString(procname), format_type_be(typeOid))));
- return InvalidOid; /* keep compiler quiet */
+ return procOid;
}
static Oid
@@ -1843,20 +1738,25 @@ findTypeSendFunction(List *procname, Oid typeOid)
Oid procOid;
/*
- * Send functions can take a single argument of the type.
+ * Send functions always take a single argument of the type and return
+ * bytea.
*/
argList[0] = typeOid;
procOid = LookupFuncName(procname, 1, argList, true);
- if (OidIsValid(procOid))
- return procOid;
+ if (!OidIsValid(procOid))
+ ereport(ERROR,
+ (errcode(ERRCODE_UNDEFINED_FUNCTION),
+ errmsg("function %s does not exist",
+ func_signature_string(procname, 1, NIL, argList))));
- ereport(ERROR,
- (errcode(ERRCODE_UNDEFINED_FUNCTION),
- errmsg("function %s does not exist",
- func_signature_string(procname, 1, NIL, argList))));
+ if (get_func_rettype(procOid) != BYTEAOID)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("type send function %s must return type %s",
+ NameListToString(procname), "bytea")));
- return InvalidOid; /* keep compiler quiet */
+ return procOid;
}
static Oid
diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h
index dede9d7..0992c23 100644
--- a/src/include/commands/defrem.h
+++ b/src/include/commands/defrem.h
@@ -55,7 +55,6 @@ extern Oid ResolveOpClass(List *opclass, Oid attrType,
extern ObjectAddress CreateFunction(ParseState *pstate, CreateFunctionStmt *stmt);
extern void RemoveFunctionById(Oid funcOid);
extern void SetFunctionReturnType(Oid funcOid, Oid newRetType);
-extern void SetFunctionArgType(Oid funcOid, int argIndex, Oid newArgType);
extern ObjectAddress AlterFunction(ParseState *pstate, AlterFunctionStmt *stmt);
extern ObjectAddress CreateCast(CreateCastStmt *stmt);
extern void DropCastById(Oid castOid);
diff --git a/src/test/regress/expected/create_type.out b/src/test/regress/expected/create_type.out
index 8309756..eb55e25 100644
--- a/src/test/regress/expected/create_type.out
+++ b/src/test/regress/expected/create_type.out
@@ -83,8 +83,10 @@ SELECT * FROM default_test;
zippo | 42
(1 row)
+-- We need a shell type to test some CREATE TYPE failure cases with
+CREATE TYPE bogus_type;
-- invalid: non-lowercase quoted identifiers
-CREATE TYPE case_int42 (
+CREATE TYPE bogus_type (
"Internallength" = 4,
"Input" = int42_in,
"Output" = int42_out,
@@ -111,6 +113,20 @@ WARNING: type attribute "Passedbyvalue" not recognized
LINE 7: "Passedbyvalue"
^
ERROR: type input function must be specified
+-- invalid: input/output function incompatibility
+CREATE TYPE bogus_type (INPUT = array_in,
+ OUTPUT = array_out,
+ ELEMENT = int,
+ INTERNALLENGTH = 32);
+ERROR: type input function array_in must return type bogus_type
+DROP TYPE bogus_type;
+-- It no longer is possible to issue CREATE TYPE without making a shell first
+CREATE TYPE bogus_type (INPUT = array_in,
+ OUTPUT = array_out,
+ ELEMENT = int,
+ INTERNALLENGTH = 32);
+ERROR: type "bogus_type" does not exist
+HINT: Create the type as a shell type, then create its I/O functions, then do a full CREATE TYPE.
-- Test stand-alone composite type
CREATE TYPE default_test_row AS (f1 text_w_default, f2 int42);
CREATE FUNCTION get_default_test() RETURNS SETOF default_test_row AS '
@@ -137,28 +153,25 @@ ERROR: type "text_w_default" already exists
DROP TYPE default_test_row CASCADE;
NOTICE: drop cascades to function get_default_test()
DROP TABLE default_test;
--- Check type create with input/output incompatibility
-CREATE TYPE not_existing_type (INPUT = array_in,
- OUTPUT = array_out,
- ELEMENT = int,
- INTERNALLENGTH = 32);
-ERROR: function array_out(not_existing_type) does not exist
--- Check dependency transfer of opaque functions when creating a new type
-CREATE FUNCTION base_fn_in(cstring) RETURNS opaque AS 'boolin'
+-- Check dependencies are established when creating a new type
+CREATE TYPE base_type;
+CREATE FUNCTION base_fn_in(cstring) RETURNS base_type AS 'boolin'
LANGUAGE internal IMMUTABLE STRICT;
-CREATE FUNCTION base_fn_out(opaque) RETURNS opaque AS 'boolout'
+NOTICE: return type base_type is only a shell
+CREATE FUNCTION base_fn_out(base_type) RETURNS cstring AS 'boolout'
LANGUAGE internal IMMUTABLE STRICT;
+NOTICE: argument type base_type is only a shell
CREATE TYPE base_type(INPUT = base_fn_in, OUTPUT = base_fn_out);
-WARNING: changing argument type of function base_fn_out from "opaque" to base_type
-WARNING: changing return type of function base_fn_in from opaque to base_type
-WARNING: changing return type of function base_fn_out from opaque to cstring
DROP FUNCTION base_fn_in(cstring); -- error
ERROR: cannot drop function base_fn_in(cstring) because other objects depend on it
DETAIL: type base_type depends on function base_fn_in(cstring)
function base_fn_out(base_type) depends on type base_type
HINT: Use DROP ... CASCADE to drop the dependent objects too.
-DROP FUNCTION base_fn_out(opaque); -- error
-ERROR: function base_fn_out(opaque) does not exist
+DROP FUNCTION base_fn_out(base_type); -- error
+ERROR: cannot drop function base_fn_out(base_type) because other objects depend on it
+DETAIL: type base_type depends on function base_fn_out(base_type)
+function base_fn_in(cstring) depends on type base_type
+HINT: Use DROP ... CASCADE to drop the dependent objects too.
DROP TYPE base_type; -- error
ERROR: cannot drop type base_type because other objects depend on it
DETAIL: function base_fn_in(cstring) depends on type base_type
diff --git a/src/test/regress/sql/create_type.sql b/src/test/regress/sql/create_type.sql
index 3d1deba..68b04fd 100644
--- a/src/test/regress/sql/create_type.sql
+++ b/src/test/regress/sql/create_type.sql
@@ -84,8 +84,11 @@ INSERT INTO default_test DEFAULT VALUES;
SELECT * FROM default_test;
+-- We need a shell type to test some CREATE TYPE failure cases with
+CREATE TYPE bogus_type;
+
-- invalid: non-lowercase quoted identifiers
-CREATE TYPE case_int42 (
+CREATE TYPE bogus_type (
"Internallength" = 4,
"Input" = int42_in,
"Output" = int42_out,
@@ -94,6 +97,20 @@ CREATE TYPE case_int42 (
"Passedbyvalue"
);
+-- invalid: input/output function incompatibility
+CREATE TYPE bogus_type (INPUT = array_in,
+ OUTPUT = array_out,
+ ELEMENT = int,
+ INTERNALLENGTH = 32);
+
+DROP TYPE bogus_type;
+
+-- It no longer is possible to issue CREATE TYPE without making a shell first
+CREATE TYPE bogus_type (INPUT = array_in,
+ OUTPUT = array_out,
+ ELEMENT = int,
+ INTERNALLENGTH = 32);
+
-- Test stand-alone composite type
CREATE TYPE default_test_row AS (f1 text_w_default, f2 int42);
@@ -119,20 +136,15 @@ DROP TYPE default_test_row CASCADE;
DROP TABLE default_test;
--- Check type create with input/output incompatibility
-CREATE TYPE not_existing_type (INPUT = array_in,
- OUTPUT = array_out,
- ELEMENT = int,
- INTERNALLENGTH = 32);
-
--- Check dependency transfer of opaque functions when creating a new type
-CREATE FUNCTION base_fn_in(cstring) RETURNS opaque AS 'boolin'
+-- Check dependencies are established when creating a new type
+CREATE TYPE base_type;
+CREATE FUNCTION base_fn_in(cstring) RETURNS base_type AS 'boolin'
LANGUAGE internal IMMUTABLE STRICT;
-CREATE FUNCTION base_fn_out(opaque) RETURNS opaque AS 'boolout'
+CREATE FUNCTION base_fn_out(base_type) RETURNS cstring AS 'boolout'
LANGUAGE internal IMMUTABLE STRICT;
CREATE TYPE base_type(INPUT = base_fn_in, OUTPUT = base_fn_out);
DROP FUNCTION base_fn_in(cstring); -- error
-DROP FUNCTION base_fn_out(opaque); -- error
+DROP FUNCTION base_fn_out(base_type); -- error
DROP TYPE base_type; -- error
DROP TYPE base_type CASCADE;
diff --git a/doc/src/sgml/datatype.sgml b/doc/src/sgml/datatype.sgml
index d1d0331..410eaed 100644
--- a/doc/src/sgml/datatype.sgml
+++ b/doc/src/sgml/datatype.sgml
@@ -4827,10 +4827,6 @@ SELECT * FROM pg_attribute
<primary>unknown</primary>
</indexterm>
- <indexterm zone="datatype-pseudo">
- <primary>opaque</primary>
- </indexterm>
-
<para>
The <productname>PostgreSQL</productname> type system contains a
number of special-purpose entries that are collectively called
@@ -4953,12 +4949,6 @@ SELECT * FROM pg_attribute
<entry>Identifies a not-yet-resolved type, e.g. of an undecorated
string literal.</entry>
</row>
-
- <row>
- <entry><type>opaque</type></entry>
- <entry>An obsolete type name that formerly served many of the above
- purposes.</entry>
- </row>
</tbody>
</tgroup>
</table>
diff --git a/doc/src/sgml/ref/create_language.sgml b/doc/src/sgml/ref/create_language.sgml
index af9d115..2243ee6 100644
--- a/doc/src/sgml/ref/create_language.sgml
+++ b/doc/src/sgml/ref/create_language.sgml
@@ -211,16 +211,6 @@ CREATE [ OR REPLACE ] [ TRUSTED ] [ PROCEDURAL ] LANGUAGE <replaceable class="pa
database, which will cause it to be available automatically in
all subsequently-created databases.
</para>
-
- <para>
- In <productname>PostgreSQL</productname> versions before 7.3, it was
- necessary to declare handler functions as returning the placeholder
- type <type>opaque</type>, rather than <type>language_handler</type>.
- To support loading
- of old dump files, <command>CREATE LANGUAGE</command> will accept a function
- declared as returning <type>opaque</type>, but it will issue a notice and
- change the function's declared return type to <type>language_handler</type>.
- </para>
</refsect1>
<refsect1 id="sql-createlanguage-examples">
diff --git a/doc/src/sgml/ref/create_trigger.sgml b/doc/src/sgml/ref/create_trigger.sgml
index 3339a4b..3b8f25e 100644
--- a/doc/src/sgml/ref/create_trigger.sgml
+++ b/doc/src/sgml/ref/create_trigger.sgml
@@ -543,15 +543,6 @@ UPDATE OF <replaceable>column_name1</replaceable> [, <replaceable>column_name2</
row-level triggers with transition relations cannot be defined on
partitions or inheritance child tables.
</para>
-
- <para>
- In <productname>PostgreSQL</productname> versions before 7.3, it was
- necessary to declare trigger functions as returning the placeholder
- type <type>opaque</type>, rather than <type>trigger</type>. To support loading
- of old dump files, <command>CREATE TRIGGER</command> will accept a function
- declared as returning <type>opaque</type>, but it will issue a notice and
- change the function's declared return type to <type>trigger</type>.
- </para>
</refsect1>
<refsect1 id="sql-createtrigger-examples">
diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c
index 9279c05..e634ccf 100644
--- a/src/backend/commands/functioncmds.c
+++ b/src/backend/commands/functioncmds.c
@@ -1399,49 +1399,6 @@ AlterFunction(ParseState *pstate, AlterFunctionStmt *stmt)
return address;
}
-/*
- * SetFunctionReturnType - change declared return type of a function
- *
- * This is presently only used for adjusting legacy functions that return
- * OPAQUE to return whatever we find their correct definition should be.
- * The caller should emit a suitable warning explaining what we did.
- */
-void
-SetFunctionReturnType(Oid funcOid, Oid newRetType)
-{
- Relation pg_proc_rel;
- HeapTuple tup;
- Form_pg_proc procForm;
- ObjectAddress func_address;
- ObjectAddress type_address;
-
- pg_proc_rel = table_open(ProcedureRelationId, RowExclusiveLock);
-
- tup = SearchSysCacheCopy1(PROCOID, ObjectIdGetDatum(funcOid));
- if (!HeapTupleIsValid(tup)) /* should not happen */
- elog(ERROR, "cache lookup failed for function %u", funcOid);
- procForm = (Form_pg_proc) GETSTRUCT(tup);
-
- if (procForm->prorettype != OPAQUEOID) /* caller messed up */
- elog(ERROR, "function %u doesn't return OPAQUE", funcOid);
-
- /* okay to overwrite copied tuple */
- procForm->prorettype = newRetType;
-
- /* update the catalog and its indexes */
- CatalogTupleUpdate(pg_proc_rel, &tup->t_self, tup);
-
- table_close(pg_proc_rel, RowExclusiveLock);
-
- /*
- * Also update the dependency to the new type. Opaque is a pinned type, so
- * there is no old dependency record for it that we would need to remove.
- */
- ObjectAddressSet(type_address, TypeRelationId, newRetType);
- ObjectAddressSet(func_address, ProcedureRelationId, funcOid);
- recordDependencyOn(&func_address, &type_address, DEPENDENCY_NORMAL);
-}
-
/*
* CREATE CAST
diff --git a/src/backend/commands/proclang.c b/src/backend/commands/proclang.c
index 9d72edb..21ceeb7 100644
--- a/src/backend/commands/proclang.c
+++ b/src/backend/commands/proclang.c
@@ -74,27 +74,10 @@ CreateProceduralLanguage(CreatePLangStmt *stmt)
handlerOid = LookupFuncName(stmt->plhandler, 0, NULL, false);
funcrettype = get_func_rettype(handlerOid);
if (funcrettype != LANGUAGE_HANDLEROID)
- {
- /*
- * We allow OPAQUE just so we can load old dump files. When we see a
- * handler function declared OPAQUE, change it to LANGUAGE_HANDLER.
- * (This is probably obsolete and removable?)
- */
- if (funcrettype == OPAQUEOID)
- {
- ereport(WARNING,
- (errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("changing return type of function %s from %s to %s",
- NameListToString(stmt->plhandler),
- "opaque", "language_handler")));
- SetFunctionReturnType(handlerOid, LANGUAGE_HANDLEROID);
- }
- else
- ereport(ERROR,
- (errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("function %s must return type %s",
- NameListToString(stmt->plhandler), "language_handler")));
- }
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("function %s must return type %s",
+ NameListToString(stmt->plhandler), "language_handler")));
/* validate the inline function */
if (stmt->plinline)
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index 6e8b722..056a912 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -699,25 +699,10 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString,
}
funcrettype = get_func_rettype(funcoid);
if (funcrettype != TRIGGEROID)
- {
- /*
- * We allow OPAQUE just so we can load old dump files. When we see a
- * trigger function declared OPAQUE, change it to TRIGGER.
- */
- if (funcrettype == OPAQUEOID)
- {
- ereport(WARNING,
- (errmsg("changing return type of function %s from %s to %s",
- NameListToString(stmt->funcname),
- "opaque", "trigger")));
- SetFunctionReturnType(funcoid, TRIGGEROID);
- }
- else
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
- errmsg("function %s must return type %s",
- NameListToString(stmt->funcname), "trigger")));
- }
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("function %s must return type %s",
+ NameListToString(stmt->funcname), "trigger")));
/*
* If the command is a user-entered CREATE CONSTRAINT TRIGGER command that
diff --git a/src/backend/utils/adt/pseudotypes.c b/src/backend/utils/adt/pseudotypes.c
index ad0e363..4653fc3 100644
--- a/src/backend/utils/adt/pseudotypes.c
+++ b/src/backend/utils/adt/pseudotypes.c
@@ -415,7 +415,6 @@ PSEUDOTYPE_DUMMY_IO_FUNCS(fdw_handler);
PSEUDOTYPE_DUMMY_IO_FUNCS(index_am_handler);
PSEUDOTYPE_DUMMY_IO_FUNCS(tsm_handler);
PSEUDOTYPE_DUMMY_IO_FUNCS(internal);
-PSEUDOTYPE_DUMMY_IO_FUNCS(opaque);
PSEUDOTYPE_DUMMY_IO_FUNCS(anyelement);
PSEUDOTYPE_DUMMY_IO_FUNCS(anynonarray);
PSEUDOTYPE_DUMMY_IO_FUNCS(table_am_handler);
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index ef15390..d3ab5bb 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -82,10 +82,9 @@ typedef struct
typedef enum OidOptions
{
- zeroAsOpaque = 1,
- zeroAsAny = 2,
- zeroAsStar = 4,
- zeroAsNone = 8
+ zeroIsError = 1,
+ zeroAsStar = 2,
+ zeroAsNone = 4
} OidOptions;
/* global decls */
@@ -122,8 +121,6 @@ static SimpleStringList tabledata_exclude_patterns = {NULL, NULL};
static SimpleOidList tabledata_exclude_oids = {NULL, NULL};
-char g_opaque_type[10]; /* name for the opaque type */
-
/* placeholders for the delimiters for comments */
char g_comment_start[10];
char g_comment_end[10];
@@ -404,7 +401,6 @@ main(int argc, char **argv)
strcpy(g_comment_start, "-- ");
g_comment_end[0] = '\0';
- strcpy(g_opaque_type, "opaque");
progname = get_progname(argv[0]);
@@ -10736,7 +10732,7 @@ dumpBaseType(Archive *fout, TypeInfo *tyinfo)
{
char *elemType;
- elemType = getFormattedTypeName(fout, tyinfo->typelem, zeroAsOpaque);
+ elemType = getFormattedTypeName(fout, tyinfo->typelem, zeroIsError);
appendPQExpBuffer(q, ",\n ELEMENT = %s", elemType);
free(elemType);
}
@@ -11547,7 +11543,7 @@ format_function_arguments_old(Archive *fout,
const char *argname;
typid = allargtypes ? atooid(allargtypes[j]) : finfo->argtypes[j];
- typname = getFormattedTypeName(fout, typid, zeroAsOpaque);
+ typname = getFormattedTypeName(fout, typid, zeroIsError);
if (argmodes)
{
@@ -11616,7 +11612,7 @@ format_function_signature(Archive *fout, FuncInfo *finfo, bool honor_quotes)
appendPQExpBufferStr(&fn, ", ");
typname = getFormattedTypeName(fout, finfo->argtypes[j],
- zeroAsOpaque);
+ zeroIsError);
appendPQExpBufferStr(&fn, typname);
free(typname);
}
@@ -12021,7 +12017,7 @@ dumpFunc(Archive *fout, FuncInfo *finfo)
else
{
rettypename = getFormattedTypeName(fout, finfo->prorettype,
- zeroAsOpaque);
+ zeroIsError);
appendPQExpBuffer(q, " RETURNS %s%s",
(proretset[0] == 't') ? "SETOF " : "",
rettypename);
@@ -13740,7 +13736,7 @@ format_aggregate_signature(AggInfo *agginfo, Archive *fout, bool honor_quotes)
char *typname;
typname = getFormattedTypeName(fout, agginfo->aggfn.argtypes[j],
- zeroAsOpaque);
+ zeroIsError);
appendPQExpBuffer(&buf, "%s%s",
(j > 0) ? ", " : "",
@@ -18363,11 +18359,7 @@ getFormattedTypeName(Archive *fout, Oid oid, OidOptions opts)
if (oid == 0)
{
- if ((opts & zeroAsOpaque) != 0)
- return pg_strdup(g_opaque_type);
- else if ((opts & zeroAsAny) != 0)
- return pg_strdup("'any'");
- else if ((opts & zeroAsStar) != 0)
+ if ((opts & zeroAsStar) != 0)
return pg_strdup("*");
else if ((opts & zeroAsNone) != 0)
return pg_strdup("NONE");
diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index 21004e5..852020b 100644
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -640,8 +640,6 @@ typedef struct _extensionMemberId
extern char g_comment_start[10];
extern char g_comment_end[10];
-extern char g_opaque_type[10]; /* name for the opaque type */
-
/*
* common utility functions
*/
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 07a86c7..7fb574f 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -7054,12 +7054,6 @@
{ oid => '2305', descr => 'I/O',
proname => 'internal_out', prorettype => 'cstring', proargtypes => 'internal',
prosrc => 'internal_out' },
-{ oid => '2306', descr => 'I/O',
- proname => 'opaque_in', proisstrict => 'f', prorettype => 'opaque',
- proargtypes => 'cstring', prosrc => 'opaque_in' },
-{ oid => '2307', descr => 'I/O',
- proname => 'opaque_out', prorettype => 'cstring', proargtypes => 'opaque',
- prosrc => 'opaque_out' },
{ oid => '2312', descr => 'I/O',
proname => 'anyelement_in', prorettype => 'anyelement',
proargtypes => 'cstring', prosrc => 'anyelement_in' },
@@ -7067,10 +7061,10 @@
proname => 'anyelement_out', prorettype => 'cstring',
proargtypes => 'anyelement', prosrc => 'anyelement_out' },
{ oid => '2398', descr => 'I/O',
- proname => 'shell_in', proisstrict => 'f', prorettype => 'opaque',
+ proname => 'shell_in', proisstrict => 'f', prorettype => 'void',
proargtypes => 'cstring', prosrc => 'shell_in' },
{ oid => '2399', descr => 'I/O',
- proname => 'shell_out', prorettype => 'cstring', proargtypes => 'opaque',
+ proname => 'shell_out', prorettype => 'cstring', proargtypes => 'void',
prosrc => 'shell_out' },
{ oid => '2597', descr => 'I/O',
proname => 'domain_in', proisstrict => 'f', provolatile => 's',
diff --git a/src/include/catalog/pg_type.dat b/src/include/catalog/pg_type.dat
index 4cf2b9d..b00597d 100644
--- a/src/include/catalog/pg_type.dat
+++ b/src/include/catalog/pg_type.dat
@@ -546,10 +546,6 @@
typtype => 'p', typcategory => 'P', typinput => 'internal_in',
typoutput => 'internal_out', typreceive => '-', typsend => '-',
typalign => 'ALIGNOF_POINTER' },
-{ oid => '2282', descr => 'obsolete, deprecated pseudo-type',
- typname => 'opaque', typlen => '4', typbyval => 't', typtype => 'p',
- typcategory => 'P', typinput => 'opaque_in', typoutput => 'opaque_out',
- typreceive => '-', typsend => '-', typalign => 'i' },
{ oid => '2283', descr => 'pseudo-type representing a polymorphic base type',
typname => 'anyelement', typlen => '4', typbyval => 't', typtype => 'p',
typcategory => 'P', typinput => 'anyelement_in',
diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h
index 0992c23..5cd6975 100644
--- a/src/include/commands/defrem.h
+++ b/src/include/commands/defrem.h
@@ -54,7 +54,6 @@ extern Oid ResolveOpClass(List *opclass, Oid attrType,
/* commands/functioncmds.c */
extern ObjectAddress CreateFunction(ParseState *pstate, CreateFunctionStmt *stmt);
extern void RemoveFunctionById(Oid funcOid);
-extern void SetFunctionReturnType(Oid funcOid, Oid newRetType);
extern ObjectAddress AlterFunction(ParseState *pstate, AlterFunctionStmt *stmt);
extern ObjectAddress CreateCast(CreateCastStmt *stmt);
extern void DropCastById(Oid castOid);
diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c
index a65bce0..5fdf303 100644
--- a/src/pl/plperl/plperl.c
+++ b/src/pl/plperl/plperl.c
@@ -2000,9 +2000,7 @@ plperl_validator(PG_FUNCTION_ARGS)
/* except for TRIGGER, EVTTRIGGER, RECORD, or VOID */
if (functyptype == TYPTYPE_PSEUDO)
{
- /* we assume OPAQUE with no arguments means a trigger */
- if (proc->prorettype == TRIGGEROID ||
- (proc->prorettype == OPAQUEOID && proc->pronargs == 0))
+ if (proc->prorettype == TRIGGEROID)
is_trigger = true;
else if (proc->prorettype == EVTTRIGGEROID)
is_event_trigger = true;
diff --git a/src/pl/plpgsql/src/pl_handler.c b/src/pl/plpgsql/src/pl_handler.c
index b83087e..b434818 100644
--- a/src/pl/plpgsql/src/pl_handler.c
+++ b/src/pl/plpgsql/src/pl_handler.c
@@ -421,12 +421,10 @@ plpgsql_validator(PG_FUNCTION_ARGS)
functyptype = get_typtype(proc->prorettype);
/* Disallow pseudotype result */
- /* except for TRIGGER, RECORD, VOID, or polymorphic */
+ /* except for TRIGGER, EVTTRIGGER, RECORD, VOID, or polymorphic */
if (functyptype == TYPTYPE_PSEUDO)
{
- /* we assume OPAQUE with no arguments means a trigger */
- if (proc->prorettype == TRIGGEROID ||
- (proc->prorettype == OPAQUEOID && proc->pronargs == 0))
+ if (proc->prorettype == TRIGGEROID)
is_dml_trigger = true;
else if (proc->prorettype == EVTTRIGGEROID)
is_event_trigger = true;
diff --git a/src/pl/plpython/plpy_main.c b/src/pl/plpython/plpy_main.c
index 882d69e..3eedaa8 100644
--- a/src/pl/plpython/plpy_main.c
+++ b/src/pl/plpython/plpy_main.c
@@ -379,9 +379,7 @@ plpython2_inline_handler(PG_FUNCTION_ARGS)
static bool
PLy_procedure_is_trigger(Form_pg_proc procStruct)
{
- return (procStruct->prorettype == TRIGGEROID ||
- (procStruct->prorettype == OPAQUEOID &&
- procStruct->pronargs == 0));
+ return (procStruct->prorettype == TRIGGEROID);
}
static void
diff --git a/src/test/regress/expected/opr_sanity.out b/src/test/regress/expected/opr_sanity.out
index fb6c029..40468e8 100644
--- a/src/test/regress/expected/opr_sanity.out
+++ b/src/test/regress/expected/opr_sanity.out
@@ -384,7 +384,7 @@ FROM pg_proc as p1
WHERE p1.prorettype = 'cstring'::regtype
AND NOT EXISTS(SELECT 1 FROM pg_type WHERE typoutput = p1.oid)
AND NOT EXISTS(SELECT 1 FROM pg_type WHERE typmodout = p1.oid)
- AND p1.oid != 'shell_out(opaque)'::regprocedure
+ AND p1.oid != 'shell_out(void)'::regprocedure
ORDER BY 1;
oid | proname
------+--------------
diff --git a/src/test/regress/sql/opr_sanity.sql b/src/test/regress/sql/opr_sanity.sql
index 8351b64..f06f245 100644
--- a/src/test/regress/sql/opr_sanity.sql
+++ b/src/test/regress/sql/opr_sanity.sql
@@ -309,7 +309,7 @@ FROM pg_proc as p1
WHERE p1.prorettype = 'cstring'::regtype
AND NOT EXISTS(SELECT 1 FROM pg_type WHERE typoutput = p1.oid)
AND NOT EXISTS(SELECT 1 FROM pg_type WHERE typmodout = p1.oid)
- AND p1.oid != 'shell_out(opaque)'::regprocedure
+ AND p1.oid != 'shell_out(void)'::regprocedure
ORDER BY 1;
-- Check for length inconsistencies between the various argument-info arrays.
diff --git a/doc/src/sgml/ref/alter_type.sgml b/doc/src/sgml/ref/alter_type.sgml
index 67be1dd..2543b20 100644
--- a/doc/src/sgml/ref/alter_type.sgml
+++ b/doc/src/sgml/ref/alter_type.sgml
@@ -30,6 +30,7 @@ ALTER TYPE <replaceable class="parameter">name</replaceable> RENAME TO <replacea
ALTER TYPE <replaceable class="parameter">name</replaceable> SET SCHEMA <replaceable
class="parameter">new_schema</replaceable>
ALTER TYPE <replaceable class="parameter">name</replaceable> ADD VALUE [ IF NOT EXISTS ] <replaceable
class="parameter">new_enum_value</replaceable>[ { BEFORE | AFTER } <replaceable
class="parameter">neighbor_enum_value</replaceable>]
ALTER TYPE <replaceable class="parameter">name</replaceable> RENAME VALUE <replaceable
class="parameter">existing_enum_value</replaceable>TO <replaceable class="parameter">new_enum_value</replaceable>
+ALTER TYPE <replaceable class="parameter">name</replaceable> SET ( <replaceable
class="parameter">property</replaceable>= <replaceable class="parameter">value</replaceable> [, ... ] )
<phrase>where <replaceable class="parameter">action</replaceable> is one of:</phrase>
@@ -70,7 +71,7 @@ ALTER TYPE <replaceable class="parameter">name</replaceable> RENAME VALUE <repla
</varlistentry>
<varlistentry>
- <term><literal>SET DATA TYPE</literal></term>
+ <term><literal>ALTER ATTRIBUTE ... SET DATA TYPE</literal></term>
<listitem>
<para>
This form changes the type of an attribute of a composite type.
@@ -135,6 +136,82 @@ ALTER TYPE <replaceable class="parameter">name</replaceable> RENAME VALUE <repla
</para>
</listitem>
</varlistentry>
+
+ <varlistentry>
+ <term>
+ <literal>SET ( <replaceable class="parameter">property</replaceable> = <replaceable
class="parameter">value</replaceable>[, ... ] )</literal>
+ </term>
+ <listitem>
+ <para>
+ This form is only applicable to base types. It allows adjustment of a
+ subset of the base-type properties that can be set in <command>CREATE
+ TYPE</command>. Specifically, these properties can be changed:
+ <itemizedlist>
+ <listitem>
+ <para>
+ <literal>RECEIVE</literal> can be set to the name of a binary input
+ function, or <literal>NONE</literal> to remove the type's binary
+ input function. Using this option requires superuser privilege.
+ </para>
+ </listitem>
+ <listitem>
+ <para>
+ <literal>SEND</literal> can be set to the name of a binary output
+ function, or <literal>NONE</literal> to remove the type's binary
+ output function. Using this option requires superuser privilege.
+ </para>
+ </listitem>
+ <listitem>
+ <para>
+ <literal>TYPMOD_IN</literal> can be set to the name of a type
+ modifier input function, or <literal>NONE</literal> to remove the
+ type's type modifier input function. Using this option requires
+ superuser privilege.
+ </para>
+ </listitem>
+ <listitem>
+ <para>
+ <literal>TYPMOD_OUT</literal> can be set to the name of a type
+ modifier output function, or <literal>NONE</literal> to remove the
+ type's type modifier output function. Using this option requires
+ superuser privilege.
+ </para>
+ </listitem>
+ <listitem>
+ <para>
+ <literal>ANALYZE</literal> can be set to the name of a type-specific
+ statistics collection function, or <literal>NONE</literal> to remove
+ the type's statistics collection function. Using this option
+ requires superuser privilege.
+ </para>
+ </listitem>
+ <listitem>
+ <para>
+ <literal>STORAGE</literal><indexterm>
+ <primary>TOAST</primary>
+ <secondary>per-type storage settings</secondary>
+ </indexterm>
+ can be set to <literal>plain</literal>,
+ <literal>extended</literal>, <literal>external</literal>,
+ or <literal>main</literal> (see <xref linkend="storage-toast"/> for
+ more information about what these mean). However, changing
+ from <literal>plain</literal> to another setting requires superuser
+ privilege (because it requires that the type's C functions all be
+ TOAST-ready), and changing to <literal>plain</literal> from another
+ setting is not allowed at all (since the type may already have
+ TOASTed values present in the database). Note that changing this
+ option doesn't by itself change any stored data, it just sets the
+ default TOAST strategy to be used for table columns created in the
+ future. See <xref linkend="sql-altertable"/> to change the TOAST
+ strategy for existing table columns.
+ </para>
+ </listitem>
+ </itemizedlist>
+ See <xref linkend="sql-createtype"/> for more details about these
+ type properties.
+ </para>
+ </listitem>
+ </varlistentry>
</variablelist>
</para>
@@ -156,7 +233,7 @@ ALTER TYPE <replaceable class="parameter">name</replaceable> RENAME VALUE <repla
doesn't do anything you couldn't do by dropping and recreating the type.
However, a superuser can alter ownership of any type anyway.)
To add an attribute or alter an attribute type, you must also
- have <literal>USAGE</literal> privilege on the data type.
+ have <literal>USAGE</literal> privilege on the attribute's data type.
</para>
</refsect1>
@@ -353,7 +430,20 @@ ALTER TYPE colors ADD VALUE 'orange' AFTER 'red';
To rename an enum value:
<programlisting>
ALTER TYPE colors RENAME VALUE 'purple' TO 'mauve';
-</programlisting></para>
+</programlisting>
+ </para>
+
+ <para>
+ To create binary I/O functions for an existing base type:
+<programlisting>
+CREATE FUNCTION mytypesend(mytype) RETURNS bytea ...;
+CREATE FUNCTION mytyperecv(internal, oid, integer) RETURNS mytype ...;
+ALTER TYPE mytype SET (
+ send = mytypesend,
+ receive = mytyperecv
+);
+</programlisting>
+ </para>
</refsect1>
<refsect1>
diff --git a/src/backend/catalog/pg_type.c b/src/backend/catalog/pg_type.c
index 56e0bcf..cd56714 100644
--- a/src/backend/catalog/pg_type.c
+++ b/src/backend/catalog/pg_type.c
@@ -155,8 +155,8 @@ TypeShellMake(const char *typeName, Oid typeNamespace, Oid ownerId)
* Create dependencies. We can/must skip this in bootstrap mode.
*/
if (!IsBootstrapProcessingMode())
- GenerateTypeDependencies(typoid,
- (Form_pg_type) GETSTRUCT(tup),
+ GenerateTypeDependencies(tup,
+ pg_type_desc,
NULL,
NULL,
0,
@@ -488,8 +488,8 @@ TypeCreate(Oid newTypeOid,
* Create dependencies. We can/must skip this in bootstrap mode.
*/
if (!IsBootstrapProcessingMode())
- GenerateTypeDependencies(typeObjectId,
- (Form_pg_type) GETSTRUCT(tup),
+ GenerateTypeDependencies(tup,
+ pg_type_desc,
(defaultTypeBin ?
stringToNode(defaultTypeBin) :
NULL),
@@ -516,12 +516,16 @@ TypeCreate(Oid newTypeOid,
* GenerateTypeDependencies: build the dependencies needed for a type
*
* Most of what this function needs to know about the type is passed as the
- * new pg_type row, typeForm. But we can't get at the varlena fields through
- * that, so defaultExpr and typacl are passed separately. (typacl is really
+ * new pg_type row, typeTuple. We make callers pass the pg_type Relation
+ * as well, so that we have easy access to a tuple descriptor for the row.
+ *
+ * While this is able to extract the defaultExpr and typacl from the tuple,
+ * doing so is relatively expensive, and callers may have those values at
+ * hand already. Pass those if handy, otherwise pass NULL. (typacl is really
* "Acl *", but we declare it "void *" to avoid including acl.h in pg_type.h.)
*
- * relationKind and isImplicitArray aren't visible in the pg_type row either,
- * so they're also passed separately.
+ * relationKind and isImplicitArray are likewise somewhat expensive to deduce
+ * from the tuple, so we make callers pass those (they're not optional).
*
* isDependentType is true if this is an implicit array or relation rowtype;
* that means it doesn't need its own dependencies on owner etc.
@@ -535,8 +539,8 @@ TypeCreate(Oid newTypeOid,
* that type will become a member of the extension.)
*/
void
-GenerateTypeDependencies(Oid typeObjectId,
- Form_pg_type typeForm,
+GenerateTypeDependencies(HeapTuple typeTuple,
+ Relation typeCatalog,
Node *defaultExpr,
void *typacl,
char relationKind, /* only for relation rowtypes */
@@ -544,9 +548,30 @@ GenerateTypeDependencies(Oid typeObjectId,
bool isDependentType,
bool rebuild)
{
+ Form_pg_type typeForm = (Form_pg_type) GETSTRUCT(typeTuple);
+ Oid typeObjectId = typeForm->oid;
+ Datum datum;
+ bool isNull;
ObjectAddress myself,
referenced;
+ /* Extract defaultExpr if caller didn't pass it */
+ if (defaultExpr == NULL)
+ {
+ datum = heap_getattr(typeTuple, Anum_pg_type_typdefaultbin,
+ RelationGetDescr(typeCatalog), &isNull);
+ if (!isNull)
+ defaultExpr = stringToNode(TextDatumGetCString(datum));
+ }
+ /* Extract typacl if caller didn't pass it */
+ if (typacl == NULL)
+ {
+ datum = heap_getattr(typeTuple, Anum_pg_type_typacl,
+ RelationGetDescr(typeCatalog), &isNull);
+ if (!isNull)
+ typacl = DatumGetAclPCopy(datum);
+ }
+
/* If rebuild, first flush old dependencies, except extension deps */
if (rebuild)
{
diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c
index d732a3a..8348d79 100644
--- a/src/backend/commands/typecmds.c
+++ b/src/backend/commands/typecmds.c
@@ -83,6 +83,25 @@ typedef struct
/* atts[] is of allocated length RelationGetNumberOfAttributes(rel) */
} RelToCheck;
+/* parameter structure for AlterTypeRecurse() */
+typedef struct
+{
+ /* Flags indicating which type attributes to update */
+ bool updateStorage;
+ bool updateReceive;
+ bool updateSend;
+ bool updateTypmodin;
+ bool updateTypmodout;
+ bool updateAnalyze;
+ /* New values for relevant attributes */
+ char storage;
+ Oid receiveOid;
+ Oid sendOid;
+ Oid typmodinOid;
+ Oid typmodoutOid;
+ Oid analyzeOid;
+} AlterTypeRecurseParams;
+
/* Potentially set by pg_upgrade_support functions */
Oid binary_upgrade_next_array_pg_type_oid = InvalidOid;
@@ -107,6 +126,8 @@ static char *domainAddConstraint(Oid domainOid, Oid domainNamespace,
const char *domainName, ObjectAddress *constrAddr);
static Node *replace_domain_constraint_value(ParseState *pstate,
ColumnRef *cref);
+static void AlterTypeRecurse(Oid typeOid, HeapTuple tup, Relation catalog,
+ AlterTypeRecurseParams *atparams);
/*
@@ -466,9 +487,12 @@ DefineType(ParseState *pstate, List *names, List *parameters)
* minimum sane check would be for execute-with-grant-option. But we
* don't have a way to make the type go away if the grant option is
* revoked, so ownership seems better.
+ *
+ * XXX For now, this is all unnecessary given the superuser check above.
+ * If we ever relax that, these calls likely should be moved into
+ * findTypeInputFunction et al, where they could be shared by AlterType.
*/
#ifdef NOT_USED
- /* XXX this is unnecessary given the superuser check above */
if (inputOid && !pg_proc_ownercheck(inputOid, GetUserId()))
aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_FUNCTION,
NameListToString(inputName));
@@ -493,47 +517,6 @@ DefineType(ParseState *pstate, List *names, List *parameters)
#endif
/*
- * Print warnings if any of the type's I/O functions are marked volatile.
- * There is a general assumption that I/O functions are stable or
- * immutable; this allows us for example to mark record_in/record_out
- * stable rather than volatile. Ideally we would throw errors not just
- * warnings here; but since this check is new as of 9.5, and since the
- * volatility marking might be just an error-of-omission and not a true
- * indication of how the function behaves, we'll let it pass as a warning
- * for now.
- */
- if (inputOid && func_volatile(inputOid) == PROVOLATILE_VOLATILE)
- ereport(WARNING,
- (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
- errmsg("type input function %s should not be volatile",
- NameListToString(inputName))));
- if (outputOid && func_volatile(outputOid) == PROVOLATILE_VOLATILE)
- ereport(WARNING,
- (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
- errmsg("type output function %s should not be volatile",
- NameListToString(outputName))));
- if (receiveOid && func_volatile(receiveOid) == PROVOLATILE_VOLATILE)
- ereport(WARNING,
- (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
- errmsg("type receive function %s should not be volatile",
- NameListToString(receiveName))));
- if (sendOid && func_volatile(sendOid) == PROVOLATILE_VOLATILE)
- ereport(WARNING,
- (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
- errmsg("type send function %s should not be volatile",
- NameListToString(sendName))));
- if (typmodinOid && func_volatile(typmodinOid) == PROVOLATILE_VOLATILE)
- ereport(WARNING,
- (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
- errmsg("type modifier input function %s should not be volatile",
- NameListToString(typmodinName))));
- if (typmodoutOid && func_volatile(typmodoutOid) == PROVOLATILE_VOLATILE)
- ereport(WARNING,
- (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
- errmsg("type modifier output function %s should not be volatile",
- NameListToString(typmodoutName))));
-
- /*
* OK, we're done checking, time to make the type. We must assign the
* array type OID ahead of calling TypeCreate, since the base type and
* array type each refer to the other.
@@ -765,6 +748,12 @@ DefineDomain(CreateDomainStmt *stmt)
aclcheck_error_type(aclresult, basetypeoid);
/*
+ * Collect the properties of the new domain. Some are inherited from the
+ * base type, some are not. If you change any of this inheritance
+ * behavior, be sure to update AlterTypeRecurse() to match!
+ */
+
+ /*
* Identify the collation if any
*/
baseColl = baseType->typcollation;
@@ -1664,6 +1653,22 @@ findTypeInputFunction(List *procname, Oid typeOid)
errmsg("type input function %s must return type %s",
NameListToString(procname), format_type_be(typeOid))));
+ /*
+ * Print warnings if any of the type's I/O functions are marked volatile.
+ * There is a general assumption that I/O functions are stable or
+ * immutable; this allows us for example to mark record_in/record_out
+ * stable rather than volatile. Ideally we would throw errors not just
+ * warnings here; but since this check is new as of 9.5, and since the
+ * volatility marking might be just an error-of-omission and not a true
+ * indication of how the function behaves, we'll let it pass as a warning
+ * for now.
+ */
+ if (func_volatile(procOid) == PROVOLATILE_VOLATILE)
+ ereport(WARNING,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("type input function %s should not be volatile",
+ NameListToString(procname))));
+
return procOid;
}
@@ -1692,6 +1697,13 @@ findTypeOutputFunction(List *procname, Oid typeOid)
errmsg("type output function %s must return type %s",
NameListToString(procname), "cstring")));
+ /* Just a warning for now, per comments in findTypeInputFunction */
+ if (func_volatile(procOid) == PROVOLATILE_VOLATILE)
+ ereport(WARNING,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("type output function %s should not be volatile",
+ NameListToString(procname))));
+
return procOid;
}
@@ -1728,6 +1740,13 @@ findTypeReceiveFunction(List *procname, Oid typeOid)
errmsg("type receive function %s must return type %s",
NameListToString(procname), format_type_be(typeOid))));
+ /* Just a warning for now, per comments in findTypeInputFunction */
+ if (func_volatile(procOid) == PROVOLATILE_VOLATILE)
+ ereport(WARNING,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("type receive function %s should not be volatile",
+ NameListToString(procname))));
+
return procOid;
}
@@ -1756,6 +1775,13 @@ findTypeSendFunction(List *procname, Oid typeOid)
errmsg("type send function %s must return type %s",
NameListToString(procname), "bytea")));
+ /* Just a warning for now, per comments in findTypeInputFunction */
+ if (func_volatile(procOid) == PROVOLATILE_VOLATILE)
+ ereport(WARNING,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("type send function %s should not be volatile",
+ NameListToString(procname))));
+
return procOid;
}
@@ -1783,6 +1809,13 @@ findTypeTypmodinFunction(List *procname)
errmsg("typmod_in function %s must return type %s",
NameListToString(procname), "integer")));
+ /* Just a warning for now, per comments in findTypeInputFunction */
+ if (func_volatile(procOid) == PROVOLATILE_VOLATILE)
+ ereport(WARNING,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("type modifier input function %s should not be volatile",
+ NameListToString(procname))));
+
return procOid;
}
@@ -1810,6 +1843,13 @@ findTypeTypmodoutFunction(List *procname)
errmsg("typmod_out function %s must return type %s",
NameListToString(procname), "cstring")));
+ /* Just a warning for now, per comments in findTypeInputFunction */
+ if (func_volatile(procOid) == PROVOLATILE_VOLATILE)
+ ereport(WARNING,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("type modifier output function %s should not be volatile",
+ NameListToString(procname))));
+
return procOid;
}
@@ -2086,9 +2126,6 @@ AlterDomainDefault(List *names, Node *defaultRaw)
Relation rel;
char *defaultValue;
Node *defaultExpr = NULL; /* NULL if no default specified */
- Acl *typacl;
- Datum aclDatum;
- bool isNull;
Datum new_record[Natts_pg_type];
bool new_record_nulls[Natts_pg_type];
bool new_record_repl[Natts_pg_type];
@@ -2141,6 +2178,7 @@ AlterDomainDefault(List *names, Node *defaultRaw)
(IsA(defaultExpr, Const) &&((Const *) defaultExpr)->constisnull))
{
/* Default is NULL, drop it */
+ defaultExpr = NULL;
new_record_nulls[Anum_pg_type_typdefaultbin - 1] = true;
new_record_repl[Anum_pg_type_typdefaultbin - 1] = true;
new_record_nulls[Anum_pg_type_typdefault - 1] = true;
@@ -2181,19 +2219,11 @@ AlterDomainDefault(List *names, Node *defaultRaw)
CatalogTupleUpdate(rel, &tup->t_self, newtuple);
- /* Must extract ACL for use of GenerateTypeDependencies */
- aclDatum = heap_getattr(newtuple, Anum_pg_type_typacl,
- RelationGetDescr(rel), &isNull);
- if (isNull)
- typacl = NULL;
- else
- typacl = DatumGetAclPCopy(aclDatum);
-
/* Rebuild dependencies */
- GenerateTypeDependencies(domainoid,
- (Form_pg_type) GETSTRUCT(newtuple),
+ GenerateTypeDependencies(newtuple,
+ rel,
defaultExpr,
- typacl,
+ NULL, /* don't have typacl handy */
0, /* relation kind is n/a */
false, /* a domain isn't an implicit array */
false, /* nor is it any kind of dependent type */
@@ -3609,3 +3639,338 @@ AlterTypeNamespaceInternal(Oid typeOid, Oid nspOid,
return oldNspOid;
}
+
+/*
+ * AlterType
+ * ALTER TYPE <type> SET (option = ...)
+ */
+ObjectAddress
+AlterType(AlterTypeStmt *stmt)
+{
+ ObjectAddress address;
+ Relation catalog;
+ TypeName *typename;
+ HeapTuple tup;
+ Oid typeOid;
+ Form_pg_type typForm;
+ bool requireSuper = false;
+ AlterTypeRecurseParams atparams;
+ ListCell *pl;
+
+ catalog = table_open(TypeRelationId, RowExclusiveLock);
+
+ /* Make a TypeName so we can use standard type lookup machinery */
+ typename = makeTypeNameFromNameList(stmt->typeName);
+ tup = typenameType(NULL, typename, NULL);
+
+ typeOid = typeTypeId(tup);
+ typForm = (Form_pg_type) GETSTRUCT(tup);
+
+ /* Process options */
+ memset(&atparams, 0, sizeof(atparams));
+ foreach(pl, stmt->options)
+ {
+ DefElem *defel = (DefElem *) lfirst(pl);
+
+ if (strcmp(defel->defname, "storage") == 0)
+ {
+ char *a = defGetString(defel);
+
+ if (pg_strcasecmp(a, "plain") == 0)
+ atparams.storage = TYPSTORAGE_PLAIN;
+ else if (pg_strcasecmp(a, "external") == 0)
+ atparams.storage = TYPSTORAGE_EXTERNAL;
+ else if (pg_strcasecmp(a, "extended") == 0)
+ atparams.storage = TYPSTORAGE_EXTENDED;
+ else if (pg_strcasecmp(a, "main") == 0)
+ atparams.storage = TYPSTORAGE_MAIN;
+ else
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("storage \"%s\" not recognized", a)));
+
+ /*
+ * Validate the storage request. If the type isn't varlena, it
+ * certainly doesn't support non-PLAIN storage.
+ */
+ if (atparams.storage != TYPSTORAGE_PLAIN && typForm->typlen != -1)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("fixed-size types must have storage PLAIN")));
+
+ /*
+ * Switching from PLAIN to non-PLAIN is allowed, but it requires
+ * superuser, since we can't validate that the type's C functions
+ * will support it. Switching from non-PLAIN to PLAIN is
+ * disallowed outright, because it's not practical to ensure that
+ * no tables have toasted values of the type. Switching among
+ * different non-PLAIN settings is OK, since it just constitutes a
+ * change in the strategy requested for columns created in the
+ * future.
+ */
+ if (atparams.storage != TYPSTORAGE_PLAIN &&
+ typForm->typstorage == TYPSTORAGE_PLAIN)
+ requireSuper = true;
+ else if (atparams.storage == TYPSTORAGE_PLAIN &&
+ typForm->typstorage != TYPSTORAGE_PLAIN)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("cannot change type's storage to PLAIN")));
+
+ atparams.updateStorage = true;
+ }
+ else if (strcmp(defel->defname, "receive") == 0)
+ {
+ if (defel->arg != NULL)
+ atparams.receiveOid =
+ findTypeReceiveFunction(defGetQualifiedName(defel),
+ typeOid);
+ else
+ atparams.receiveOid = InvalidOid; /* NONE, remove function */
+ atparams.updateReceive = true;
+ /* Replacing an I/O function requires superuser. */
+ requireSuper = true;
+ }
+ else if (strcmp(defel->defname, "send") == 0)
+ {
+ if (defel->arg != NULL)
+ atparams.sendOid =
+ findTypeSendFunction(defGetQualifiedName(defel),
+ typeOid);
+ else
+ atparams.sendOid = InvalidOid; /* NONE, remove function */
+ atparams.updateSend = true;
+ /* Replacing an I/O function requires superuser. */
+ requireSuper = true;
+ }
+ else if (strcmp(defel->defname, "typmod_in") == 0)
+ {
+ if (defel->arg != NULL)
+ atparams.typmodinOid =
+ findTypeTypmodinFunction(defGetQualifiedName(defel));
+ else
+ atparams.typmodinOid = InvalidOid; /* NONE, remove function */
+ atparams.updateTypmodin = true;
+ /* Replacing an I/O function requires superuser. */
+ requireSuper = true;
+ }
+ else if (strcmp(defel->defname, "typmod_out") == 0)
+ {
+ if (defel->arg != NULL)
+ atparams.typmodoutOid =
+ findTypeTypmodoutFunction(defGetQualifiedName(defel));
+ else
+ atparams.typmodoutOid = InvalidOid; /* NONE, remove function */
+ atparams.updateTypmodout = true;
+ /* Replacing an I/O function requires superuser. */
+ requireSuper = true;
+ }
+ else if (strcmp(defel->defname, "analyze") == 0)
+ {
+ if (defel->arg != NULL)
+ atparams.analyzeOid =
+ findTypeAnalyzeFunction(defGetQualifiedName(defel),
+ typeOid);
+ else
+ atparams.analyzeOid = InvalidOid; /* NONE, remove function */
+ atparams.updateAnalyze = true;
+ /* Replacing an analyze function requires superuser. */
+ requireSuper = true;
+ }
+
+ /*
+ * The rest of the options that CREATE accepts cannot be changed.
+ * Check for them so that we can give a meaningful error message.
+ */
+ else if (strcmp(defel->defname, "input") == 0 ||
+ strcmp(defel->defname, "output") == 0 ||
+ strcmp(defel->defname, "internallength") == 0 ||
+ strcmp(defel->defname, "passedbyvalue") == 0 ||
+ strcmp(defel->defname, "alignment") == 0 ||
+ strcmp(defel->defname, "like") == 0 ||
+ strcmp(defel->defname, "category") == 0 ||
+ strcmp(defel->defname, "preferred") == 0 ||
+ strcmp(defel->defname, "default") == 0 ||
+ strcmp(defel->defname, "element") == 0 ||
+ strcmp(defel->defname, "delimiter") == 0 ||
+ strcmp(defel->defname, "collatable") == 0)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("type attribute \"%s\" cannot be changed",
+ defel->defname)));
+ else
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("type attribute \"%s\" not recognized",
+ defel->defname)));
+ }
+
+ /*
+ * Permissions check. Require superuser if we decided the command
+ * requires that, else must own the type.
+ */
+ if (requireSuper)
+ {
+ if (!superuser())
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("must be superuser to alter a type")));
+ }
+ else
+ {
+ if (!pg_type_ownercheck(typeOid, GetUserId()))
+ aclcheck_error_type(ACLCHECK_NOT_OWNER, typeOid);
+ }
+
+ /*
+ * We disallow all forms of ALTER TYPE SET on types that aren't plain base
+ * types. It would for example be highly unsafe, not to mention
+ * pointless, to change the send/receive functions for a composite type.
+ * Moreover, pg_dump has no support for changing these properties on
+ * non-base types. We might weaken this someday, but not now.
+ *
+ * Note: if you weaken this enough to allow composite types, be sure to
+ * adjust the GenerateTypeDependencies call below.
+ */
+ if (typForm->typtype != TYPTYPE_BASE)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("%s is not a base type",
+ format_type_be(typeOid))));
+
+ /*
+ * For the same reasons, don't allow direct alteration of array types.
+ */
+ if (OidIsValid(typForm->typelem) &&
+ get_array_type(typForm->typelem) == typeOid)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("%s is not a base type",
+ format_type_be(typeOid))));
+
+ /* OK, recursively update this type and any domains over it */
+ AlterTypeRecurse(typeOid, tup, catalog, &atparams);
+
+ /* Clean up */
+ ReleaseSysCache(tup);
+
+ table_close(catalog, RowExclusiveLock);
+
+ ObjectAddressSet(address, TypeRelationId, typeOid);
+
+ return address;
+}
+
+/*
+ * AlterTypeRecurse: one recursion step for AlterType()
+ *
+ * Apply the changes specified by atparams to the type identified by
+ * "typeOid", whose existing pg_type tuple is "tup". Then search for any
+ * domains over this type, and recursively apply (most of) the same changes
+ * to those domains.
+ *
+ * We need this because the system generally assumes that a domain inherits
+ * many properties from its base type. See DefineDomain() above for details
+ * of what is inherited.
+ */
+static void
+AlterTypeRecurse(Oid typeOid, HeapTuple tup, Relation catalog,
+ AlterTypeRecurseParams *atparams)
+{
+ Datum values[Natts_pg_type];
+ bool nulls[Natts_pg_type];
+ bool replaces[Natts_pg_type];
+ HeapTuple newtup;
+ SysScanDesc scan;
+ ScanKeyData key[1];
+ HeapTuple domainTup;
+
+ /* Since this function recurses, it could be driven to stack overflow */
+ check_stack_depth();
+
+ /* Update the current type's tuple */
+ memset(values, 0, sizeof(values));
+ memset(nulls, 0, sizeof(nulls));
+ memset(replaces, 0, sizeof(replaces));
+
+ if (atparams->updateStorage)
+ {
+ replaces[Anum_pg_type_typstorage - 1] = true;
+ values[Anum_pg_type_typstorage - 1] = CharGetDatum(atparams->storage);
+ }
+ if (atparams->updateReceive)
+ {
+ replaces[Anum_pg_type_typreceive - 1] = true;
+ values[Anum_pg_type_typreceive - 1] = ObjectIdGetDatum(atparams->receiveOid);
+ }
+ if (atparams->updateSend)
+ {
+ replaces[Anum_pg_type_typsend - 1] = true;
+ values[Anum_pg_type_typsend - 1] = ObjectIdGetDatum(atparams->sendOid);
+ }
+ if (atparams->updateTypmodin)
+ {
+ replaces[Anum_pg_type_typmodin - 1] = true;
+ values[Anum_pg_type_typmodin - 1] = ObjectIdGetDatum(atparams->typmodinOid);
+ }
+ if (atparams->updateTypmodout)
+ {
+ replaces[Anum_pg_type_typmodout - 1] = true;
+ values[Anum_pg_type_typmodout - 1] = ObjectIdGetDatum(atparams->typmodoutOid);
+ }
+ if (atparams->updateAnalyze)
+ {
+ replaces[Anum_pg_type_typanalyze - 1] = true;
+ values[Anum_pg_type_typanalyze - 1] = ObjectIdGetDatum(atparams->analyzeOid);
+ }
+
+ newtup = heap_modify_tuple(tup, RelationGetDescr(catalog),
+ values, nulls, replaces);
+
+ CatalogTupleUpdate(catalog, &newtup->t_self, newtup);
+
+ /* Rebuild dependencies for this type */
+ GenerateTypeDependencies(newtup,
+ catalog,
+ NULL, /* don't have defaultExpr handy */
+ NULL, /* don't have typacl handy */
+ 0, /* we rejected composite types above */
+ false, /* and we rejected implicit arrays above */
+ false, /* so it can't be a dependent type */
+ true);
+
+ InvokeObjectPostAlterHook(TypeRelationId, typeOid, 0);
+
+ /*
+ * Now we need to recurse to domains. However, some properties are not
+ * inherited by domains, so clear the update flags for those.
+ */
+ atparams->updateReceive = false; /* domains use F_DOMAIN_RECV */
+ atparams->updateTypmodin = false; /* domains don't have typmods */
+ atparams->updateTypmodout = false;
+
+ /* Search pg_type for possible domains over this type */
+ ScanKeyInit(&key[0],
+ Anum_pg_type_typbasetype,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(typeOid));
+
+ scan = systable_beginscan(catalog, InvalidOid, false,
+ NULL, 1, key);
+
+ while ((domainTup = systable_getnext(scan)) != NULL)
+ {
+ Form_pg_type domainForm = (Form_pg_type) GETSTRUCT(domainTup);
+
+ /*
+ * Shouldn't have a nonzero typbasetype in a non-domain, but let's
+ * check
+ */
+ if (domainForm->typtype != TYPTYPE_DOMAIN)
+ continue;
+
+ AlterTypeRecurse(domainForm->oid, domainTup, catalog, atparams);
+ }
+
+ systable_endscan(scan);
+}
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index e04c33e..eaab97f 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -3636,6 +3636,17 @@ _copyAlterOperatorStmt(const AlterOperatorStmt *from)
return newnode;
}
+static AlterTypeStmt *
+_copyAlterTypeStmt(const AlterTypeStmt *from)
+{
+ AlterTypeStmt *newnode = makeNode(AlterTypeStmt);
+
+ COPY_NODE_FIELD(typeName);
+ COPY_NODE_FIELD(options);
+
+ return newnode;
+}
+
static RuleStmt *
_copyRuleStmt(const RuleStmt *from)
{
@@ -5263,6 +5274,9 @@ copyObjectImpl(const void *from)
case T_AlterOperatorStmt:
retval = _copyAlterOperatorStmt(from);
break;
+ case T_AlterTypeStmt:
+ retval = _copyAlterTypeStmt(from);
+ break;
case T_RuleStmt:
retval = _copyRuleStmt(from);
break;
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 5b1ba14..88b9129 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -1482,6 +1482,15 @@ _equalAlterOperatorStmt(const AlterOperatorStmt *a, const AlterOperatorStmt *b)
}
static bool
+_equalAlterTypeStmt(const AlterTypeStmt *a, const AlterTypeStmt *b)
+{
+ COMPARE_NODE_FIELD(typeName);
+ COMPARE_NODE_FIELD(options);
+
+ return true;
+}
+
+static bool
_equalRuleStmt(const RuleStmt *a, const RuleStmt *b)
{
COMPARE_NODE_FIELD(relation);
@@ -3359,6 +3368,9 @@ equal(const void *a, const void *b)
case T_AlterOperatorStmt:
retval = _equalAlterOperatorStmt(a, b);
break;
+ case T_AlterTypeStmt:
+ retval = _equalAlterTypeStmt(a, b);
+ break;
case T_RuleStmt:
retval = _equalRuleStmt(a, b);
break;
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 96e7fdb..7e384f9 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -249,7 +249,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
AlterDatabaseStmt AlterDatabaseSetStmt AlterDomainStmt AlterEnumStmt
AlterFdwStmt AlterForeignServerStmt AlterGroupStmt
AlterObjectDependsStmt AlterObjectSchemaStmt AlterOwnerStmt
- AlterOperatorStmt AlterSeqStmt AlterSystemStmt AlterTableStmt
+ AlterOperatorStmt AlterTypeStmt AlterSeqStmt AlterSystemStmt AlterTableStmt
AlterTblSpcStmt AlterExtensionStmt AlterExtensionContentsStmt AlterForeignTableStmt
AlterCompositeTypeStmt AlterUserMappingStmt
AlterRoleStmt AlterRoleSetStmt AlterPolicyStmt AlterStatsStmt
@@ -847,6 +847,7 @@ stmt :
| AlterObjectSchemaStmt
| AlterOwnerStmt
| AlterOperatorStmt
+ | AlterTypeStmt
| AlterPolicyStmt
| AlterSeqStmt
| AlterSystemStmt
@@ -9367,6 +9368,24 @@ operator_def_arg:
/*****************************************************************************
*
+ * ALTER TYPE name SET define
+ *
+ * We repurpose ALTER OPERATOR's version of "definition" here
+ *
+ *****************************************************************************/
+
+AlterTypeStmt:
+ ALTER TYPE_P any_name SET '(' operator_def_list ')'
+ {
+ AlterTypeStmt *n = makeNode(AlterTypeStmt);
+ n->typeName = $3;
+ n->options = $6;
+ $$ = (Node *)n;
+ }
+ ;
+
+/*****************************************************************************
+ *
* ALTER THING name OWNER TO newname
*
*****************************************************************************/
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 1b460a2..b1f7f6e 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -162,6 +162,7 @@ ClassifyUtilityCommandAsReadOnly(Node *parsetree)
case T_AlterTableMoveAllStmt:
case T_AlterTableSpaceOptionsStmt:
case T_AlterTableStmt:
+ case T_AlterTypeStmt:
case T_AlterUserMappingStmt:
case T_CommentStmt:
case T_CompositeTypeStmt:
@@ -1713,6 +1714,10 @@ ProcessUtilitySlow(ParseState *pstate,
address = AlterOperator((AlterOperatorStmt *) parsetree);
break;
+ case T_AlterTypeStmt:
+ address = AlterType((AlterTypeStmt *) parsetree);
+ break;
+
case T_CommentStmt:
address = CommentObject((CommentStmt *) parsetree);
break;
@@ -2895,6 +2900,10 @@ CreateCommandTag(Node *parsetree)
tag = CMDTAG_ALTER_OPERATOR;
break;
+ case T_AlterTypeStmt:
+ tag = CMDTAG_ALTER_TYPE;
+ break;
+
case T_AlterTSDictionaryStmt:
tag = CMDTAG_ALTER_TEXT_SEARCH_DICTIONARY;
break;
@@ -3251,6 +3260,10 @@ GetCommandLogLevel(Node *parsetree)
lev = LOGSTMT_DDL;
break;
+ case T_AlterTypeStmt:
+ lev = LOGSTMT_DDL;
+ break;
+
case T_AlterTableMoveAllStmt:
case T_AlterTableStmt:
lev = LOGSTMT_DDL;
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index b6b08d0..54d0317 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -2150,7 +2150,7 @@ psql_completion(const char *text, int start, int end)
else if (Matches("ALTER", "TYPE", MatchAny))
COMPLETE_WITH("ADD ATTRIBUTE", "ADD VALUE", "ALTER ATTRIBUTE",
"DROP ATTRIBUTE",
- "OWNER TO", "RENAME", "SET SCHEMA");
+ "OWNER TO", "RENAME", "SET SCHEMA", "SET (");
/* complete ALTER TYPE <foo> ADD with actions */
else if (Matches("ALTER", "TYPE", MatchAny, "ADD"))
COMPLETE_WITH("ATTRIBUTE", "VALUE");
diff --git a/src/include/catalog/pg_type.h b/src/include/catalog/pg_type.h
index f972f94..9789094 100644
--- a/src/include/catalog/pg_type.h
+++ b/src/include/catalog/pg_type.h
@@ -340,8 +340,8 @@ extern ObjectAddress TypeCreate(Oid newTypeOid,
bool typeNotNull,
Oid typeCollation);
-extern void GenerateTypeDependencies(Oid typeObjectId,
- Form_pg_type typeForm,
+extern void GenerateTypeDependencies(HeapTuple typeTuple,
+ Relation typeCatalog,
Node *defaultExpr,
void *typacl,
char relationKind, /* only for relation
diff --git a/src/include/commands/typecmds.h b/src/include/commands/typecmds.h
index fc18d64..0162bc2 100644
--- a/src/include/commands/typecmds.h
+++ b/src/include/commands/typecmds.h
@@ -54,4 +54,6 @@ extern Oid AlterTypeNamespaceInternal(Oid typeOid, Oid nspOid,
bool errorOnTableType,
ObjectAddresses *objsMoved);
+extern ObjectAddress AlterType(AlterTypeStmt *stmt);
+
#endif /* TYPECMDS_H */
diff --git a/src/include/nodes/nodes.h b/src/include/nodes/nodes.h
index baced7e..8a76afe 100644
--- a/src/include/nodes/nodes.h
+++ b/src/include/nodes/nodes.h
@@ -380,6 +380,7 @@ typedef enum NodeTag
T_AlterObjectSchemaStmt,
T_AlterOwnerStmt,
T_AlterOperatorStmt,
+ T_AlterTypeStmt,
T_DropOwnedStmt,
T_ReassignOwnedStmt,
T_CompositeTypeStmt,
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index da0706a..2039b42 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -2959,9 +2959,8 @@ typedef struct AlterOwnerStmt
RoleSpec *newowner; /* the new owner */
} AlterOwnerStmt;
-
/* ----------------------
- * Alter Operator Set Restrict, Join
+ * Alter Operator Set ( this-n-that )
* ----------------------
*/
typedef struct AlterOperatorStmt
@@ -2971,6 +2970,16 @@ typedef struct AlterOperatorStmt
List *options; /* List of DefElem nodes */
} AlterOperatorStmt;
+/* ------------------------
+ * Alter Type Set ( this-n-that )
+ * ------------------------
+ */
+typedef struct AlterTypeStmt
+{
+ NodeTag type;
+ List *typeName; /* type name (possibly qualified) */
+ List *options; /* List of DefElem nodes */
+} AlterTypeStmt;
/* ----------------------
* Create Rule Statement
diff --git a/src/test/regress/expected/create_type.out b/src/test/regress/expected/create_type.out
index eb55e25..86a8b65 100644
--- a/src/test/regress/expected/create_type.out
+++ b/src/test/regress/expected/create_type.out
@@ -224,3 +224,81 @@ select format_type('bpchar'::regtype, -1);
bpchar
(1 row)
+--
+-- Test CREATE/ALTER TYPE using a type that's compatible with varchar,
+-- so we can re-use those support functions
+--
+CREATE TYPE myvarchar;
+CREATE FUNCTION myvarcharin(cstring, oid, integer) RETURNS myvarchar
+LANGUAGE internal IMMUTABLE PARALLEL SAFE STRICT AS 'varcharin';
+NOTICE: return type myvarchar is only a shell
+CREATE FUNCTION myvarcharout(myvarchar) RETURNS cstring
+LANGUAGE internal IMMUTABLE PARALLEL SAFE STRICT AS 'varcharout';
+NOTICE: argument type myvarchar is only a shell
+CREATE FUNCTION myvarcharsend(myvarchar) RETURNS bytea
+LANGUAGE internal STABLE PARALLEL SAFE STRICT AS 'varcharsend';
+NOTICE: argument type myvarchar is only a shell
+CREATE FUNCTION myvarcharrecv(internal, oid, integer) RETURNS myvarchar
+LANGUAGE internal STABLE PARALLEL SAFE STRICT AS 'varcharrecv';
+NOTICE: return type myvarchar is only a shell
+-- fail, it's still a shell:
+ALTER TYPE myvarchar SET (storage = extended);
+ERROR: type "myvarchar" is only a shell
+CREATE TYPE myvarchar (
+ input = myvarcharin,
+ output = myvarcharout,
+ alignment = integer,
+ storage = main
+);
+-- want to check updating of a domain over the target type, too
+CREATE DOMAIN myvarchardom AS myvarchar;
+ALTER TYPE myvarchar SET (storage = plain); -- not allowed
+ERROR: cannot change type's storage to PLAIN
+ALTER TYPE myvarchar SET (storage = extended);
+ALTER TYPE myvarchar SET (
+ send = myvarcharsend,
+ receive = myvarcharrecv,
+ typmod_in = varchartypmodin,
+ typmod_out = varchartypmodout,
+ analyze = array_typanalyze -- bogus, but it doesn't matter
+);
+SELECT typinput, typoutput, typreceive, typsend, typmodin, typmodout,
+ typanalyze, typstorage
+FROM pg_type WHERE typname = 'myvarchar';
+ typinput | typoutput | typreceive | typsend | typmodin | typmodout | typanalyze |
typstorage
+-------------+--------------+---------------+---------------+-----------------+------------------+------------------+------------
+ myvarcharin | myvarcharout | myvarcharrecv | myvarcharsend | varchartypmodin | varchartypmodout | array_typanalyze |
x
+(1 row)
+
+SELECT typinput, typoutput, typreceive, typsend, typmodin, typmodout,
+ typanalyze, typstorage
+FROM pg_type WHERE typname = 'myvarchardom';
+ typinput | typoutput | typreceive | typsend | typmodin | typmodout | typanalyze | typstorage
+-----------+--------------+-------------+---------------+----------+-----------+------------------+------------
+ domain_in | myvarcharout | domain_recv | myvarcharsend | - | - | array_typanalyze | x
+(1 row)
+
+-- ensure dependencies are straight
+DROP FUNCTION myvarcharsend(myvarchar); -- fail
+ERROR: cannot drop function myvarcharsend(myvarchar) because other objects depend on it
+DETAIL: type myvarchar depends on function myvarcharsend(myvarchar)
+function myvarcharin(cstring,oid,integer) depends on type myvarchar
+function myvarcharout(myvarchar) depends on type myvarchar
+function myvarcharrecv(internal,oid,integer) depends on type myvarchar
+type myvarchardom depends on function myvarcharsend(myvarchar)
+HINT: Use DROP ... CASCADE to drop the dependent objects too.
+DROP TYPE myvarchar; -- fail
+ERROR: cannot drop type myvarchar because other objects depend on it
+DETAIL: function myvarcharin(cstring,oid,integer) depends on type myvarchar
+function myvarcharout(myvarchar) depends on type myvarchar
+function myvarcharsend(myvarchar) depends on type myvarchar
+function myvarcharrecv(internal,oid,integer) depends on type myvarchar
+type myvarchardom depends on type myvarchar
+HINT: Use DROP ... CASCADE to drop the dependent objects too.
+DROP TYPE myvarchar CASCADE;
+NOTICE: drop cascades to 5 other objects
+DETAIL: drop cascades to function myvarcharin(cstring,oid,integer)
+drop cascades to function myvarcharout(myvarchar)
+drop cascades to function myvarcharsend(myvarchar)
+drop cascades to function myvarcharrecv(internal,oid,integer)
+drop cascades to type myvarchardom
diff --git a/src/test/regress/sql/create_type.sql b/src/test/regress/sql/create_type.sql
index 68b04fd..5b176bb 100644
--- a/src/test/regress/sql/create_type.sql
+++ b/src/test/regress/sql/create_type.sql
@@ -166,3 +166,60 @@ select format_type('varchar'::regtype, 42);
select format_type('bpchar'::regtype, null);
-- this behavior difference is intentional
select format_type('bpchar'::regtype, -1);
+
+--
+-- Test CREATE/ALTER TYPE using a type that's compatible with varchar,
+-- so we can re-use those support functions
+--
+CREATE TYPE myvarchar;
+
+CREATE FUNCTION myvarcharin(cstring, oid, integer) RETURNS myvarchar
+LANGUAGE internal IMMUTABLE PARALLEL SAFE STRICT AS 'varcharin';
+
+CREATE FUNCTION myvarcharout(myvarchar) RETURNS cstring
+LANGUAGE internal IMMUTABLE PARALLEL SAFE STRICT AS 'varcharout';
+
+CREATE FUNCTION myvarcharsend(myvarchar) RETURNS bytea
+LANGUAGE internal STABLE PARALLEL SAFE STRICT AS 'varcharsend';
+
+CREATE FUNCTION myvarcharrecv(internal, oid, integer) RETURNS myvarchar
+LANGUAGE internal STABLE PARALLEL SAFE STRICT AS 'varcharrecv';
+
+-- fail, it's still a shell:
+ALTER TYPE myvarchar SET (storage = extended);
+
+CREATE TYPE myvarchar (
+ input = myvarcharin,
+ output = myvarcharout,
+ alignment = integer,
+ storage = main
+);
+
+-- want to check updating of a domain over the target type, too
+CREATE DOMAIN myvarchardom AS myvarchar;
+
+ALTER TYPE myvarchar SET (storage = plain); -- not allowed
+
+ALTER TYPE myvarchar SET (storage = extended);
+
+ALTER TYPE myvarchar SET (
+ send = myvarcharsend,
+ receive = myvarcharrecv,
+ typmod_in = varchartypmodin,
+ typmod_out = varchartypmodout,
+ analyze = array_typanalyze -- bogus, but it doesn't matter
+);
+
+SELECT typinput, typoutput, typreceive, typsend, typmodin, typmodout,
+ typanalyze, typstorage
+FROM pg_type WHERE typname = 'myvarchar';
+
+SELECT typinput, typoutput, typreceive, typsend, typmodin, typmodout,
+ typanalyze, typstorage
+FROM pg_type WHERE typname = 'myvarchardom';
+
+-- ensure dependencies are straight
+DROP FUNCTION myvarcharsend(myvarchar); -- fail
+DROP TYPE myvarchar; -- fail
+
+DROP TYPE myvarchar CASCADE;
I wrote:
> I think this is committable --- how about you?
... or not. I just noticed that the typcache tracks each type's
typstorage setting, and there's no provision for flushing/reloading
that.
As far as I can find, there is only one place where the cached
value is used, and that's in rangetypes.c which needs to know
whether the range element type is toastable. (It doesn't actually
need to know the exact value of typstorage, only whether it is or
isn't PLAIN.)
We have a number of possible fixes for that:
1. Upgrade typcache.c to support flushing and rebuilding this data.
That seems fairly expensive; while we may be forced into that someday,
I'm hesitant to do it for a fairly marginal feature like this one.
2. Stop using the typcache for this particular purpose in rangetypes.c.
That seems rather undesirable from a performance standpoint, too.
3. Drop the ability for ALTER TYPE to promote from PLAIN to not-PLAIN
typstorage, and adjust the typcache so that it only remembers boolean
toastability not the specific toasting strategy. Then the cache is
still immutable so no need for update logic.
I'm kind of liking #3, ugly as it sounds, because I'm not sure how
much of a use-case there is for the upgrade-from-PLAIN case.
Particularly now that TOAST is so ingrained in the system, it seems
rather unlikely that a production-grade data type wouldn't have
been designed to be toastable from the beginning, if there could be
any advantage to that. Either #1 or #2 seem like mighty high prices
to pay for offering an option that might have no real-world uses.
regards, tom lane
I wrote:
> 3. Drop the ability for ALTER TYPE to promote from PLAIN to not-PLAIN
> typstorage, and adjust the typcache so that it only remembers boolean
> toastability not the specific toasting strategy. Then the cache is
> still immutable so no need for update logic.
>
> I'm kind of liking #3, ugly as it sounds, because I'm not sure how
> much of a use-case there is for the upgrade-from-PLAIN case.
> Particularly now that TOAST is so ingrained in the system, it seems
> rather unlikely that a production-grade data type wouldn't have
> been designed to be toastable from the beginning, if there could be
> any advantage to that. Either #1 or #2 seem like mighty high prices
> to pay for offering an option that might have no real-world uses.
Here's a v5 based on that approach. I also added some comments about
the potential race conditions involved in recursing to domains.
regards, tom lane
diff --git a/doc/src/sgml/ref/create_type.sgml b/doc/src/sgml/ref/create_type.sgml
index 175315f..111f8e6 100644
--- a/doc/src/sgml/ref/create_type.sgml
+++ b/doc/src/sgml/ref/create_type.sgml
@@ -823,18 +823,6 @@ CREATE TYPE <replaceable class="parameter">name</replaceable>
function is written in C.
</para>
- <para>
- In <productname>PostgreSQL</productname> versions before 7.3, it
- was customary to avoid creating a shell type at all, by replacing the
- functions' forward references to the type name with the placeholder
- pseudo-type <type>opaque</type>. The <type>cstring</type> arguments and
- results also had to be declared as <type>opaque</type> before 7.3. To
- support loading of old dump files, <command>CREATE TYPE</command> will
- accept I/O functions declared using <type>opaque</type>, but it will issue
- a notice and change the function declarations to use the correct
- types.
- </para>
-
</refsect1>
<refsect1>
diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c
index 540044b..9279c05 100644
--- a/src/backend/commands/functioncmds.c
+++ b/src/backend/commands/functioncmds.c
@@ -1444,50 +1444,6 @@ SetFunctionReturnType(Oid funcOid, Oid newRetType)
/*
- * SetFunctionArgType - change declared argument type of a function
- *
- * As above, but change an argument's type.
- */
-void
-SetFunctionArgType(Oid funcOid, int argIndex, Oid newArgType)
-{
- Relation pg_proc_rel;
- HeapTuple tup;
- Form_pg_proc procForm;
- ObjectAddress func_address;
- ObjectAddress type_address;
-
- pg_proc_rel = table_open(ProcedureRelationId, RowExclusiveLock);
-
- tup = SearchSysCacheCopy1(PROCOID, ObjectIdGetDatum(funcOid));
- if (!HeapTupleIsValid(tup)) /* should not happen */
- elog(ERROR, "cache lookup failed for function %u", funcOid);
- procForm = (Form_pg_proc) GETSTRUCT(tup);
-
- if (argIndex < 0 || argIndex >= procForm->pronargs ||
- procForm->proargtypes.values[argIndex] != OPAQUEOID)
- elog(ERROR, "function %u doesn't take OPAQUE", funcOid);
-
- /* okay to overwrite copied tuple */
- procForm->proargtypes.values[argIndex] = newArgType;
-
- /* update the catalog and its indexes */
- CatalogTupleUpdate(pg_proc_rel, &tup->t_self, tup);
-
- table_close(pg_proc_rel, RowExclusiveLock);
-
- /*
- * Also update the dependency to the new type. Opaque is a pinned type, so
- * there is no old dependency record for it that we would need to remove.
- */
- ObjectAddressSet(type_address, TypeRelationId, newArgType);
- ObjectAddressSet(func_address, ProcedureRelationId, funcOid);
- recordDependencyOn(&func_address, &type_address, DEPENDENCY_NORMAL);
-}
-
-
-
-/*
* CREATE CAST
*/
ObjectAddress
diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c
index 5209736..d6e694e 100644
--- a/src/backend/commands/typecmds.c
+++ b/src/backend/commands/typecmds.c
@@ -163,7 +163,6 @@ DefineType(ParseState *pstate, List *names, List *parameters)
char *array_type;
Oid array_oid;
Oid typoid;
- Oid resulttype;
ListCell *pl;
ObjectAddress address;
@@ -196,8 +195,7 @@ DefineType(ParseState *pstate, List *names, List *parameters)
#endif
/*
- * Look to see if type already exists (presumably as a shell; if not,
- * TypeCreate will complain).
+ * Look to see if type already exists.
*/
typoid = GetSysCacheOid2(TYPENAMENSP, Anum_pg_type_oid,
CStringGetDatum(typeName),
@@ -211,35 +209,37 @@ DefineType(ParseState *pstate, List *names, List *parameters)
{
if (moveArrayTypeName(typoid, typeName, typeNamespace))
typoid = InvalidOid;
+ else
+ ereport(ERROR,
+ (errcode(ERRCODE_DUPLICATE_OBJECT),
+ errmsg("type \"%s\" already exists", typeName)));
}
/*
- * If it doesn't exist, create it as a shell, so that the OID is known for
- * use in the I/O function definitions.
+ * If this command is a parameterless CREATE TYPE, then we're just here to
+ * make a shell type, so do that (or fail if there already is a shell).
*/
- if (!OidIsValid(typoid))
+ if (parameters == NIL)
{
- address = TypeShellMake(typeName, typeNamespace, GetUserId());
- typoid = address.objectId;
- /* Make new shell type visible for modification below */
- CommandCounterIncrement();
-
- /*
- * If the command was a parameterless CREATE TYPE, we're done ---
- * creating the shell type was all we're supposed to do.
- */
- if (parameters == NIL)
- return address;
- }
- else
- {
- /* Complain if dummy CREATE TYPE and entry already exists */
- if (parameters == NIL)
+ if (OidIsValid(typoid))
ereport(ERROR,
(errcode(ERRCODE_DUPLICATE_OBJECT),
errmsg("type \"%s\" already exists", typeName)));
+
+ address = TypeShellMake(typeName, typeNamespace, GetUserId());
+ return address;
}
+ /*
+ * Otherwise, we must already have a shell type, since there is no other
+ * way that the I/O functions could have been created.
+ */
+ if (!OidIsValid(typoid))
+ ereport(ERROR,
+ (errcode(ERRCODE_DUPLICATE_OBJECT),
+ errmsg("type \"%s\" does not exist", typeName),
+ errhint("Create the type as a shell type, then create its I/O functions, then do a full CREATE
TYPE.")));
+
/* Extract the parameters from the parameter list */
foreach(pl, parameters)
{
@@ -445,63 +445,6 @@ DefineType(ParseState *pstate, List *names, List *parameters)
sendOid = findTypeSendFunction(sendName, typoid);
/*
- * Verify that I/O procs return the expected thing. If we see OPAQUE,
- * complain and change it to the correct type-safe choice.
- */
- resulttype = get_func_rettype(inputOid);
- if (resulttype != typoid)
- {
- if (resulttype == OPAQUEOID)
- {
- /* backwards-compatibility hack */
- ereport(WARNING,
- (errmsg("changing return type of function %s from %s to %s",
- NameListToString(inputName), "opaque", typeName)));
- SetFunctionReturnType(inputOid, typoid);
- }
- else
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
- errmsg("type input function %s must return type %s",
- NameListToString(inputName), typeName)));
- }
- resulttype = get_func_rettype(outputOid);
- if (resulttype != CSTRINGOID)
- {
- if (resulttype == OPAQUEOID)
- {
- /* backwards-compatibility hack */
- ereport(WARNING,
- (errmsg("changing return type of function %s from %s to %s",
- NameListToString(outputName), "opaque", "cstring")));
- SetFunctionReturnType(outputOid, CSTRINGOID);
- }
- else
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
- errmsg("type output function %s must return type %s",
- NameListToString(outputName), "cstring")));
- }
- if (receiveOid)
- {
- resulttype = get_func_rettype(receiveOid);
- if (resulttype != typoid)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
- errmsg("type receive function %s must return type %s",
- NameListToString(receiveName), typeName)));
- }
- if (sendOid)
- {
- resulttype = get_func_rettype(sendOid);
- if (resulttype != BYTEAOID)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
- errmsg("type send function %s must return type %s",
- NameListToString(sendName), "bytea")));
- }
-
- /*
* Convert typmodin/out function proc names to OIDs.
*/
if (typmodinName)
@@ -1404,16 +1347,9 @@ DefineRange(CreateRangeStmt *stmt)
}
/*
- * If it doesn't exist, create it as a shell, so that the OID is known for
- * use in the range function definitions.
+ * Unlike DefineType(), we don't insist on a shell type existing first, as
+ * it's only needed if the user wants to specify a canonical function.
*/
- if (!OidIsValid(typoid))
- {
- address = TypeShellMake(typeName, typeNamespace, GetUserId());
- typoid = address.objectId;
- /* Make new shell type visible for modification below */
- CommandCounterIncrement();
- }
/* Extract the parameters from the parameter list */
foreach(lc, stmt->params)
@@ -1502,8 +1438,15 @@ DefineRange(CreateRangeStmt *stmt)
/* Identify support functions, if provided */
if (rangeCanonicalName != NIL)
+ {
+ if (!OidIsValid(typoid))
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("cannot specify a canonical function without a pre-created shell type"),
+ errhint("Create the type as a shell type, then create its canonicalization function, then do a
fullCREATE TYPE.")));
rangeCanonical = findRangeCanonicalFunction(rangeCanonicalName,
typoid);
+ }
else
rangeCanonical = InvalidOid;
@@ -1555,7 +1498,8 @@ DefineRange(CreateRangeStmt *stmt)
0, /* Array dimensions of typbasetype */
false, /* Type NOT NULL */
InvalidOid); /* type's collation (ranges never have one) */
- Assert(typoid == address.objectId);
+ Assert(typoid == InvalidOid || typoid == address.objectId);
+ typoid = address.objectId;
/* Create the entry in pg_range */
RangeCreate(typoid, rangeSubtype, rangeCollation, rangeSubOpclass,
@@ -1695,63 +1639,32 @@ findTypeInputFunction(List *procname, Oid typeOid)
/*
* Input functions can take a single argument of type CSTRING, or three
- * arguments (string, typioparam OID, typmod).
- *
- * For backwards compatibility we allow OPAQUE in place of CSTRING; if we
- * see this, we issue a warning and fix up the pg_proc entry.
+ * arguments (string, typioparam OID, typmod). They must return the
+ * target type.
*/
argList[0] = CSTRINGOID;
procOid = LookupFuncName(procname, 1, argList, true);
- if (OidIsValid(procOid))
- return procOid;
-
- argList[1] = OIDOID;
- argList[2] = INT4OID;
-
- procOid = LookupFuncName(procname, 3, argList, true);
- if (OidIsValid(procOid))
- return procOid;
-
- /* No luck, try it with OPAQUE */
- argList[0] = OPAQUEOID;
-
- procOid = LookupFuncName(procname, 1, argList, true);
-
if (!OidIsValid(procOid))
{
argList[1] = OIDOID;
argList[2] = INT4OID;
procOid = LookupFuncName(procname, 3, argList, true);
+ if (!OidIsValid(procOid))
+ ereport(ERROR,
+ (errcode(ERRCODE_UNDEFINED_FUNCTION),
+ errmsg("function %s does not exist",
+ func_signature_string(procname, 1, NIL, argList))));
}
- if (OidIsValid(procOid))
- {
- /* Found, but must complain and fix the pg_proc entry */
- ereport(WARNING,
- (errmsg("changing argument type of function %s from \"opaque\" to \"cstring\"",
- NameListToString(procname))));
- SetFunctionArgType(procOid, 0, CSTRINGOID);
-
- /*
- * Need CommandCounterIncrement since DefineType will likely try to
- * alter the pg_proc tuple again.
- */
- CommandCounterIncrement();
-
- return procOid;
- }
-
- /* Use CSTRING (preferred) in the error message */
- argList[0] = CSTRINGOID;
-
- ereport(ERROR,
- (errcode(ERRCODE_UNDEFINED_FUNCTION),
- errmsg("function %s does not exist",
- func_signature_string(procname, 1, NIL, argList))));
+ if (get_func_rettype(procOid) != typeOid)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("type input function %s must return type %s",
+ NameListToString(procname), format_type_be(typeOid))));
- return InvalidOid; /* keep compiler quiet */
+ return procOid;
}
static Oid
@@ -1761,48 +1674,25 @@ findTypeOutputFunction(List *procname, Oid typeOid)
Oid procOid;
/*
- * Output functions can take a single argument of the type.
- *
- * For backwards compatibility we allow OPAQUE in place of the actual type
- * name; if we see this, we issue a warning and fix up the pg_proc entry.
+ * Output functions always take a single argument of the type and return
+ * cstring.
*/
argList[0] = typeOid;
procOid = LookupFuncName(procname, 1, argList, true);
- if (OidIsValid(procOid))
- return procOid;
-
- /* No luck, try it with OPAQUE */
- argList[0] = OPAQUEOID;
-
- procOid = LookupFuncName(procname, 1, argList, true);
-
- if (OidIsValid(procOid))
- {
- /* Found, but must complain and fix the pg_proc entry */
- ereport(WARNING,
- (errmsg("changing argument type of function %s from \"opaque\" to %s",
- NameListToString(procname), format_type_be(typeOid))));
- SetFunctionArgType(procOid, 0, typeOid);
-
- /*
- * Need CommandCounterIncrement since DefineType will likely try to
- * alter the pg_proc tuple again.
- */
- CommandCounterIncrement();
-
- return procOid;
- }
-
- /* Use type name, not OPAQUE, in the failure message. */
- argList[0] = typeOid;
+ if (!OidIsValid(procOid))
+ ereport(ERROR,
+ (errcode(ERRCODE_UNDEFINED_FUNCTION),
+ errmsg("function %s does not exist",
+ func_signature_string(procname, 1, NIL, argList))));
- ereport(ERROR,
- (errcode(ERRCODE_UNDEFINED_FUNCTION),
- errmsg("function %s does not exist",
- func_signature_string(procname, 1, NIL, argList))));
+ if (get_func_rettype(procOid) != CSTRINGOID)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("type output function %s must return type %s",
+ NameListToString(procname), "cstring")));
- return InvalidOid; /* keep compiler quiet */
+ return procOid;
}
static Oid
@@ -1813,27 +1703,32 @@ findTypeReceiveFunction(List *procname, Oid typeOid)
/*
* Receive functions can take a single argument of type INTERNAL, or three
- * arguments (internal, typioparam OID, typmod).
+ * arguments (internal, typioparam OID, typmod). They must return the
+ * target type.
*/
argList[0] = INTERNALOID;
procOid = LookupFuncName(procname, 1, argList, true);
- if (OidIsValid(procOid))
- return procOid;
-
- argList[1] = OIDOID;
- argList[2] = INT4OID;
+ if (!OidIsValid(procOid))
+ {
+ argList[1] = OIDOID;
+ argList[2] = INT4OID;
- procOid = LookupFuncName(procname, 3, argList, true);
- if (OidIsValid(procOid))
- return procOid;
+ procOid = LookupFuncName(procname, 3, argList, true);
+ if (!OidIsValid(procOid))
+ ereport(ERROR,
+ (errcode(ERRCODE_UNDEFINED_FUNCTION),
+ errmsg("function %s does not exist",
+ func_signature_string(procname, 1, NIL, argList))));
+ }
- ereport(ERROR,
- (errcode(ERRCODE_UNDEFINED_FUNCTION),
- errmsg("function %s does not exist",
- func_signature_string(procname, 1, NIL, argList))));
+ if (get_func_rettype(procOid) != typeOid)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("type receive function %s must return type %s",
+ NameListToString(procname), format_type_be(typeOid))));
- return InvalidOid; /* keep compiler quiet */
+ return procOid;
}
static Oid
@@ -1843,20 +1738,25 @@ findTypeSendFunction(List *procname, Oid typeOid)
Oid procOid;
/*
- * Send functions can take a single argument of the type.
+ * Send functions always take a single argument of the type and return
+ * bytea.
*/
argList[0] = typeOid;
procOid = LookupFuncName(procname, 1, argList, true);
- if (OidIsValid(procOid))
- return procOid;
+ if (!OidIsValid(procOid))
+ ereport(ERROR,
+ (errcode(ERRCODE_UNDEFINED_FUNCTION),
+ errmsg("function %s does not exist",
+ func_signature_string(procname, 1, NIL, argList))));
- ereport(ERROR,
- (errcode(ERRCODE_UNDEFINED_FUNCTION),
- errmsg("function %s does not exist",
- func_signature_string(procname, 1, NIL, argList))));
+ if (get_func_rettype(procOid) != BYTEAOID)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("type send function %s must return type %s",
+ NameListToString(procname), "bytea")));
- return InvalidOid; /* keep compiler quiet */
+ return procOid;
}
static Oid
diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h
index dede9d7..0992c23 100644
--- a/src/include/commands/defrem.h
+++ b/src/include/commands/defrem.h
@@ -55,7 +55,6 @@ extern Oid ResolveOpClass(List *opclass, Oid attrType,
extern ObjectAddress CreateFunction(ParseState *pstate, CreateFunctionStmt *stmt);
extern void RemoveFunctionById(Oid funcOid);
extern void SetFunctionReturnType(Oid funcOid, Oid newRetType);
-extern void SetFunctionArgType(Oid funcOid, int argIndex, Oid newArgType);
extern ObjectAddress AlterFunction(ParseState *pstate, AlterFunctionStmt *stmt);
extern ObjectAddress CreateCast(CreateCastStmt *stmt);
extern void DropCastById(Oid castOid);
diff --git a/src/test/regress/expected/create_type.out b/src/test/regress/expected/create_type.out
index 8309756..eb55e25 100644
--- a/src/test/regress/expected/create_type.out
+++ b/src/test/regress/expected/create_type.out
@@ -83,8 +83,10 @@ SELECT * FROM default_test;
zippo | 42
(1 row)
+-- We need a shell type to test some CREATE TYPE failure cases with
+CREATE TYPE bogus_type;
-- invalid: non-lowercase quoted identifiers
-CREATE TYPE case_int42 (
+CREATE TYPE bogus_type (
"Internallength" = 4,
"Input" = int42_in,
"Output" = int42_out,
@@ -111,6 +113,20 @@ WARNING: type attribute "Passedbyvalue" not recognized
LINE 7: "Passedbyvalue"
^
ERROR: type input function must be specified
+-- invalid: input/output function incompatibility
+CREATE TYPE bogus_type (INPUT = array_in,
+ OUTPUT = array_out,
+ ELEMENT = int,
+ INTERNALLENGTH = 32);
+ERROR: type input function array_in must return type bogus_type
+DROP TYPE bogus_type;
+-- It no longer is possible to issue CREATE TYPE without making a shell first
+CREATE TYPE bogus_type (INPUT = array_in,
+ OUTPUT = array_out,
+ ELEMENT = int,
+ INTERNALLENGTH = 32);
+ERROR: type "bogus_type" does not exist
+HINT: Create the type as a shell type, then create its I/O functions, then do a full CREATE TYPE.
-- Test stand-alone composite type
CREATE TYPE default_test_row AS (f1 text_w_default, f2 int42);
CREATE FUNCTION get_default_test() RETURNS SETOF default_test_row AS '
@@ -137,28 +153,25 @@ ERROR: type "text_w_default" already exists
DROP TYPE default_test_row CASCADE;
NOTICE: drop cascades to function get_default_test()
DROP TABLE default_test;
--- Check type create with input/output incompatibility
-CREATE TYPE not_existing_type (INPUT = array_in,
- OUTPUT = array_out,
- ELEMENT = int,
- INTERNALLENGTH = 32);
-ERROR: function array_out(not_existing_type) does not exist
--- Check dependency transfer of opaque functions when creating a new type
-CREATE FUNCTION base_fn_in(cstring) RETURNS opaque AS 'boolin'
+-- Check dependencies are established when creating a new type
+CREATE TYPE base_type;
+CREATE FUNCTION base_fn_in(cstring) RETURNS base_type AS 'boolin'
LANGUAGE internal IMMUTABLE STRICT;
-CREATE FUNCTION base_fn_out(opaque) RETURNS opaque AS 'boolout'
+NOTICE: return type base_type is only a shell
+CREATE FUNCTION base_fn_out(base_type) RETURNS cstring AS 'boolout'
LANGUAGE internal IMMUTABLE STRICT;
+NOTICE: argument type base_type is only a shell
CREATE TYPE base_type(INPUT = base_fn_in, OUTPUT = base_fn_out);
-WARNING: changing argument type of function base_fn_out from "opaque" to base_type
-WARNING: changing return type of function base_fn_in from opaque to base_type
-WARNING: changing return type of function base_fn_out from opaque to cstring
DROP FUNCTION base_fn_in(cstring); -- error
ERROR: cannot drop function base_fn_in(cstring) because other objects depend on it
DETAIL: type base_type depends on function base_fn_in(cstring)
function base_fn_out(base_type) depends on type base_type
HINT: Use DROP ... CASCADE to drop the dependent objects too.
-DROP FUNCTION base_fn_out(opaque); -- error
-ERROR: function base_fn_out(opaque) does not exist
+DROP FUNCTION base_fn_out(base_type); -- error
+ERROR: cannot drop function base_fn_out(base_type) because other objects depend on it
+DETAIL: type base_type depends on function base_fn_out(base_type)
+function base_fn_in(cstring) depends on type base_type
+HINT: Use DROP ... CASCADE to drop the dependent objects too.
DROP TYPE base_type; -- error
ERROR: cannot drop type base_type because other objects depend on it
DETAIL: function base_fn_in(cstring) depends on type base_type
diff --git a/src/test/regress/sql/create_type.sql b/src/test/regress/sql/create_type.sql
index 3d1deba..68b04fd 100644
--- a/src/test/regress/sql/create_type.sql
+++ b/src/test/regress/sql/create_type.sql
@@ -84,8 +84,11 @@ INSERT INTO default_test DEFAULT VALUES;
SELECT * FROM default_test;
+-- We need a shell type to test some CREATE TYPE failure cases with
+CREATE TYPE bogus_type;
+
-- invalid: non-lowercase quoted identifiers
-CREATE TYPE case_int42 (
+CREATE TYPE bogus_type (
"Internallength" = 4,
"Input" = int42_in,
"Output" = int42_out,
@@ -94,6 +97,20 @@ CREATE TYPE case_int42 (
"Passedbyvalue"
);
+-- invalid: input/output function incompatibility
+CREATE TYPE bogus_type (INPUT = array_in,
+ OUTPUT = array_out,
+ ELEMENT = int,
+ INTERNALLENGTH = 32);
+
+DROP TYPE bogus_type;
+
+-- It no longer is possible to issue CREATE TYPE without making a shell first
+CREATE TYPE bogus_type (INPUT = array_in,
+ OUTPUT = array_out,
+ ELEMENT = int,
+ INTERNALLENGTH = 32);
+
-- Test stand-alone composite type
CREATE TYPE default_test_row AS (f1 text_w_default, f2 int42);
@@ -119,20 +136,15 @@ DROP TYPE default_test_row CASCADE;
DROP TABLE default_test;
--- Check type create with input/output incompatibility
-CREATE TYPE not_existing_type (INPUT = array_in,
- OUTPUT = array_out,
- ELEMENT = int,
- INTERNALLENGTH = 32);
-
--- Check dependency transfer of opaque functions when creating a new type
-CREATE FUNCTION base_fn_in(cstring) RETURNS opaque AS 'boolin'
+-- Check dependencies are established when creating a new type
+CREATE TYPE base_type;
+CREATE FUNCTION base_fn_in(cstring) RETURNS base_type AS 'boolin'
LANGUAGE internal IMMUTABLE STRICT;
-CREATE FUNCTION base_fn_out(opaque) RETURNS opaque AS 'boolout'
+CREATE FUNCTION base_fn_out(base_type) RETURNS cstring AS 'boolout'
LANGUAGE internal IMMUTABLE STRICT;
CREATE TYPE base_type(INPUT = base_fn_in, OUTPUT = base_fn_out);
DROP FUNCTION base_fn_in(cstring); -- error
-DROP FUNCTION base_fn_out(opaque); -- error
+DROP FUNCTION base_fn_out(base_type); -- error
DROP TYPE base_type; -- error
DROP TYPE base_type CASCADE;
diff --git a/doc/src/sgml/datatype.sgml b/doc/src/sgml/datatype.sgml
index d1d0331..410eaed 100644
--- a/doc/src/sgml/datatype.sgml
+++ b/doc/src/sgml/datatype.sgml
@@ -4827,10 +4827,6 @@ SELECT * FROM pg_attribute
<primary>unknown</primary>
</indexterm>
- <indexterm zone="datatype-pseudo">
- <primary>opaque</primary>
- </indexterm>
-
<para>
The <productname>PostgreSQL</productname> type system contains a
number of special-purpose entries that are collectively called
@@ -4953,12 +4949,6 @@ SELECT * FROM pg_attribute
<entry>Identifies a not-yet-resolved type, e.g. of an undecorated
string literal.</entry>
</row>
-
- <row>
- <entry><type>opaque</type></entry>
- <entry>An obsolete type name that formerly served many of the above
- purposes.</entry>
- </row>
</tbody>
</tgroup>
</table>
diff --git a/doc/src/sgml/ref/create_language.sgml b/doc/src/sgml/ref/create_language.sgml
index af9d115..2243ee6 100644
--- a/doc/src/sgml/ref/create_language.sgml
+++ b/doc/src/sgml/ref/create_language.sgml
@@ -211,16 +211,6 @@ CREATE [ OR REPLACE ] [ TRUSTED ] [ PROCEDURAL ] LANGUAGE <replaceable class="pa
database, which will cause it to be available automatically in
all subsequently-created databases.
</para>
-
- <para>
- In <productname>PostgreSQL</productname> versions before 7.3, it was
- necessary to declare handler functions as returning the placeholder
- type <type>opaque</type>, rather than <type>language_handler</type>.
- To support loading
- of old dump files, <command>CREATE LANGUAGE</command> will accept a function
- declared as returning <type>opaque</type>, but it will issue a notice and
- change the function's declared return type to <type>language_handler</type>.
- </para>
</refsect1>
<refsect1 id="sql-createlanguage-examples">
diff --git a/doc/src/sgml/ref/create_trigger.sgml b/doc/src/sgml/ref/create_trigger.sgml
index 3339a4b..3b8f25e 100644
--- a/doc/src/sgml/ref/create_trigger.sgml
+++ b/doc/src/sgml/ref/create_trigger.sgml
@@ -543,15 +543,6 @@ UPDATE OF <replaceable>column_name1</replaceable> [, <replaceable>column_name2</
row-level triggers with transition relations cannot be defined on
partitions or inheritance child tables.
</para>
-
- <para>
- In <productname>PostgreSQL</productname> versions before 7.3, it was
- necessary to declare trigger functions as returning the placeholder
- type <type>opaque</type>, rather than <type>trigger</type>. To support loading
- of old dump files, <command>CREATE TRIGGER</command> will accept a function
- declared as returning <type>opaque</type>, but it will issue a notice and
- change the function's declared return type to <type>trigger</type>.
- </para>
</refsect1>
<refsect1 id="sql-createtrigger-examples">
diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c
index 9279c05..e634ccf 100644
--- a/src/backend/commands/functioncmds.c
+++ b/src/backend/commands/functioncmds.c
@@ -1399,49 +1399,6 @@ AlterFunction(ParseState *pstate, AlterFunctionStmt *stmt)
return address;
}
-/*
- * SetFunctionReturnType - change declared return type of a function
- *
- * This is presently only used for adjusting legacy functions that return
- * OPAQUE to return whatever we find their correct definition should be.
- * The caller should emit a suitable warning explaining what we did.
- */
-void
-SetFunctionReturnType(Oid funcOid, Oid newRetType)
-{
- Relation pg_proc_rel;
- HeapTuple tup;
- Form_pg_proc procForm;
- ObjectAddress func_address;
- ObjectAddress type_address;
-
- pg_proc_rel = table_open(ProcedureRelationId, RowExclusiveLock);
-
- tup = SearchSysCacheCopy1(PROCOID, ObjectIdGetDatum(funcOid));
- if (!HeapTupleIsValid(tup)) /* should not happen */
- elog(ERROR, "cache lookup failed for function %u", funcOid);
- procForm = (Form_pg_proc) GETSTRUCT(tup);
-
- if (procForm->prorettype != OPAQUEOID) /* caller messed up */
- elog(ERROR, "function %u doesn't return OPAQUE", funcOid);
-
- /* okay to overwrite copied tuple */
- procForm->prorettype = newRetType;
-
- /* update the catalog and its indexes */
- CatalogTupleUpdate(pg_proc_rel, &tup->t_self, tup);
-
- table_close(pg_proc_rel, RowExclusiveLock);
-
- /*
- * Also update the dependency to the new type. Opaque is a pinned type, so
- * there is no old dependency record for it that we would need to remove.
- */
- ObjectAddressSet(type_address, TypeRelationId, newRetType);
- ObjectAddressSet(func_address, ProcedureRelationId, funcOid);
- recordDependencyOn(&func_address, &type_address, DEPENDENCY_NORMAL);
-}
-
/*
* CREATE CAST
diff --git a/src/backend/commands/proclang.c b/src/backend/commands/proclang.c
index 9d72edb..21ceeb7 100644
--- a/src/backend/commands/proclang.c
+++ b/src/backend/commands/proclang.c
@@ -74,27 +74,10 @@ CreateProceduralLanguage(CreatePLangStmt *stmt)
handlerOid = LookupFuncName(stmt->plhandler, 0, NULL, false);
funcrettype = get_func_rettype(handlerOid);
if (funcrettype != LANGUAGE_HANDLEROID)
- {
- /*
- * We allow OPAQUE just so we can load old dump files. When we see a
- * handler function declared OPAQUE, change it to LANGUAGE_HANDLER.
- * (This is probably obsolete and removable?)
- */
- if (funcrettype == OPAQUEOID)
- {
- ereport(WARNING,
- (errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("changing return type of function %s from %s to %s",
- NameListToString(stmt->plhandler),
- "opaque", "language_handler")));
- SetFunctionReturnType(handlerOid, LANGUAGE_HANDLEROID);
- }
- else
- ereport(ERROR,
- (errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("function %s must return type %s",
- NameListToString(stmt->plhandler), "language_handler")));
- }
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("function %s must return type %s",
+ NameListToString(stmt->plhandler), "language_handler")));
/* validate the inline function */
if (stmt->plinline)
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index 6e8b722..056a912 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -699,25 +699,10 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString,
}
funcrettype = get_func_rettype(funcoid);
if (funcrettype != TRIGGEROID)
- {
- /*
- * We allow OPAQUE just so we can load old dump files. When we see a
- * trigger function declared OPAQUE, change it to TRIGGER.
- */
- if (funcrettype == OPAQUEOID)
- {
- ereport(WARNING,
- (errmsg("changing return type of function %s from %s to %s",
- NameListToString(stmt->funcname),
- "opaque", "trigger")));
- SetFunctionReturnType(funcoid, TRIGGEROID);
- }
- else
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
- errmsg("function %s must return type %s",
- NameListToString(stmt->funcname), "trigger")));
- }
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("function %s must return type %s",
+ NameListToString(stmt->funcname), "trigger")));
/*
* If the command is a user-entered CREATE CONSTRAINT TRIGGER command that
diff --git a/src/backend/utils/adt/pseudotypes.c b/src/backend/utils/adt/pseudotypes.c
index ad0e363..4653fc3 100644
--- a/src/backend/utils/adt/pseudotypes.c
+++ b/src/backend/utils/adt/pseudotypes.c
@@ -415,7 +415,6 @@ PSEUDOTYPE_DUMMY_IO_FUNCS(fdw_handler);
PSEUDOTYPE_DUMMY_IO_FUNCS(index_am_handler);
PSEUDOTYPE_DUMMY_IO_FUNCS(tsm_handler);
PSEUDOTYPE_DUMMY_IO_FUNCS(internal);
-PSEUDOTYPE_DUMMY_IO_FUNCS(opaque);
PSEUDOTYPE_DUMMY_IO_FUNCS(anyelement);
PSEUDOTYPE_DUMMY_IO_FUNCS(anynonarray);
PSEUDOTYPE_DUMMY_IO_FUNCS(table_am_handler);
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index ef15390..d3ab5bb 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -82,10 +82,9 @@ typedef struct
typedef enum OidOptions
{
- zeroAsOpaque = 1,
- zeroAsAny = 2,
- zeroAsStar = 4,
- zeroAsNone = 8
+ zeroIsError = 1,
+ zeroAsStar = 2,
+ zeroAsNone = 4
} OidOptions;
/* global decls */
@@ -122,8 +121,6 @@ static SimpleStringList tabledata_exclude_patterns = {NULL, NULL};
static SimpleOidList tabledata_exclude_oids = {NULL, NULL};
-char g_opaque_type[10]; /* name for the opaque type */
-
/* placeholders for the delimiters for comments */
char g_comment_start[10];
char g_comment_end[10];
@@ -404,7 +401,6 @@ main(int argc, char **argv)
strcpy(g_comment_start, "-- ");
g_comment_end[0] = '\0';
- strcpy(g_opaque_type, "opaque");
progname = get_progname(argv[0]);
@@ -10736,7 +10732,7 @@ dumpBaseType(Archive *fout, TypeInfo *tyinfo)
{
char *elemType;
- elemType = getFormattedTypeName(fout, tyinfo->typelem, zeroAsOpaque);
+ elemType = getFormattedTypeName(fout, tyinfo->typelem, zeroIsError);
appendPQExpBuffer(q, ",\n ELEMENT = %s", elemType);
free(elemType);
}
@@ -11547,7 +11543,7 @@ format_function_arguments_old(Archive *fout,
const char *argname;
typid = allargtypes ? atooid(allargtypes[j]) : finfo->argtypes[j];
- typname = getFormattedTypeName(fout, typid, zeroAsOpaque);
+ typname = getFormattedTypeName(fout, typid, zeroIsError);
if (argmodes)
{
@@ -11616,7 +11612,7 @@ format_function_signature(Archive *fout, FuncInfo *finfo, bool honor_quotes)
appendPQExpBufferStr(&fn, ", ");
typname = getFormattedTypeName(fout, finfo->argtypes[j],
- zeroAsOpaque);
+ zeroIsError);
appendPQExpBufferStr(&fn, typname);
free(typname);
}
@@ -12021,7 +12017,7 @@ dumpFunc(Archive *fout, FuncInfo *finfo)
else
{
rettypename = getFormattedTypeName(fout, finfo->prorettype,
- zeroAsOpaque);
+ zeroIsError);
appendPQExpBuffer(q, " RETURNS %s%s",
(proretset[0] == 't') ? "SETOF " : "",
rettypename);
@@ -13740,7 +13736,7 @@ format_aggregate_signature(AggInfo *agginfo, Archive *fout, bool honor_quotes)
char *typname;
typname = getFormattedTypeName(fout, agginfo->aggfn.argtypes[j],
- zeroAsOpaque);
+ zeroIsError);
appendPQExpBuffer(&buf, "%s%s",
(j > 0) ? ", " : "",
@@ -18363,11 +18359,7 @@ getFormattedTypeName(Archive *fout, Oid oid, OidOptions opts)
if (oid == 0)
{
- if ((opts & zeroAsOpaque) != 0)
- return pg_strdup(g_opaque_type);
- else if ((opts & zeroAsAny) != 0)
- return pg_strdup("'any'");
- else if ((opts & zeroAsStar) != 0)
+ if ((opts & zeroAsStar) != 0)
return pg_strdup("*");
else if ((opts & zeroAsNone) != 0)
return pg_strdup("NONE");
diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index 21004e5..852020b 100644
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -640,8 +640,6 @@ typedef struct _extensionMemberId
extern char g_comment_start[10];
extern char g_comment_end[10];
-extern char g_opaque_type[10]; /* name for the opaque type */
-
/*
* common utility functions
*/
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 07a86c7..7fb574f 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -7054,12 +7054,6 @@
{ oid => '2305', descr => 'I/O',
proname => 'internal_out', prorettype => 'cstring', proargtypes => 'internal',
prosrc => 'internal_out' },
-{ oid => '2306', descr => 'I/O',
- proname => 'opaque_in', proisstrict => 'f', prorettype => 'opaque',
- proargtypes => 'cstring', prosrc => 'opaque_in' },
-{ oid => '2307', descr => 'I/O',
- proname => 'opaque_out', prorettype => 'cstring', proargtypes => 'opaque',
- prosrc => 'opaque_out' },
{ oid => '2312', descr => 'I/O',
proname => 'anyelement_in', prorettype => 'anyelement',
proargtypes => 'cstring', prosrc => 'anyelement_in' },
@@ -7067,10 +7061,10 @@
proname => 'anyelement_out', prorettype => 'cstring',
proargtypes => 'anyelement', prosrc => 'anyelement_out' },
{ oid => '2398', descr => 'I/O',
- proname => 'shell_in', proisstrict => 'f', prorettype => 'opaque',
+ proname => 'shell_in', proisstrict => 'f', prorettype => 'void',
proargtypes => 'cstring', prosrc => 'shell_in' },
{ oid => '2399', descr => 'I/O',
- proname => 'shell_out', prorettype => 'cstring', proargtypes => 'opaque',
+ proname => 'shell_out', prorettype => 'cstring', proargtypes => 'void',
prosrc => 'shell_out' },
{ oid => '2597', descr => 'I/O',
proname => 'domain_in', proisstrict => 'f', provolatile => 's',
diff --git a/src/include/catalog/pg_type.dat b/src/include/catalog/pg_type.dat
index 4cf2b9d..b00597d 100644
--- a/src/include/catalog/pg_type.dat
+++ b/src/include/catalog/pg_type.dat
@@ -546,10 +546,6 @@
typtype => 'p', typcategory => 'P', typinput => 'internal_in',
typoutput => 'internal_out', typreceive => '-', typsend => '-',
typalign => 'ALIGNOF_POINTER' },
-{ oid => '2282', descr => 'obsolete, deprecated pseudo-type',
- typname => 'opaque', typlen => '4', typbyval => 't', typtype => 'p',
- typcategory => 'P', typinput => 'opaque_in', typoutput => 'opaque_out',
- typreceive => '-', typsend => '-', typalign => 'i' },
{ oid => '2283', descr => 'pseudo-type representing a polymorphic base type',
typname => 'anyelement', typlen => '4', typbyval => 't', typtype => 'p',
typcategory => 'P', typinput => 'anyelement_in',
diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h
index 0992c23..5cd6975 100644
--- a/src/include/commands/defrem.h
+++ b/src/include/commands/defrem.h
@@ -54,7 +54,6 @@ extern Oid ResolveOpClass(List *opclass, Oid attrType,
/* commands/functioncmds.c */
extern ObjectAddress CreateFunction(ParseState *pstate, CreateFunctionStmt *stmt);
extern void RemoveFunctionById(Oid funcOid);
-extern void SetFunctionReturnType(Oid funcOid, Oid newRetType);
extern ObjectAddress AlterFunction(ParseState *pstate, AlterFunctionStmt *stmt);
extern ObjectAddress CreateCast(CreateCastStmt *stmt);
extern void DropCastById(Oid castOid);
diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c
index a65bce0..5fdf303 100644
--- a/src/pl/plperl/plperl.c
+++ b/src/pl/plperl/plperl.c
@@ -2000,9 +2000,7 @@ plperl_validator(PG_FUNCTION_ARGS)
/* except for TRIGGER, EVTTRIGGER, RECORD, or VOID */
if (functyptype == TYPTYPE_PSEUDO)
{
- /* we assume OPAQUE with no arguments means a trigger */
- if (proc->prorettype == TRIGGEROID ||
- (proc->prorettype == OPAQUEOID && proc->pronargs == 0))
+ if (proc->prorettype == TRIGGEROID)
is_trigger = true;
else if (proc->prorettype == EVTTRIGGEROID)
is_event_trigger = true;
diff --git a/src/pl/plpgsql/src/pl_handler.c b/src/pl/plpgsql/src/pl_handler.c
index b83087e..b434818 100644
--- a/src/pl/plpgsql/src/pl_handler.c
+++ b/src/pl/plpgsql/src/pl_handler.c
@@ -421,12 +421,10 @@ plpgsql_validator(PG_FUNCTION_ARGS)
functyptype = get_typtype(proc->prorettype);
/* Disallow pseudotype result */
- /* except for TRIGGER, RECORD, VOID, or polymorphic */
+ /* except for TRIGGER, EVTTRIGGER, RECORD, VOID, or polymorphic */
if (functyptype == TYPTYPE_PSEUDO)
{
- /* we assume OPAQUE with no arguments means a trigger */
- if (proc->prorettype == TRIGGEROID ||
- (proc->prorettype == OPAQUEOID && proc->pronargs == 0))
+ if (proc->prorettype == TRIGGEROID)
is_dml_trigger = true;
else if (proc->prorettype == EVTTRIGGEROID)
is_event_trigger = true;
diff --git a/src/pl/plpython/plpy_main.c b/src/pl/plpython/plpy_main.c
index 882d69e..3eedaa8 100644
--- a/src/pl/plpython/plpy_main.c
+++ b/src/pl/plpython/plpy_main.c
@@ -379,9 +379,7 @@ plpython2_inline_handler(PG_FUNCTION_ARGS)
static bool
PLy_procedure_is_trigger(Form_pg_proc procStruct)
{
- return (procStruct->prorettype == TRIGGEROID ||
- (procStruct->prorettype == OPAQUEOID &&
- procStruct->pronargs == 0));
+ return (procStruct->prorettype == TRIGGEROID);
}
static void
diff --git a/src/test/regress/expected/opr_sanity.out b/src/test/regress/expected/opr_sanity.out
index fb6c029..40468e8 100644
--- a/src/test/regress/expected/opr_sanity.out
+++ b/src/test/regress/expected/opr_sanity.out
@@ -384,7 +384,7 @@ FROM pg_proc as p1
WHERE p1.prorettype = 'cstring'::regtype
AND NOT EXISTS(SELECT 1 FROM pg_type WHERE typoutput = p1.oid)
AND NOT EXISTS(SELECT 1 FROM pg_type WHERE typmodout = p1.oid)
- AND p1.oid != 'shell_out(opaque)'::regprocedure
+ AND p1.oid != 'shell_out(void)'::regprocedure
ORDER BY 1;
oid | proname
------+--------------
diff --git a/src/test/regress/sql/opr_sanity.sql b/src/test/regress/sql/opr_sanity.sql
index 8351b64..f06f245 100644
--- a/src/test/regress/sql/opr_sanity.sql
+++ b/src/test/regress/sql/opr_sanity.sql
@@ -309,7 +309,7 @@ FROM pg_proc as p1
WHERE p1.prorettype = 'cstring'::regtype
AND NOT EXISTS(SELECT 1 FROM pg_type WHERE typoutput = p1.oid)
AND NOT EXISTS(SELECT 1 FROM pg_type WHERE typmodout = p1.oid)
- AND p1.oid != 'shell_out(opaque)'::regprocedure
+ AND p1.oid != 'shell_out(void)'::regprocedure
ORDER BY 1;
-- Check for length inconsistencies between the various argument-info arrays.
diff --git a/doc/src/sgml/ref/alter_type.sgml b/doc/src/sgml/ref/alter_type.sgml
index 67be1dd..0380f6c 100644
--- a/doc/src/sgml/ref/alter_type.sgml
+++ b/doc/src/sgml/ref/alter_type.sgml
@@ -30,6 +30,7 @@ ALTER TYPE <replaceable class="parameter">name</replaceable> RENAME TO <replacea
ALTER TYPE <replaceable class="parameter">name</replaceable> SET SCHEMA <replaceable
class="parameter">new_schema</replaceable>
ALTER TYPE <replaceable class="parameter">name</replaceable> ADD VALUE [ IF NOT EXISTS ] <replaceable
class="parameter">new_enum_value</replaceable>[ { BEFORE | AFTER } <replaceable
class="parameter">neighbor_enum_value</replaceable>]
ALTER TYPE <replaceable class="parameter">name</replaceable> RENAME VALUE <replaceable
class="parameter">existing_enum_value</replaceable>TO <replaceable class="parameter">new_enum_value</replaceable>
+ALTER TYPE <replaceable class="parameter">name</replaceable> SET ( <replaceable
class="parameter">property</replaceable>= <replaceable class="parameter">value</replaceable> [, ... ] )
<phrase>where <replaceable class="parameter">action</replaceable> is one of:</phrase>
@@ -70,7 +71,7 @@ ALTER TYPE <replaceable class="parameter">name</replaceable> RENAME VALUE <repla
</varlistentry>
<varlistentry>
- <term><literal>SET DATA TYPE</literal></term>
+ <term><literal>ALTER ATTRIBUTE ... SET DATA TYPE</literal></term>
<listitem>
<para>
This form changes the type of an attribute of a composite type.
@@ -135,6 +136,80 @@ ALTER TYPE <replaceable class="parameter">name</replaceable> RENAME VALUE <repla
</para>
</listitem>
</varlistentry>
+
+ <varlistentry>
+ <term>
+ <literal>SET ( <replaceable class="parameter">property</replaceable> = <replaceable
class="parameter">value</replaceable>[, ... ] )</literal>
+ </term>
+ <listitem>
+ <para>
+ This form is only applicable to base types. It allows adjustment of a
+ subset of the base-type properties that can be set in <command>CREATE
+ TYPE</command>. Specifically, these properties can be changed:
+ <itemizedlist>
+ <listitem>
+ <para>
+ <literal>RECEIVE</literal> can be set to the name of a binary input
+ function, or <literal>NONE</literal> to remove the type's binary
+ input function. Using this option requires superuser privilege.
+ </para>
+ </listitem>
+ <listitem>
+ <para>
+ <literal>SEND</literal> can be set to the name of a binary output
+ function, or <literal>NONE</literal> to remove the type's binary
+ output function. Using this option requires superuser privilege.
+ </para>
+ </listitem>
+ <listitem>
+ <para>
+ <literal>TYPMOD_IN</literal> can be set to the name of a type
+ modifier input function, or <literal>NONE</literal> to remove the
+ type's type modifier input function. Using this option requires
+ superuser privilege.
+ </para>
+ </listitem>
+ <listitem>
+ <para>
+ <literal>TYPMOD_OUT</literal> can be set to the name of a type
+ modifier output function, or <literal>NONE</literal> to remove the
+ type's type modifier output function. Using this option requires
+ superuser privilege.
+ </para>
+ </listitem>
+ <listitem>
+ <para>
+ <literal>ANALYZE</literal> can be set to the name of a type-specific
+ statistics collection function, or <literal>NONE</literal> to remove
+ the type's statistics collection function. Using this option
+ requires superuser privilege.
+ </para>
+ </listitem>
+ <listitem>
+ <para>
+ <literal>STORAGE</literal><indexterm>
+ <primary>TOAST</primary>
+ <secondary>per-type storage settings</secondary>
+ </indexterm>
+ can be set to <literal>plain</literal>,
+ <literal>extended</literal>, <literal>external</literal>,
+ or <literal>main</literal> (see <xref linkend="storage-toast"/> for
+ more information about what these mean). However, changing
+ between <literal>plain</literal> and other settings is not allowed.
+ Note that changing this option doesn't by itself change any stored
+ data, it just sets the default TOAST strategy to be used for table
+ columns created in the future. See <xref linkend="sql-altertable"/>
+ to change the TOAST strategy for existing table columns.
+ </para>
+ </listitem>
+ </itemizedlist>
+ See <xref linkend="sql-createtype"/> for more details about these
+ type properties. Note that where appropriate, a change in these
+ properties for a base type will be propagated automatically to domains
+ based on that type.
+ </para>
+ </listitem>
+ </varlistentry>
</variablelist>
</para>
@@ -156,7 +231,7 @@ ALTER TYPE <replaceable class="parameter">name</replaceable> RENAME VALUE <repla
doesn't do anything you couldn't do by dropping and recreating the type.
However, a superuser can alter ownership of any type anyway.)
To add an attribute or alter an attribute type, you must also
- have <literal>USAGE</literal> privilege on the data type.
+ have <literal>USAGE</literal> privilege on the attribute's data type.
</para>
</refsect1>
@@ -353,7 +428,20 @@ ALTER TYPE colors ADD VALUE 'orange' AFTER 'red';
To rename an enum value:
<programlisting>
ALTER TYPE colors RENAME VALUE 'purple' TO 'mauve';
-</programlisting></para>
+</programlisting>
+ </para>
+
+ <para>
+ To create binary I/O functions for an existing base type:
+<programlisting>
+CREATE FUNCTION mytypesend(mytype) RETURNS bytea ...;
+CREATE FUNCTION mytyperecv(internal, oid, integer) RETURNS mytype ...;
+ALTER TYPE mytype SET (
+ send = mytypesend,
+ receive = mytyperecv
+);
+</programlisting>
+ </para>
</refsect1>
<refsect1>
diff --git a/src/backend/catalog/pg_type.c b/src/backend/catalog/pg_type.c
index 56e0bcf..cd56714 100644
--- a/src/backend/catalog/pg_type.c
+++ b/src/backend/catalog/pg_type.c
@@ -155,8 +155,8 @@ TypeShellMake(const char *typeName, Oid typeNamespace, Oid ownerId)
* Create dependencies. We can/must skip this in bootstrap mode.
*/
if (!IsBootstrapProcessingMode())
- GenerateTypeDependencies(typoid,
- (Form_pg_type) GETSTRUCT(tup),
+ GenerateTypeDependencies(tup,
+ pg_type_desc,
NULL,
NULL,
0,
@@ -488,8 +488,8 @@ TypeCreate(Oid newTypeOid,
* Create dependencies. We can/must skip this in bootstrap mode.
*/
if (!IsBootstrapProcessingMode())
- GenerateTypeDependencies(typeObjectId,
- (Form_pg_type) GETSTRUCT(tup),
+ GenerateTypeDependencies(tup,
+ pg_type_desc,
(defaultTypeBin ?
stringToNode(defaultTypeBin) :
NULL),
@@ -516,12 +516,16 @@ TypeCreate(Oid newTypeOid,
* GenerateTypeDependencies: build the dependencies needed for a type
*
* Most of what this function needs to know about the type is passed as the
- * new pg_type row, typeForm. But we can't get at the varlena fields through
- * that, so defaultExpr and typacl are passed separately. (typacl is really
+ * new pg_type row, typeTuple. We make callers pass the pg_type Relation
+ * as well, so that we have easy access to a tuple descriptor for the row.
+ *
+ * While this is able to extract the defaultExpr and typacl from the tuple,
+ * doing so is relatively expensive, and callers may have those values at
+ * hand already. Pass those if handy, otherwise pass NULL. (typacl is really
* "Acl *", but we declare it "void *" to avoid including acl.h in pg_type.h.)
*
- * relationKind and isImplicitArray aren't visible in the pg_type row either,
- * so they're also passed separately.
+ * relationKind and isImplicitArray are likewise somewhat expensive to deduce
+ * from the tuple, so we make callers pass those (they're not optional).
*
* isDependentType is true if this is an implicit array or relation rowtype;
* that means it doesn't need its own dependencies on owner etc.
@@ -535,8 +539,8 @@ TypeCreate(Oid newTypeOid,
* that type will become a member of the extension.)
*/
void
-GenerateTypeDependencies(Oid typeObjectId,
- Form_pg_type typeForm,
+GenerateTypeDependencies(HeapTuple typeTuple,
+ Relation typeCatalog,
Node *defaultExpr,
void *typacl,
char relationKind, /* only for relation rowtypes */
@@ -544,9 +548,30 @@ GenerateTypeDependencies(Oid typeObjectId,
bool isDependentType,
bool rebuild)
{
+ Form_pg_type typeForm = (Form_pg_type) GETSTRUCT(typeTuple);
+ Oid typeObjectId = typeForm->oid;
+ Datum datum;
+ bool isNull;
ObjectAddress myself,
referenced;
+ /* Extract defaultExpr if caller didn't pass it */
+ if (defaultExpr == NULL)
+ {
+ datum = heap_getattr(typeTuple, Anum_pg_type_typdefaultbin,
+ RelationGetDescr(typeCatalog), &isNull);
+ if (!isNull)
+ defaultExpr = stringToNode(TextDatumGetCString(datum));
+ }
+ /* Extract typacl if caller didn't pass it */
+ if (typacl == NULL)
+ {
+ datum = heap_getattr(typeTuple, Anum_pg_type_typacl,
+ RelationGetDescr(typeCatalog), &isNull);
+ if (!isNull)
+ typacl = DatumGetAclPCopy(datum);
+ }
+
/* If rebuild, first flush old dependencies, except extension deps */
if (rebuild)
{
diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c
index d732a3a..e282d19 100644
--- a/src/backend/commands/typecmds.c
+++ b/src/backend/commands/typecmds.c
@@ -83,6 +83,25 @@ typedef struct
/* atts[] is of allocated length RelationGetNumberOfAttributes(rel) */
} RelToCheck;
+/* parameter structure for AlterTypeRecurse() */
+typedef struct
+{
+ /* Flags indicating which type attributes to update */
+ bool updateStorage;
+ bool updateReceive;
+ bool updateSend;
+ bool updateTypmodin;
+ bool updateTypmodout;
+ bool updateAnalyze;
+ /* New values for relevant attributes */
+ char storage;
+ Oid receiveOid;
+ Oid sendOid;
+ Oid typmodinOid;
+ Oid typmodoutOid;
+ Oid analyzeOid;
+} AlterTypeRecurseParams;
+
/* Potentially set by pg_upgrade_support functions */
Oid binary_upgrade_next_array_pg_type_oid = InvalidOid;
@@ -107,6 +126,8 @@ static char *domainAddConstraint(Oid domainOid, Oid domainNamespace,
const char *domainName, ObjectAddress *constrAddr);
static Node *replace_domain_constraint_value(ParseState *pstate,
ColumnRef *cref);
+static void AlterTypeRecurse(Oid typeOid, HeapTuple tup, Relation catalog,
+ AlterTypeRecurseParams *atparams);
/*
@@ -466,9 +487,12 @@ DefineType(ParseState *pstate, List *names, List *parameters)
* minimum sane check would be for execute-with-grant-option. But we
* don't have a way to make the type go away if the grant option is
* revoked, so ownership seems better.
+ *
+ * XXX For now, this is all unnecessary given the superuser check above.
+ * If we ever relax that, these calls likely should be moved into
+ * findTypeInputFunction et al, where they could be shared by AlterType.
*/
#ifdef NOT_USED
- /* XXX this is unnecessary given the superuser check above */
if (inputOid && !pg_proc_ownercheck(inputOid, GetUserId()))
aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_FUNCTION,
NameListToString(inputName));
@@ -493,47 +517,6 @@ DefineType(ParseState *pstate, List *names, List *parameters)
#endif
/*
- * Print warnings if any of the type's I/O functions are marked volatile.
- * There is a general assumption that I/O functions are stable or
- * immutable; this allows us for example to mark record_in/record_out
- * stable rather than volatile. Ideally we would throw errors not just
- * warnings here; but since this check is new as of 9.5, and since the
- * volatility marking might be just an error-of-omission and not a true
- * indication of how the function behaves, we'll let it pass as a warning
- * for now.
- */
- if (inputOid && func_volatile(inputOid) == PROVOLATILE_VOLATILE)
- ereport(WARNING,
- (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
- errmsg("type input function %s should not be volatile",
- NameListToString(inputName))));
- if (outputOid && func_volatile(outputOid) == PROVOLATILE_VOLATILE)
- ereport(WARNING,
- (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
- errmsg("type output function %s should not be volatile",
- NameListToString(outputName))));
- if (receiveOid && func_volatile(receiveOid) == PROVOLATILE_VOLATILE)
- ereport(WARNING,
- (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
- errmsg("type receive function %s should not be volatile",
- NameListToString(receiveName))));
- if (sendOid && func_volatile(sendOid) == PROVOLATILE_VOLATILE)
- ereport(WARNING,
- (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
- errmsg("type send function %s should not be volatile",
- NameListToString(sendName))));
- if (typmodinOid && func_volatile(typmodinOid) == PROVOLATILE_VOLATILE)
- ereport(WARNING,
- (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
- errmsg("type modifier input function %s should not be volatile",
- NameListToString(typmodinName))));
- if (typmodoutOid && func_volatile(typmodoutOid) == PROVOLATILE_VOLATILE)
- ereport(WARNING,
- (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
- errmsg("type modifier output function %s should not be volatile",
- NameListToString(typmodoutName))));
-
- /*
* OK, we're done checking, time to make the type. We must assign the
* array type OID ahead of calling TypeCreate, since the base type and
* array type each refer to the other.
@@ -765,6 +748,12 @@ DefineDomain(CreateDomainStmt *stmt)
aclcheck_error_type(aclresult, basetypeoid);
/*
+ * Collect the properties of the new domain. Some are inherited from the
+ * base type, some are not. If you change any of this inheritance
+ * behavior, be sure to update AlterTypeRecurse() to match!
+ */
+
+ /*
* Identify the collation if any
*/
baseColl = baseType->typcollation;
@@ -1664,6 +1653,22 @@ findTypeInputFunction(List *procname, Oid typeOid)
errmsg("type input function %s must return type %s",
NameListToString(procname), format_type_be(typeOid))));
+ /*
+ * Print warnings if any of the type's I/O functions are marked volatile.
+ * There is a general assumption that I/O functions are stable or
+ * immutable; this allows us for example to mark record_in/record_out
+ * stable rather than volatile. Ideally we would throw errors not just
+ * warnings here; but since this check is new as of 9.5, and since the
+ * volatility marking might be just an error-of-omission and not a true
+ * indication of how the function behaves, we'll let it pass as a warning
+ * for now.
+ */
+ if (func_volatile(procOid) == PROVOLATILE_VOLATILE)
+ ereport(WARNING,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("type input function %s should not be volatile",
+ NameListToString(procname))));
+
return procOid;
}
@@ -1692,6 +1697,13 @@ findTypeOutputFunction(List *procname, Oid typeOid)
errmsg("type output function %s must return type %s",
NameListToString(procname), "cstring")));
+ /* Just a warning for now, per comments in findTypeInputFunction */
+ if (func_volatile(procOid) == PROVOLATILE_VOLATILE)
+ ereport(WARNING,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("type output function %s should not be volatile",
+ NameListToString(procname))));
+
return procOid;
}
@@ -1728,6 +1740,13 @@ findTypeReceiveFunction(List *procname, Oid typeOid)
errmsg("type receive function %s must return type %s",
NameListToString(procname), format_type_be(typeOid))));
+ /* Just a warning for now, per comments in findTypeInputFunction */
+ if (func_volatile(procOid) == PROVOLATILE_VOLATILE)
+ ereport(WARNING,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("type receive function %s should not be volatile",
+ NameListToString(procname))));
+
return procOid;
}
@@ -1756,6 +1775,13 @@ findTypeSendFunction(List *procname, Oid typeOid)
errmsg("type send function %s must return type %s",
NameListToString(procname), "bytea")));
+ /* Just a warning for now, per comments in findTypeInputFunction */
+ if (func_volatile(procOid) == PROVOLATILE_VOLATILE)
+ ereport(WARNING,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("type send function %s should not be volatile",
+ NameListToString(procname))));
+
return procOid;
}
@@ -1783,6 +1809,13 @@ findTypeTypmodinFunction(List *procname)
errmsg("typmod_in function %s must return type %s",
NameListToString(procname), "integer")));
+ /* Just a warning for now, per comments in findTypeInputFunction */
+ if (func_volatile(procOid) == PROVOLATILE_VOLATILE)
+ ereport(WARNING,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("type modifier input function %s should not be volatile",
+ NameListToString(procname))));
+
return procOid;
}
@@ -1810,6 +1843,13 @@ findTypeTypmodoutFunction(List *procname)
errmsg("typmod_out function %s must return type %s",
NameListToString(procname), "cstring")));
+ /* Just a warning for now, per comments in findTypeInputFunction */
+ if (func_volatile(procOid) == PROVOLATILE_VOLATILE)
+ ereport(WARNING,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("type modifier output function %s should not be volatile",
+ NameListToString(procname))));
+
return procOid;
}
@@ -2086,9 +2126,6 @@ AlterDomainDefault(List *names, Node *defaultRaw)
Relation rel;
char *defaultValue;
Node *defaultExpr = NULL; /* NULL if no default specified */
- Acl *typacl;
- Datum aclDatum;
- bool isNull;
Datum new_record[Natts_pg_type];
bool new_record_nulls[Natts_pg_type];
bool new_record_repl[Natts_pg_type];
@@ -2141,6 +2178,7 @@ AlterDomainDefault(List *names, Node *defaultRaw)
(IsA(defaultExpr, Const) &&((Const *) defaultExpr)->constisnull))
{
/* Default is NULL, drop it */
+ defaultExpr = NULL;
new_record_nulls[Anum_pg_type_typdefaultbin - 1] = true;
new_record_repl[Anum_pg_type_typdefaultbin - 1] = true;
new_record_nulls[Anum_pg_type_typdefault - 1] = true;
@@ -2181,19 +2219,11 @@ AlterDomainDefault(List *names, Node *defaultRaw)
CatalogTupleUpdate(rel, &tup->t_self, newtuple);
- /* Must extract ACL for use of GenerateTypeDependencies */
- aclDatum = heap_getattr(newtuple, Anum_pg_type_typacl,
- RelationGetDescr(rel), &isNull);
- if (isNull)
- typacl = NULL;
- else
- typacl = DatumGetAclPCopy(aclDatum);
-
/* Rebuild dependencies */
- GenerateTypeDependencies(domainoid,
- (Form_pg_type) GETSTRUCT(newtuple),
+ GenerateTypeDependencies(newtuple,
+ rel,
defaultExpr,
- typacl,
+ NULL, /* don't have typacl handy */
0, /* relation kind is n/a */
false, /* a domain isn't an implicit array */
false, /* nor is it any kind of dependent type */
@@ -3609,3 +3639,358 @@ AlterTypeNamespaceInternal(Oid typeOid, Oid nspOid,
return oldNspOid;
}
+
+/*
+ * AlterType
+ * ALTER TYPE <type> SET (option = ...)
+ *
+ * NOTE: the set of changes that can be allowed here is constrained by many
+ * non-obvious implementation restrictions. Tread carefully when considering
+ * adding new flexibility. One place to look at is whether typcache.c caches
+ * a particular type property.
+ */
+ObjectAddress
+AlterType(AlterTypeStmt *stmt)
+{
+ ObjectAddress address;
+ Relation catalog;
+ TypeName *typename;
+ HeapTuple tup;
+ Oid typeOid;
+ Form_pg_type typForm;
+ bool requireSuper = false;
+ AlterTypeRecurseParams atparams;
+ ListCell *pl;
+
+ catalog = table_open(TypeRelationId, RowExclusiveLock);
+
+ /* Make a TypeName so we can use standard type lookup machinery */
+ typename = makeTypeNameFromNameList(stmt->typeName);
+ tup = typenameType(NULL, typename, NULL);
+
+ typeOid = typeTypeId(tup);
+ typForm = (Form_pg_type) GETSTRUCT(tup);
+
+ /* Process options */
+ memset(&atparams, 0, sizeof(atparams));
+ foreach(pl, stmt->options)
+ {
+ DefElem *defel = (DefElem *) lfirst(pl);
+
+ if (strcmp(defel->defname, "storage") == 0)
+ {
+ char *a = defGetString(defel);
+
+ if (pg_strcasecmp(a, "plain") == 0)
+ atparams.storage = TYPSTORAGE_PLAIN;
+ else if (pg_strcasecmp(a, "external") == 0)
+ atparams.storage = TYPSTORAGE_EXTERNAL;
+ else if (pg_strcasecmp(a, "extended") == 0)
+ atparams.storage = TYPSTORAGE_EXTENDED;
+ else if (pg_strcasecmp(a, "main") == 0)
+ atparams.storage = TYPSTORAGE_MAIN;
+ else
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("storage \"%s\" not recognized", a)));
+
+ /*
+ * Validate the storage request. If the type isn't varlena, it
+ * certainly doesn't support non-PLAIN storage.
+ */
+ if (atparams.storage != TYPSTORAGE_PLAIN && typForm->typlen != -1)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("fixed-size types must have storage PLAIN")));
+
+ /*
+ * We disallow switching between PLAIN and non-PLAIN typstorage,
+ * because that implies changing whether the type can be toasted
+ * at all, which is problematic for a number of reasons. Switching
+ * from PLAIN to non-PLAIN could perhaps be allowed to superusers,
+ * who would then be responsible for ensuring that all the type's
+ * C functions are TOAST-ready; but we'd need fixes in the
+ * typcache and perhaps other places. Switching from non-PLAIN to
+ * PLAIN seems impractical because of the risk that toasted values
+ * exist (and possibly more are getting created by concurrent
+ * transactions). But switching among different non-PLAIN
+ * settings is OK, since it just constitutes a change in the
+ * strategy requested for columns created in the future.
+ */
+ if (atparams.storage != TYPSTORAGE_PLAIN &&
+ typForm->typstorage == TYPSTORAGE_PLAIN)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("cannot change type's storage from PLAIN")));
+ else if (atparams.storage == TYPSTORAGE_PLAIN &&
+ typForm->typstorage != TYPSTORAGE_PLAIN)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("cannot change type's storage to PLAIN")));
+
+ atparams.updateStorage = true;
+ }
+ else if (strcmp(defel->defname, "receive") == 0)
+ {
+ if (defel->arg != NULL)
+ atparams.receiveOid =
+ findTypeReceiveFunction(defGetQualifiedName(defel),
+ typeOid);
+ else
+ atparams.receiveOid = InvalidOid; /* NONE, remove function */
+ atparams.updateReceive = true;
+ /* Replacing an I/O function requires superuser. */
+ requireSuper = true;
+ }
+ else if (strcmp(defel->defname, "send") == 0)
+ {
+ if (defel->arg != NULL)
+ atparams.sendOid =
+ findTypeSendFunction(defGetQualifiedName(defel),
+ typeOid);
+ else
+ atparams.sendOid = InvalidOid; /* NONE, remove function */
+ atparams.updateSend = true;
+ /* Replacing an I/O function requires superuser. */
+ requireSuper = true;
+ }
+ else if (strcmp(defel->defname, "typmod_in") == 0)
+ {
+ if (defel->arg != NULL)
+ atparams.typmodinOid =
+ findTypeTypmodinFunction(defGetQualifiedName(defel));
+ else
+ atparams.typmodinOid = InvalidOid; /* NONE, remove function */
+ atparams.updateTypmodin = true;
+ /* Replacing an I/O function requires superuser. */
+ requireSuper = true;
+ }
+ else if (strcmp(defel->defname, "typmod_out") == 0)
+ {
+ if (defel->arg != NULL)
+ atparams.typmodoutOid =
+ findTypeTypmodoutFunction(defGetQualifiedName(defel));
+ else
+ atparams.typmodoutOid = InvalidOid; /* NONE, remove function */
+ atparams.updateTypmodout = true;
+ /* Replacing an I/O function requires superuser. */
+ requireSuper = true;
+ }
+ else if (strcmp(defel->defname, "analyze") == 0)
+ {
+ if (defel->arg != NULL)
+ atparams.analyzeOid =
+ findTypeAnalyzeFunction(defGetQualifiedName(defel),
+ typeOid);
+ else
+ atparams.analyzeOid = InvalidOid; /* NONE, remove function */
+ atparams.updateAnalyze = true;
+ /* Replacing an analyze function requires superuser. */
+ requireSuper = true;
+ }
+
+ /*
+ * The rest of the options that CREATE accepts cannot be changed.
+ * Check for them so that we can give a meaningful error message.
+ */
+ else if (strcmp(defel->defname, "input") == 0 ||
+ strcmp(defel->defname, "output") == 0 ||
+ strcmp(defel->defname, "internallength") == 0 ||
+ strcmp(defel->defname, "passedbyvalue") == 0 ||
+ strcmp(defel->defname, "alignment") == 0 ||
+ strcmp(defel->defname, "like") == 0 ||
+ strcmp(defel->defname, "category") == 0 ||
+ strcmp(defel->defname, "preferred") == 0 ||
+ strcmp(defel->defname, "default") == 0 ||
+ strcmp(defel->defname, "element") == 0 ||
+ strcmp(defel->defname, "delimiter") == 0 ||
+ strcmp(defel->defname, "collatable") == 0)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("type attribute \"%s\" cannot be changed",
+ defel->defname)));
+ else
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("type attribute \"%s\" not recognized",
+ defel->defname)));
+ }
+
+ /*
+ * Permissions check. Require superuser if we decided the command
+ * requires that, else must own the type.
+ */
+ if (requireSuper)
+ {
+ if (!superuser())
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("must be superuser to alter a type")));
+ }
+ else
+ {
+ if (!pg_type_ownercheck(typeOid, GetUserId()))
+ aclcheck_error_type(ACLCHECK_NOT_OWNER, typeOid);
+ }
+
+ /*
+ * We disallow all forms of ALTER TYPE SET on types that aren't plain base
+ * types. It would for example be highly unsafe, not to mention
+ * pointless, to change the send/receive functions for a composite type.
+ * Moreover, pg_dump has no support for changing these properties on
+ * non-base types. We might weaken this someday, but not now.
+ *
+ * Note: if you weaken this enough to allow composite types, be sure to
+ * adjust the GenerateTypeDependencies call in AlterTypeRecurse.
+ */
+ if (typForm->typtype != TYPTYPE_BASE)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("%s is not a base type",
+ format_type_be(typeOid))));
+
+ /*
+ * For the same reasons, don't allow direct alteration of array types.
+ */
+ if (OidIsValid(typForm->typelem) &&
+ get_array_type(typForm->typelem) == typeOid)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("%s is not a base type",
+ format_type_be(typeOid))));
+
+ /* OK, recursively update this type and any domains over it */
+ AlterTypeRecurse(typeOid, tup, catalog, &atparams);
+
+ /* Clean up */
+ ReleaseSysCache(tup);
+
+ table_close(catalog, RowExclusiveLock);
+
+ ObjectAddressSet(address, TypeRelationId, typeOid);
+
+ return address;
+}
+
+/*
+ * AlterTypeRecurse: one recursion step for AlterType()
+ *
+ * Apply the changes specified by "atparams" to the type identified by
+ * "typeOid", whose existing pg_type tuple is "tup". Then search for any
+ * domains over this type, and recursively apply (most of) the same changes
+ * to those domains.
+ *
+ * We need this because the system generally assumes that a domain inherits
+ * many properties from its base type. See DefineDomain() above for details
+ * of what is inherited.
+ *
+ * There's a race condition here, in that some other transaction could
+ * concurrently add another domain atop this base type; we'd miss updating
+ * that one. Hence, be wary of allowing ALTER TYPE to change properties for
+ * which it'd be really fatal for a domain to be out of sync with its base
+ * type (typlen, for example). In practice, races seem unlikely to be an
+ * issue for plausible use-cases for ALTER TYPE. If one does happen, it could
+ * be fixed by re-doing the same ALTER TYPE once all prior transactions have
+ * committed.
+ */
+static void
+AlterTypeRecurse(Oid typeOid, HeapTuple tup, Relation catalog,
+ AlterTypeRecurseParams *atparams)
+{
+ Datum values[Natts_pg_type];
+ bool nulls[Natts_pg_type];
+ bool replaces[Natts_pg_type];
+ HeapTuple newtup;
+ SysScanDesc scan;
+ ScanKeyData key[1];
+ HeapTuple domainTup;
+
+ /* Since this function recurses, it could be driven to stack overflow */
+ check_stack_depth();
+
+ /* Update the current type's tuple */
+ memset(values, 0, sizeof(values));
+ memset(nulls, 0, sizeof(nulls));
+ memset(replaces, 0, sizeof(replaces));
+
+ if (atparams->updateStorage)
+ {
+ replaces[Anum_pg_type_typstorage - 1] = true;
+ values[Anum_pg_type_typstorage - 1] = CharGetDatum(atparams->storage);
+ }
+ if (atparams->updateReceive)
+ {
+ replaces[Anum_pg_type_typreceive - 1] = true;
+ values[Anum_pg_type_typreceive - 1] = ObjectIdGetDatum(atparams->receiveOid);
+ }
+ if (atparams->updateSend)
+ {
+ replaces[Anum_pg_type_typsend - 1] = true;
+ values[Anum_pg_type_typsend - 1] = ObjectIdGetDatum(atparams->sendOid);
+ }
+ if (atparams->updateTypmodin)
+ {
+ replaces[Anum_pg_type_typmodin - 1] = true;
+ values[Anum_pg_type_typmodin - 1] = ObjectIdGetDatum(atparams->typmodinOid);
+ }
+ if (atparams->updateTypmodout)
+ {
+ replaces[Anum_pg_type_typmodout - 1] = true;
+ values[Anum_pg_type_typmodout - 1] = ObjectIdGetDatum(atparams->typmodoutOid);
+ }
+ if (atparams->updateAnalyze)
+ {
+ replaces[Anum_pg_type_typanalyze - 1] = true;
+ values[Anum_pg_type_typanalyze - 1] = ObjectIdGetDatum(atparams->analyzeOid);
+ }
+
+ newtup = heap_modify_tuple(tup, RelationGetDescr(catalog),
+ values, nulls, replaces);
+
+ CatalogTupleUpdate(catalog, &newtup->t_self, newtup);
+
+ /* Rebuild dependencies for this type */
+ GenerateTypeDependencies(newtup,
+ catalog,
+ NULL, /* don't have defaultExpr handy */
+ NULL, /* don't have typacl handy */
+ 0, /* we rejected composite types above */
+ false, /* and we rejected implicit arrays above */
+ false, /* so it can't be a dependent type */
+ true);
+
+ InvokeObjectPostAlterHook(TypeRelationId, typeOid, 0);
+
+ /*
+ * Now we need to recurse to domains. However, some properties are not
+ * inherited by domains, so clear the update flags for those.
+ */
+ atparams->updateReceive = false; /* domains use F_DOMAIN_RECV */
+ atparams->updateTypmodin = false; /* domains don't have typmods */
+ atparams->updateTypmodout = false;
+
+ /* Search pg_type for possible domains over this type */
+ ScanKeyInit(&key[0],
+ Anum_pg_type_typbasetype,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(typeOid));
+
+ scan = systable_beginscan(catalog, InvalidOid, false,
+ NULL, 1, key);
+
+ while ((domainTup = systable_getnext(scan)) != NULL)
+ {
+ Form_pg_type domainForm = (Form_pg_type) GETSTRUCT(domainTup);
+
+ /*
+ * Shouldn't have a nonzero typbasetype in a non-domain, but let's
+ * check
+ */
+ if (domainForm->typtype != TYPTYPE_DOMAIN)
+ continue;
+
+ AlterTypeRecurse(domainForm->oid, domainTup, catalog, atparams);
+ }
+
+ systable_endscan(scan);
+}
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index e04c33e..eaab97f 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -3636,6 +3636,17 @@ _copyAlterOperatorStmt(const AlterOperatorStmt *from)
return newnode;
}
+static AlterTypeStmt *
+_copyAlterTypeStmt(const AlterTypeStmt *from)
+{
+ AlterTypeStmt *newnode = makeNode(AlterTypeStmt);
+
+ COPY_NODE_FIELD(typeName);
+ COPY_NODE_FIELD(options);
+
+ return newnode;
+}
+
static RuleStmt *
_copyRuleStmt(const RuleStmt *from)
{
@@ -5263,6 +5274,9 @@ copyObjectImpl(const void *from)
case T_AlterOperatorStmt:
retval = _copyAlterOperatorStmt(from);
break;
+ case T_AlterTypeStmt:
+ retval = _copyAlterTypeStmt(from);
+ break;
case T_RuleStmt:
retval = _copyRuleStmt(from);
break;
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 5b1ba14..88b9129 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -1482,6 +1482,15 @@ _equalAlterOperatorStmt(const AlterOperatorStmt *a, const AlterOperatorStmt *b)
}
static bool
+_equalAlterTypeStmt(const AlterTypeStmt *a, const AlterTypeStmt *b)
+{
+ COMPARE_NODE_FIELD(typeName);
+ COMPARE_NODE_FIELD(options);
+
+ return true;
+}
+
+static bool
_equalRuleStmt(const RuleStmt *a, const RuleStmt *b)
{
COMPARE_NODE_FIELD(relation);
@@ -3359,6 +3368,9 @@ equal(const void *a, const void *b)
case T_AlterOperatorStmt:
retval = _equalAlterOperatorStmt(a, b);
break;
+ case T_AlterTypeStmt:
+ retval = _equalAlterTypeStmt(a, b);
+ break;
case T_RuleStmt:
retval = _equalRuleStmt(a, b);
break;
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 96e7fdb..7e384f9 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -249,7 +249,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
AlterDatabaseStmt AlterDatabaseSetStmt AlterDomainStmt AlterEnumStmt
AlterFdwStmt AlterForeignServerStmt AlterGroupStmt
AlterObjectDependsStmt AlterObjectSchemaStmt AlterOwnerStmt
- AlterOperatorStmt AlterSeqStmt AlterSystemStmt AlterTableStmt
+ AlterOperatorStmt AlterTypeStmt AlterSeqStmt AlterSystemStmt AlterTableStmt
AlterTblSpcStmt AlterExtensionStmt AlterExtensionContentsStmt AlterForeignTableStmt
AlterCompositeTypeStmt AlterUserMappingStmt
AlterRoleStmt AlterRoleSetStmt AlterPolicyStmt AlterStatsStmt
@@ -847,6 +847,7 @@ stmt :
| AlterObjectSchemaStmt
| AlterOwnerStmt
| AlterOperatorStmt
+ | AlterTypeStmt
| AlterPolicyStmt
| AlterSeqStmt
| AlterSystemStmt
@@ -9367,6 +9368,24 @@ operator_def_arg:
/*****************************************************************************
*
+ * ALTER TYPE name SET define
+ *
+ * We repurpose ALTER OPERATOR's version of "definition" here
+ *
+ *****************************************************************************/
+
+AlterTypeStmt:
+ ALTER TYPE_P any_name SET '(' operator_def_list ')'
+ {
+ AlterTypeStmt *n = makeNode(AlterTypeStmt);
+ n->typeName = $3;
+ n->options = $6;
+ $$ = (Node *)n;
+ }
+ ;
+
+/*****************************************************************************
+ *
* ALTER THING name OWNER TO newname
*
*****************************************************************************/
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 1b460a2..b1f7f6e 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -162,6 +162,7 @@ ClassifyUtilityCommandAsReadOnly(Node *parsetree)
case T_AlterTableMoveAllStmt:
case T_AlterTableSpaceOptionsStmt:
case T_AlterTableStmt:
+ case T_AlterTypeStmt:
case T_AlterUserMappingStmt:
case T_CommentStmt:
case T_CompositeTypeStmt:
@@ -1713,6 +1714,10 @@ ProcessUtilitySlow(ParseState *pstate,
address = AlterOperator((AlterOperatorStmt *) parsetree);
break;
+ case T_AlterTypeStmt:
+ address = AlterType((AlterTypeStmt *) parsetree);
+ break;
+
case T_CommentStmt:
address = CommentObject((CommentStmt *) parsetree);
break;
@@ -2895,6 +2900,10 @@ CreateCommandTag(Node *parsetree)
tag = CMDTAG_ALTER_OPERATOR;
break;
+ case T_AlterTypeStmt:
+ tag = CMDTAG_ALTER_TYPE;
+ break;
+
case T_AlterTSDictionaryStmt:
tag = CMDTAG_ALTER_TEXT_SEARCH_DICTIONARY;
break;
@@ -3251,6 +3260,10 @@ GetCommandLogLevel(Node *parsetree)
lev = LOGSTMT_DDL;
break;
+ case T_AlterTypeStmt:
+ lev = LOGSTMT_DDL;
+ break;
+
case T_AlterTableMoveAllStmt:
case T_AlterTableStmt:
lev = LOGSTMT_DDL;
diff --git a/src/backend/utils/adt/rangetypes.c b/src/backend/utils/adt/rangetypes.c
index 490bc2a..e34daaa 100644
--- a/src/backend/utils/adt/rangetypes.c
+++ b/src/backend/utils/adt/rangetypes.c
@@ -66,9 +66,9 @@ static char *range_deparse(char flags, const char *lbound_str,
const char *ubound_str);
static char *range_bound_escape(const char *value);
static Size datum_compute_size(Size sz, Datum datum, bool typbyval,
- char typalign, int16 typlen, char typstorage);
+ char typalign, int16 typlen, bool type_is_packable);
static Pointer datum_write(Pointer ptr, Datum datum, bool typbyval,
- char typalign, int16 typlen, char typstorage);
+ char typalign, int16 typlen, bool type_is_packable);
/*
@@ -1577,7 +1577,7 @@ range_serialize(TypeCacheEntry *typcache, RangeBound *lower, RangeBound *upper,
int16 typlen;
bool typbyval;
char typalign;
- char typstorage;
+ bool type_is_packable;
char flags = 0;
/*
@@ -1620,7 +1620,7 @@ range_serialize(TypeCacheEntry *typcache, RangeBound *lower, RangeBound *upper,
typlen = typcache->rngelemtype->typlen;
typbyval = typcache->rngelemtype->typbyval;
typalign = typcache->rngelemtype->typalign;
- typstorage = typcache->rngelemtype->typstorage;
+ type_is_packable = typcache->rngelemtype->type_is_packable;
/* Count space for varlena header and range type's OID */
msize = sizeof(RangeType);
@@ -1642,7 +1642,7 @@ range_serialize(TypeCacheEntry *typcache, RangeBound *lower, RangeBound *upper,
lower->val = PointerGetDatum(PG_DETOAST_DATUM_PACKED(lower->val));
msize = datum_compute_size(msize, lower->val, typbyval, typalign,
- typlen, typstorage);
+ typlen, type_is_packable);
}
if (RANGE_HAS_UBOUND(flags))
@@ -1652,7 +1652,7 @@ range_serialize(TypeCacheEntry *typcache, RangeBound *lower, RangeBound *upper,
upper->val = PointerGetDatum(PG_DETOAST_DATUM_PACKED(upper->val));
msize = datum_compute_size(msize, upper->val, typbyval, typalign,
- typlen, typstorage);
+ typlen, type_is_packable);
}
/* Add space for flag byte */
@@ -1671,14 +1671,14 @@ range_serialize(TypeCacheEntry *typcache, RangeBound *lower, RangeBound *upper,
{
Assert(lower->lower);
ptr = datum_write(ptr, lower->val, typbyval, typalign, typlen,
- typstorage);
+ type_is_packable);
}
if (RANGE_HAS_UBOUND(flags))
{
Assert(!upper->lower);
ptr = datum_write(ptr, upper->val, typbyval, typalign, typlen,
- typstorage);
+ type_is_packable);
}
*((char *) ptr) = flags;
@@ -2383,24 +2383,18 @@ range_contains_elem_internal(TypeCacheEntry *typcache, const RangeType *r, Datum
* datum_compute_size() and datum_write() are used to insert the bound
* values into a range object. They are modeled after heaptuple.c's
* heap_compute_data_size() and heap_fill_tuple(), but we need not handle
- * null values here. TYPE_IS_PACKABLE must test the same conditions as
- * heaptuple.c's ATT_IS_PACKABLE macro.
+ * null values here.
*/
-/* Does datatype allow packing into the 1-byte-header varlena format? */
-#define TYPE_IS_PACKABLE(typlen, typstorage) \
- ((typlen) == -1 && (typstorage) != TYPSTORAGE_PLAIN)
-
/*
* Increment data_length by the space needed by the datum, including any
* preceding alignment padding.
*/
static Size
datum_compute_size(Size data_length, Datum val, bool typbyval, char typalign,
- int16 typlen, char typstorage)
+ int16 typlen, bool type_is_packable)
{
- if (TYPE_IS_PACKABLE(typlen, typstorage) &&
- VARATT_CAN_MAKE_SHORT(DatumGetPointer(val)))
+ if (type_is_packable && VARATT_CAN_MAKE_SHORT(DatumGetPointer(val)))
{
/*
* we're anticipating converting to a short varlena header, so adjust
@@ -2423,7 +2417,7 @@ datum_compute_size(Size data_length, Datum val, bool typbyval, char typalign,
*/
static Pointer
datum_write(Pointer ptr, Datum datum, bool typbyval, char typalign,
- int16 typlen, char typstorage)
+ int16 typlen, bool type_is_packable)
{
Size data_length;
@@ -2454,8 +2448,7 @@ datum_write(Pointer ptr, Datum datum, bool typbyval, char typalign,
data_length = VARSIZE_SHORT(val);
memcpy(ptr, val, data_length);
}
- else if (TYPE_IS_PACKABLE(typlen, typstorage) &&
- VARATT_CAN_MAKE_SHORT(val))
+ else if (type_is_packable && VARATT_CAN_MAKE_SHORT(val))
{
/* convert to short varlena -- no alignment */
data_length = VARATT_CONVERTED_SHORT_SIZE(val);
diff --git a/src/backend/utils/cache/typcache.c b/src/backend/utils/cache/typcache.c
index cdf6331..88913ca 100644
--- a/src/backend/utils/cache/typcache.c
+++ b/src/backend/utils/cache/typcache.c
@@ -382,11 +382,21 @@ lookup_type_cache(Oid type_id, int flags)
MemSet(typentry, 0, sizeof(TypeCacheEntry));
typentry->type_id = type_id;
+
+ /*
+ * Caution: there's no provision for flushing/rebuilding these basic
+ * cache fields; they'll remain for the life of the backend. Hence,
+ * do not cache anything here that ALTER TYPE can change. (Although
+ * we allow ALTER TYPE to change typstorage, it can't change it
+ * between PLAIN and not-PLAIN, so caching the packable flag is safe.)
+ */
typentry->typlen = typtup->typlen;
typentry->typbyval = typtup->typbyval;
typentry->typalign = typtup->typalign;
- typentry->typstorage = typtup->typstorage;
typentry->typtype = typtup->typtype;
+ /* This test must match heaptuple.c's ATT_IS_PACKABLE macro */
+ typentry->type_is_packable = (typtup->typlen == -1 &&
+ typtup->typstorage != TYPSTORAGE_PLAIN);
typentry->typrelid = typtup->typrelid;
typentry->typelem = typtup->typelem;
typentry->typcollation = typtup->typcollation;
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index b6b08d0..54d0317 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -2150,7 +2150,7 @@ psql_completion(const char *text, int start, int end)
else if (Matches("ALTER", "TYPE", MatchAny))
COMPLETE_WITH("ADD ATTRIBUTE", "ADD VALUE", "ALTER ATTRIBUTE",
"DROP ATTRIBUTE",
- "OWNER TO", "RENAME", "SET SCHEMA");
+ "OWNER TO", "RENAME", "SET SCHEMA", "SET (");
/* complete ALTER TYPE <foo> ADD with actions */
else if (Matches("ALTER", "TYPE", MatchAny, "ADD"))
COMPLETE_WITH("ATTRIBUTE", "VALUE");
diff --git a/src/include/catalog/pg_type.h b/src/include/catalog/pg_type.h
index f972f94..9789094 100644
--- a/src/include/catalog/pg_type.h
+++ b/src/include/catalog/pg_type.h
@@ -340,8 +340,8 @@ extern ObjectAddress TypeCreate(Oid newTypeOid,
bool typeNotNull,
Oid typeCollation);
-extern void GenerateTypeDependencies(Oid typeObjectId,
- Form_pg_type typeForm,
+extern void GenerateTypeDependencies(HeapTuple typeTuple,
+ Relation typeCatalog,
Node *defaultExpr,
void *typacl,
char relationKind, /* only for relation
diff --git a/src/include/commands/typecmds.h b/src/include/commands/typecmds.h
index fc18d64..0162bc2 100644
--- a/src/include/commands/typecmds.h
+++ b/src/include/commands/typecmds.h
@@ -54,4 +54,6 @@ extern Oid AlterTypeNamespaceInternal(Oid typeOid, Oid nspOid,
bool errorOnTableType,
ObjectAddresses *objsMoved);
+extern ObjectAddress AlterType(AlterTypeStmt *stmt);
+
#endif /* TYPECMDS_H */
diff --git a/src/include/nodes/nodes.h b/src/include/nodes/nodes.h
index baced7e..8a76afe 100644
--- a/src/include/nodes/nodes.h
+++ b/src/include/nodes/nodes.h
@@ -380,6 +380,7 @@ typedef enum NodeTag
T_AlterObjectSchemaStmt,
T_AlterOwnerStmt,
T_AlterOperatorStmt,
+ T_AlterTypeStmt,
T_DropOwnedStmt,
T_ReassignOwnedStmt,
T_CompositeTypeStmt,
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index da0706a..2039b42 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -2959,9 +2959,8 @@ typedef struct AlterOwnerStmt
RoleSpec *newowner; /* the new owner */
} AlterOwnerStmt;
-
/* ----------------------
- * Alter Operator Set Restrict, Join
+ * Alter Operator Set ( this-n-that )
* ----------------------
*/
typedef struct AlterOperatorStmt
@@ -2971,6 +2970,16 @@ typedef struct AlterOperatorStmt
List *options; /* List of DefElem nodes */
} AlterOperatorStmt;
+/* ------------------------
+ * Alter Type Set ( this-n-that )
+ * ------------------------
+ */
+typedef struct AlterTypeStmt
+{
+ NodeTag type;
+ List *typeName; /* type name (possibly qualified) */
+ List *options; /* List of DefElem nodes */
+} AlterTypeStmt;
/* ----------------------
* Create Rule Statement
diff --git a/src/include/utils/typcache.h b/src/include/utils/typcache.h
index 66ff17d..77b9a90 100644
--- a/src/include/utils/typcache.h
+++ b/src/include/utils/typcache.h
@@ -33,12 +33,16 @@ typedef struct TypeCacheEntry
/* typeId is the hash lookup key and MUST BE FIRST */
Oid type_id; /* OID of the data type */
- /* some subsidiary information copied from the pg_type row */
+ /*
+ * Some subsidiary information copied from the pg_type row. Caution: do
+ * not cache anything here that ALTER TYPE can change, since there's no
+ * provision for flushing this part of the cache.
+ */
int16 typlen;
bool typbyval;
char typalign;
- char typstorage;
char typtype;
+ bool type_is_packable;
Oid typrelid;
Oid typelem;
Oid typcollation;
diff --git a/src/test/regress/expected/create_type.out b/src/test/regress/expected/create_type.out
index eb55e25..86a8b65 100644
--- a/src/test/regress/expected/create_type.out
+++ b/src/test/regress/expected/create_type.out
@@ -224,3 +224,81 @@ select format_type('bpchar'::regtype, -1);
bpchar
(1 row)
+--
+-- Test CREATE/ALTER TYPE using a type that's compatible with varchar,
+-- so we can re-use those support functions
+--
+CREATE TYPE myvarchar;
+CREATE FUNCTION myvarcharin(cstring, oid, integer) RETURNS myvarchar
+LANGUAGE internal IMMUTABLE PARALLEL SAFE STRICT AS 'varcharin';
+NOTICE: return type myvarchar is only a shell
+CREATE FUNCTION myvarcharout(myvarchar) RETURNS cstring
+LANGUAGE internal IMMUTABLE PARALLEL SAFE STRICT AS 'varcharout';
+NOTICE: argument type myvarchar is only a shell
+CREATE FUNCTION myvarcharsend(myvarchar) RETURNS bytea
+LANGUAGE internal STABLE PARALLEL SAFE STRICT AS 'varcharsend';
+NOTICE: argument type myvarchar is only a shell
+CREATE FUNCTION myvarcharrecv(internal, oid, integer) RETURNS myvarchar
+LANGUAGE internal STABLE PARALLEL SAFE STRICT AS 'varcharrecv';
+NOTICE: return type myvarchar is only a shell
+-- fail, it's still a shell:
+ALTER TYPE myvarchar SET (storage = extended);
+ERROR: type "myvarchar" is only a shell
+CREATE TYPE myvarchar (
+ input = myvarcharin,
+ output = myvarcharout,
+ alignment = integer,
+ storage = main
+);
+-- want to check updating of a domain over the target type, too
+CREATE DOMAIN myvarchardom AS myvarchar;
+ALTER TYPE myvarchar SET (storage = plain); -- not allowed
+ERROR: cannot change type's storage to PLAIN
+ALTER TYPE myvarchar SET (storage = extended);
+ALTER TYPE myvarchar SET (
+ send = myvarcharsend,
+ receive = myvarcharrecv,
+ typmod_in = varchartypmodin,
+ typmod_out = varchartypmodout,
+ analyze = array_typanalyze -- bogus, but it doesn't matter
+);
+SELECT typinput, typoutput, typreceive, typsend, typmodin, typmodout,
+ typanalyze, typstorage
+FROM pg_type WHERE typname = 'myvarchar';
+ typinput | typoutput | typreceive | typsend | typmodin | typmodout | typanalyze |
typstorage
+-------------+--------------+---------------+---------------+-----------------+------------------+------------------+------------
+ myvarcharin | myvarcharout | myvarcharrecv | myvarcharsend | varchartypmodin | varchartypmodout | array_typanalyze |
x
+(1 row)
+
+SELECT typinput, typoutput, typreceive, typsend, typmodin, typmodout,
+ typanalyze, typstorage
+FROM pg_type WHERE typname = 'myvarchardom';
+ typinput | typoutput | typreceive | typsend | typmodin | typmodout | typanalyze | typstorage
+-----------+--------------+-------------+---------------+----------+-----------+------------------+------------
+ domain_in | myvarcharout | domain_recv | myvarcharsend | - | - | array_typanalyze | x
+(1 row)
+
+-- ensure dependencies are straight
+DROP FUNCTION myvarcharsend(myvarchar); -- fail
+ERROR: cannot drop function myvarcharsend(myvarchar) because other objects depend on it
+DETAIL: type myvarchar depends on function myvarcharsend(myvarchar)
+function myvarcharin(cstring,oid,integer) depends on type myvarchar
+function myvarcharout(myvarchar) depends on type myvarchar
+function myvarcharrecv(internal,oid,integer) depends on type myvarchar
+type myvarchardom depends on function myvarcharsend(myvarchar)
+HINT: Use DROP ... CASCADE to drop the dependent objects too.
+DROP TYPE myvarchar; -- fail
+ERROR: cannot drop type myvarchar because other objects depend on it
+DETAIL: function myvarcharin(cstring,oid,integer) depends on type myvarchar
+function myvarcharout(myvarchar) depends on type myvarchar
+function myvarcharsend(myvarchar) depends on type myvarchar
+function myvarcharrecv(internal,oid,integer) depends on type myvarchar
+type myvarchardom depends on type myvarchar
+HINT: Use DROP ... CASCADE to drop the dependent objects too.
+DROP TYPE myvarchar CASCADE;
+NOTICE: drop cascades to 5 other objects
+DETAIL: drop cascades to function myvarcharin(cstring,oid,integer)
+drop cascades to function myvarcharout(myvarchar)
+drop cascades to function myvarcharsend(myvarchar)
+drop cascades to function myvarcharrecv(internal,oid,integer)
+drop cascades to type myvarchardom
diff --git a/src/test/regress/sql/create_type.sql b/src/test/regress/sql/create_type.sql
index 68b04fd..5b176bb 100644
--- a/src/test/regress/sql/create_type.sql
+++ b/src/test/regress/sql/create_type.sql
@@ -166,3 +166,60 @@ select format_type('varchar'::regtype, 42);
select format_type('bpchar'::regtype, null);
-- this behavior difference is intentional
select format_type('bpchar'::regtype, -1);
+
+--
+-- Test CREATE/ALTER TYPE using a type that's compatible with varchar,
+-- so we can re-use those support functions
+--
+CREATE TYPE myvarchar;
+
+CREATE FUNCTION myvarcharin(cstring, oid, integer) RETURNS myvarchar
+LANGUAGE internal IMMUTABLE PARALLEL SAFE STRICT AS 'varcharin';
+
+CREATE FUNCTION myvarcharout(myvarchar) RETURNS cstring
+LANGUAGE internal IMMUTABLE PARALLEL SAFE STRICT AS 'varcharout';
+
+CREATE FUNCTION myvarcharsend(myvarchar) RETURNS bytea
+LANGUAGE internal STABLE PARALLEL SAFE STRICT AS 'varcharsend';
+
+CREATE FUNCTION myvarcharrecv(internal, oid, integer) RETURNS myvarchar
+LANGUAGE internal STABLE PARALLEL SAFE STRICT AS 'varcharrecv';
+
+-- fail, it's still a shell:
+ALTER TYPE myvarchar SET (storage = extended);
+
+CREATE TYPE myvarchar (
+ input = myvarcharin,
+ output = myvarcharout,
+ alignment = integer,
+ storage = main
+);
+
+-- want to check updating of a domain over the target type, too
+CREATE DOMAIN myvarchardom AS myvarchar;
+
+ALTER TYPE myvarchar SET (storage = plain); -- not allowed
+
+ALTER TYPE myvarchar SET (storage = extended);
+
+ALTER TYPE myvarchar SET (
+ send = myvarcharsend,
+ receive = myvarcharrecv,
+ typmod_in = varchartypmodin,
+ typmod_out = varchartypmodout,
+ analyze = array_typanalyze -- bogus, but it doesn't matter
+);
+
+SELECT typinput, typoutput, typreceive, typsend, typmodin, typmodout,
+ typanalyze, typstorage
+FROM pg_type WHERE typname = 'myvarchar';
+
+SELECT typinput, typoutput, typreceive, typsend, typmodin, typmodout,
+ typanalyze, typstorage
+FROM pg_type WHERE typname = 'myvarchardom';
+
+-- ensure dependencies are straight
+DROP FUNCTION myvarcharsend(myvarchar); -- fail
+DROP TYPE myvarchar; -- fail
+
+DROP TYPE myvarchar CASCADE;
On Wed, Mar 4, 2020 at 4:15 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
I wrote:
> I think this is committable --- how about you?
... or not. I just noticed that the typcache tracks each type's
typstorage setting, and there's no provision for flushing/reloading
that.
As far as I can find, there is only one place where the cached
value is used, and that's in rangetypes.c which needs to know
whether the range element type is toastable. (It doesn't actually
need to know the exact value of typstorage, only whether it is or
isn't PLAIN.)
[...]
3. Drop the ability for ALTER TYPE to promote from PLAIN to not-PLAIN
typstorage, and adjust the typcache so that it only remembers boolean
toastability not the specific toasting strategy. Then the cache is
still immutable so no need for update logic.
I'm kind of liking #3, ugly as it sounds, because I'm not sure how
much of a use-case there is for the upgrade-from-PLAIN case.
Particularly now that TOAST is so ingrained in the system, it seems
rather unlikely that a production-grade data type wouldn't have
been designed to be toastable from the beginning, if there could be
any advantage to that. Either #1 or #2 seem like mighty high prices
to pay for offering an option that might have no real-world uses.
Tomas' opening paragraph for this thread indicated this was motivated by the plain-to-toast change but I'm not in a position to provide independent insight.
Without that piece this is mainly about being able to specify a type's preference for when and how it can be toasted. That seems like sufficient motivation, though that functionality seems basic enough that I'm wondering why it hasn't come up before now (this seems like a different topic of wonder than what Tomas mentioned in the OP).
Is there also an issue with whether the type has implemented compression or not - i.e., should the x->e and m->e paths be forbidden too? Or is it always the case a non-plain type is compressible and the other non-plain options just switch between preferences (so External just says "while I can be compressed, please don't")?
Separately...
Can you please include an edit to [1] indicating that "e" is the abbreviation for External and "x" is Extended (spelling out the other two as well). Might be worth a comment at [2] as well.
Thanks!
David J.
On Wed, Mar 04, 2020 at 06:56:42PM -0500, Tom Lane wrote: >I wrote: >> 3. Drop the ability for ALTER TYPE to promote from PLAIN to not-PLAIN >> typstorage, and adjust the typcache so that it only remembers boolean >> toastability not the specific toasting strategy. Then the cache is >> still immutable so no need for update logic. >> >> I'm kind of liking #3, ugly as it sounds, because I'm not sure how >> much of a use-case there is for the upgrade-from-PLAIN case. >> Particularly now that TOAST is so ingrained in the system, it seems >> rather unlikely that a production-grade data type wouldn't have >> been designed to be toastable from the beginning, if there could be >> any advantage to that. Either #1 or #2 seem like mighty high prices >> to pay for offering an option that might have no real-world uses. > >Here's a v5 based on that approach. I also added some comments about >the potential race conditions involved in recursing to domains. > Well, I don't know what to say, really. This very thread started with me explaining how I've repeatedly needed a way to upgrade from PLAIN, so I don't quite agree with your claim that there's no use case for that. Granted, the cases may be my faults - sometimes I have not expected the type to need TOAST initially, and then later realizing I've been wrong. In other cases I simply failed to realize PLAIN is the default value even for varlena types (yes, it's a silly mistake). FWIW I'm not suggesting you go and implement #1 or #2 for me, that'd be up to me I guess. But I disagree there's no use case for it, and #3 makes this featuer useless for me. regards -- Tomas Vondra http://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Tomas Vondra <tomas.vondra@2ndquadrant.com> writes:
> FWIW I'm not suggesting you go and implement #1 or #2 for me, that'd be
> up to me I guess. But I disagree there's no use case for it, and #3
> makes this featuer useless for me.
OK, then we need to do something else. Do you have ideas for other
alternatives?
If not, we probably should bite the bullet and go for #1, since
I have little doubt that we'll need that someday anyway.
The trick will be to keep down the cache invalidation overhead...
regards, tom lane
On Thu, Mar 05, 2020 at 02:52:44PM -0500, Tom Lane wrote: >Tomas Vondra <tomas.vondra@2ndquadrant.com> writes: >> FWIW I'm not suggesting you go and implement #1 or #2 for me, that'd be >> up to me I guess. But I disagree there's no use case for it, and #3 >> makes this featuer useless for me. > >OK, then we need to do something else. Do you have ideas for other >alternatives? > I don't have any other ideas, unfortunately. And I think if I had one, it'd probably be some sort of ugly hack anyway :-/ >If not, we probably should bite the bullet and go for #1, since >I have little doubt that we'll need that someday anyway. >The trick will be to keep down the cache invalidation overhead... > Yeah, I agree #1 seems like the cleanest/best option. Are you worried about the overhead due to the extra complexity, or overhead due to cache getting invalidated for this particular reason? regards -- Tomas Vondra http://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
I wrote:
> If not, we probably should bite the bullet and go for #1, since
> I have little doubt that we'll need that someday anyway.
> The trick will be to keep down the cache invalidation overhead...
Here's a version that does it like that. I'm less worried about the
overhead than I was before, because I realized that we already had
a syscache callback for pg_type there. And it was being pretty
stupid about which entries it reset, too, so this version might
actually net out as less overhead (in some workloads anyway).
For ease of review I just added the new TCFLAGS value out of
sequence, but I'd be inclined to renumber the bits back into
sequence before committing.
regards, tom lane
diff --git a/doc/src/sgml/ref/alter_type.sgml b/doc/src/sgml/ref/alter_type.sgml
index 67be1dd..c20ed99 100644
--- a/doc/src/sgml/ref/alter_type.sgml
+++ b/doc/src/sgml/ref/alter_type.sgml
@@ -30,6 +30,7 @@ ALTER TYPE <replaceable class="parameter">name</replaceable> RENAME TO <replacea
ALTER TYPE <replaceable class="parameter">name</replaceable> SET SCHEMA <replaceable
class="parameter">new_schema</replaceable>
ALTER TYPE <replaceable class="parameter">name</replaceable> ADD VALUE [ IF NOT EXISTS ] <replaceable
class="parameter">new_enum_value</replaceable>[ { BEFORE | AFTER } <replaceable
class="parameter">neighbor_enum_value</replaceable>]
ALTER TYPE <replaceable class="parameter">name</replaceable> RENAME VALUE <replaceable
class="parameter">existing_enum_value</replaceable>TO <replaceable class="parameter">new_enum_value</replaceable>
+ALTER TYPE <replaceable class="parameter">name</replaceable> SET ( <replaceable
class="parameter">property</replaceable>= <replaceable class="parameter">value</replaceable> [, ... ] )
<phrase>where <replaceable class="parameter">action</replaceable> is one of:</phrase>
@@ -70,7 +71,7 @@ ALTER TYPE <replaceable class="parameter">name</replaceable> RENAME VALUE <repla
</varlistentry>
<varlistentry>
- <term><literal>SET DATA TYPE</literal></term>
+ <term><literal>ALTER ATTRIBUTE ... SET DATA TYPE</literal></term>
<listitem>
<para>
This form changes the type of an attribute of a composite type.
@@ -135,6 +136,84 @@ ALTER TYPE <replaceable class="parameter">name</replaceable> RENAME VALUE <repla
</para>
</listitem>
</varlistentry>
+
+ <varlistentry>
+ <term>
+ <literal>SET ( <replaceable class="parameter">property</replaceable> = <replaceable
class="parameter">value</replaceable>[, ... ] )</literal>
+ </term>
+ <listitem>
+ <para>
+ This form is only applicable to base types. It allows adjustment of a
+ subset of the base-type properties that can be set in <command>CREATE
+ TYPE</command>. Specifically, these properties can be changed:
+ <itemizedlist>
+ <listitem>
+ <para>
+ <literal>RECEIVE</literal> can be set to the name of a binary input
+ function, or <literal>NONE</literal> to remove the type's binary
+ input function. Using this option requires superuser privilege.
+ </para>
+ </listitem>
+ <listitem>
+ <para>
+ <literal>SEND</literal> can be set to the name of a binary output
+ function, or <literal>NONE</literal> to remove the type's binary
+ output function. Using this option requires superuser privilege.
+ </para>
+ </listitem>
+ <listitem>
+ <para>
+ <literal>TYPMOD_IN</literal> can be set to the name of a type
+ modifier input function, or <literal>NONE</literal> to remove the
+ type's type modifier input function. Using this option requires
+ superuser privilege.
+ </para>
+ </listitem>
+ <listitem>
+ <para>
+ <literal>TYPMOD_OUT</literal> can be set to the name of a type
+ modifier output function, or <literal>NONE</literal> to remove the
+ type's type modifier output function. Using this option requires
+ superuser privilege.
+ </para>
+ </listitem>
+ <listitem>
+ <para>
+ <literal>ANALYZE</literal> can be set to the name of a type-specific
+ statistics collection function, or <literal>NONE</literal> to remove
+ the type's statistics collection function. Using this option
+ requires superuser privilege.
+ </para>
+ </listitem>
+ <listitem>
+ <para>
+ <literal>STORAGE</literal><indexterm>
+ <primary>TOAST</primary>
+ <secondary>per-type storage settings</secondary>
+ </indexterm>
+ can be set to <literal>plain</literal>,
+ <literal>extended</literal>, <literal>external</literal>,
+ or <literal>main</literal> (see <xref linkend="storage-toast"/> for
+ more information about what these mean). However, changing
+ from <literal>plain</literal> to another setting requires superuser
+ privilege (because it requires that the type's C functions all be
+ TOAST-ready), and changing to <literal>plain</literal> from another
+ setting is not allowed at all (since the type may already have
+ TOASTed values present in the database). Note that changing this
+ option doesn't by itself change any stored data, it just sets the
+ default TOAST strategy to be used for table columns created in the
+ future. See <xref linkend="sql-altertable"/> to change the TOAST
+ strategy for existing table columns.
+ </para>
+ </listitem>
+ </itemizedlist>
+ See <xref linkend="sql-createtype"/> for more details about these
+ type properties. Note that where appropriate, a change in these
+ properties for a base type will be propagated automatically to domains
+ based on that type.
+ </para>
+ </listitem>
+ </varlistentry>
</variablelist>
</para>
@@ -156,7 +235,7 @@ ALTER TYPE <replaceable class="parameter">name</replaceable> RENAME VALUE <repla
doesn't do anything you couldn't do by dropping and recreating the type.
However, a superuser can alter ownership of any type anyway.)
To add an attribute or alter an attribute type, you must also
- have <literal>USAGE</literal> privilege on the data type.
+ have <literal>USAGE</literal> privilege on the attribute's data type.
</para>
</refsect1>
@@ -353,7 +432,20 @@ ALTER TYPE colors ADD VALUE 'orange' AFTER 'red';
To rename an enum value:
<programlisting>
ALTER TYPE colors RENAME VALUE 'purple' TO 'mauve';
-</programlisting></para>
+</programlisting>
+ </para>
+
+ <para>
+ To create binary I/O functions for an existing base type:
+<programlisting>
+CREATE FUNCTION mytypesend(mytype) RETURNS bytea ...;
+CREATE FUNCTION mytyperecv(internal, oid, integer) RETURNS mytype ...;
+ALTER TYPE mytype SET (
+ send = mytypesend,
+ receive = mytyperecv
+);
+</programlisting>
+ </para>
</refsect1>
<refsect1>
diff --git a/src/backend/catalog/pg_type.c b/src/backend/catalog/pg_type.c
index 56e0bcf..cd56714 100644
--- a/src/backend/catalog/pg_type.c
+++ b/src/backend/catalog/pg_type.c
@@ -155,8 +155,8 @@ TypeShellMake(const char *typeName, Oid typeNamespace, Oid ownerId)
* Create dependencies. We can/must skip this in bootstrap mode.
*/
if (!IsBootstrapProcessingMode())
- GenerateTypeDependencies(typoid,
- (Form_pg_type) GETSTRUCT(tup),
+ GenerateTypeDependencies(tup,
+ pg_type_desc,
NULL,
NULL,
0,
@@ -488,8 +488,8 @@ TypeCreate(Oid newTypeOid,
* Create dependencies. We can/must skip this in bootstrap mode.
*/
if (!IsBootstrapProcessingMode())
- GenerateTypeDependencies(typeObjectId,
- (Form_pg_type) GETSTRUCT(tup),
+ GenerateTypeDependencies(tup,
+ pg_type_desc,
(defaultTypeBin ?
stringToNode(defaultTypeBin) :
NULL),
@@ -516,12 +516,16 @@ TypeCreate(Oid newTypeOid,
* GenerateTypeDependencies: build the dependencies needed for a type
*
* Most of what this function needs to know about the type is passed as the
- * new pg_type row, typeForm. But we can't get at the varlena fields through
- * that, so defaultExpr and typacl are passed separately. (typacl is really
+ * new pg_type row, typeTuple. We make callers pass the pg_type Relation
+ * as well, so that we have easy access to a tuple descriptor for the row.
+ *
+ * While this is able to extract the defaultExpr and typacl from the tuple,
+ * doing so is relatively expensive, and callers may have those values at
+ * hand already. Pass those if handy, otherwise pass NULL. (typacl is really
* "Acl *", but we declare it "void *" to avoid including acl.h in pg_type.h.)
*
- * relationKind and isImplicitArray aren't visible in the pg_type row either,
- * so they're also passed separately.
+ * relationKind and isImplicitArray are likewise somewhat expensive to deduce
+ * from the tuple, so we make callers pass those (they're not optional).
*
* isDependentType is true if this is an implicit array or relation rowtype;
* that means it doesn't need its own dependencies on owner etc.
@@ -535,8 +539,8 @@ TypeCreate(Oid newTypeOid,
* that type will become a member of the extension.)
*/
void
-GenerateTypeDependencies(Oid typeObjectId,
- Form_pg_type typeForm,
+GenerateTypeDependencies(HeapTuple typeTuple,
+ Relation typeCatalog,
Node *defaultExpr,
void *typacl,
char relationKind, /* only for relation rowtypes */
@@ -544,9 +548,30 @@ GenerateTypeDependencies(Oid typeObjectId,
bool isDependentType,
bool rebuild)
{
+ Form_pg_type typeForm = (Form_pg_type) GETSTRUCT(typeTuple);
+ Oid typeObjectId = typeForm->oid;
+ Datum datum;
+ bool isNull;
ObjectAddress myself,
referenced;
+ /* Extract defaultExpr if caller didn't pass it */
+ if (defaultExpr == NULL)
+ {
+ datum = heap_getattr(typeTuple, Anum_pg_type_typdefaultbin,
+ RelationGetDescr(typeCatalog), &isNull);
+ if (!isNull)
+ defaultExpr = stringToNode(TextDatumGetCString(datum));
+ }
+ /* Extract typacl if caller didn't pass it */
+ if (typacl == NULL)
+ {
+ datum = heap_getattr(typeTuple, Anum_pg_type_typacl,
+ RelationGetDescr(typeCatalog), &isNull);
+ if (!isNull)
+ typacl = DatumGetAclPCopy(datum);
+ }
+
/* If rebuild, first flush old dependencies, except extension deps */
if (rebuild)
{
diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c
index d732a3a..b088ca8 100644
--- a/src/backend/commands/typecmds.c
+++ b/src/backend/commands/typecmds.c
@@ -83,6 +83,25 @@ typedef struct
/* atts[] is of allocated length RelationGetNumberOfAttributes(rel) */
} RelToCheck;
+/* parameter structure for AlterTypeRecurse() */
+typedef struct
+{
+ /* Flags indicating which type attributes to update */
+ bool updateStorage;
+ bool updateReceive;
+ bool updateSend;
+ bool updateTypmodin;
+ bool updateTypmodout;
+ bool updateAnalyze;
+ /* New values for relevant attributes */
+ char storage;
+ Oid receiveOid;
+ Oid sendOid;
+ Oid typmodinOid;
+ Oid typmodoutOid;
+ Oid analyzeOid;
+} AlterTypeRecurseParams;
+
/* Potentially set by pg_upgrade_support functions */
Oid binary_upgrade_next_array_pg_type_oid = InvalidOid;
@@ -107,6 +126,8 @@ static char *domainAddConstraint(Oid domainOid, Oid domainNamespace,
const char *domainName, ObjectAddress *constrAddr);
static Node *replace_domain_constraint_value(ParseState *pstate,
ColumnRef *cref);
+static void AlterTypeRecurse(Oid typeOid, HeapTuple tup, Relation catalog,
+ AlterTypeRecurseParams *atparams);
/*
@@ -466,9 +487,12 @@ DefineType(ParseState *pstate, List *names, List *parameters)
* minimum sane check would be for execute-with-grant-option. But we
* don't have a way to make the type go away if the grant option is
* revoked, so ownership seems better.
+ *
+ * XXX For now, this is all unnecessary given the superuser check above.
+ * If we ever relax that, these calls likely should be moved into
+ * findTypeInputFunction et al, where they could be shared by AlterType.
*/
#ifdef NOT_USED
- /* XXX this is unnecessary given the superuser check above */
if (inputOid && !pg_proc_ownercheck(inputOid, GetUserId()))
aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_FUNCTION,
NameListToString(inputName));
@@ -493,47 +517,6 @@ DefineType(ParseState *pstate, List *names, List *parameters)
#endif
/*
- * Print warnings if any of the type's I/O functions are marked volatile.
- * There is a general assumption that I/O functions are stable or
- * immutable; this allows us for example to mark record_in/record_out
- * stable rather than volatile. Ideally we would throw errors not just
- * warnings here; but since this check is new as of 9.5, and since the
- * volatility marking might be just an error-of-omission and not a true
- * indication of how the function behaves, we'll let it pass as a warning
- * for now.
- */
- if (inputOid && func_volatile(inputOid) == PROVOLATILE_VOLATILE)
- ereport(WARNING,
- (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
- errmsg("type input function %s should not be volatile",
- NameListToString(inputName))));
- if (outputOid && func_volatile(outputOid) == PROVOLATILE_VOLATILE)
- ereport(WARNING,
- (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
- errmsg("type output function %s should not be volatile",
- NameListToString(outputName))));
- if (receiveOid && func_volatile(receiveOid) == PROVOLATILE_VOLATILE)
- ereport(WARNING,
- (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
- errmsg("type receive function %s should not be volatile",
- NameListToString(receiveName))));
- if (sendOid && func_volatile(sendOid) == PROVOLATILE_VOLATILE)
- ereport(WARNING,
- (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
- errmsg("type send function %s should not be volatile",
- NameListToString(sendName))));
- if (typmodinOid && func_volatile(typmodinOid) == PROVOLATILE_VOLATILE)
- ereport(WARNING,
- (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
- errmsg("type modifier input function %s should not be volatile",
- NameListToString(typmodinName))));
- if (typmodoutOid && func_volatile(typmodoutOid) == PROVOLATILE_VOLATILE)
- ereport(WARNING,
- (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
- errmsg("type modifier output function %s should not be volatile",
- NameListToString(typmodoutName))));
-
- /*
* OK, we're done checking, time to make the type. We must assign the
* array type OID ahead of calling TypeCreate, since the base type and
* array type each refer to the other.
@@ -765,6 +748,12 @@ DefineDomain(CreateDomainStmt *stmt)
aclcheck_error_type(aclresult, basetypeoid);
/*
+ * Collect the properties of the new domain. Some are inherited from the
+ * base type, some are not. If you change any of this inheritance
+ * behavior, be sure to update AlterTypeRecurse() to match!
+ */
+
+ /*
* Identify the collation if any
*/
baseColl = baseType->typcollation;
@@ -1664,6 +1653,22 @@ findTypeInputFunction(List *procname, Oid typeOid)
errmsg("type input function %s must return type %s",
NameListToString(procname), format_type_be(typeOid))));
+ /*
+ * Print warnings if any of the type's I/O functions are marked volatile.
+ * There is a general assumption that I/O functions are stable or
+ * immutable; this allows us for example to mark record_in/record_out
+ * stable rather than volatile. Ideally we would throw errors not just
+ * warnings here; but since this check is new as of 9.5, and since the
+ * volatility marking might be just an error-of-omission and not a true
+ * indication of how the function behaves, we'll let it pass as a warning
+ * for now.
+ */
+ if (func_volatile(procOid) == PROVOLATILE_VOLATILE)
+ ereport(WARNING,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("type input function %s should not be volatile",
+ NameListToString(procname))));
+
return procOid;
}
@@ -1692,6 +1697,13 @@ findTypeOutputFunction(List *procname, Oid typeOid)
errmsg("type output function %s must return type %s",
NameListToString(procname), "cstring")));
+ /* Just a warning for now, per comments in findTypeInputFunction */
+ if (func_volatile(procOid) == PROVOLATILE_VOLATILE)
+ ereport(WARNING,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("type output function %s should not be volatile",
+ NameListToString(procname))));
+
return procOid;
}
@@ -1728,6 +1740,13 @@ findTypeReceiveFunction(List *procname, Oid typeOid)
errmsg("type receive function %s must return type %s",
NameListToString(procname), format_type_be(typeOid))));
+ /* Just a warning for now, per comments in findTypeInputFunction */
+ if (func_volatile(procOid) == PROVOLATILE_VOLATILE)
+ ereport(WARNING,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("type receive function %s should not be volatile",
+ NameListToString(procname))));
+
return procOid;
}
@@ -1756,6 +1775,13 @@ findTypeSendFunction(List *procname, Oid typeOid)
errmsg("type send function %s must return type %s",
NameListToString(procname), "bytea")));
+ /* Just a warning for now, per comments in findTypeInputFunction */
+ if (func_volatile(procOid) == PROVOLATILE_VOLATILE)
+ ereport(WARNING,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("type send function %s should not be volatile",
+ NameListToString(procname))));
+
return procOid;
}
@@ -1783,6 +1809,13 @@ findTypeTypmodinFunction(List *procname)
errmsg("typmod_in function %s must return type %s",
NameListToString(procname), "integer")));
+ /* Just a warning for now, per comments in findTypeInputFunction */
+ if (func_volatile(procOid) == PROVOLATILE_VOLATILE)
+ ereport(WARNING,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("type modifier input function %s should not be volatile",
+ NameListToString(procname))));
+
return procOid;
}
@@ -1810,6 +1843,13 @@ findTypeTypmodoutFunction(List *procname)
errmsg("typmod_out function %s must return type %s",
NameListToString(procname), "cstring")));
+ /* Just a warning for now, per comments in findTypeInputFunction */
+ if (func_volatile(procOid) == PROVOLATILE_VOLATILE)
+ ereport(WARNING,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("type modifier output function %s should not be volatile",
+ NameListToString(procname))));
+
return procOid;
}
@@ -2086,9 +2126,6 @@ AlterDomainDefault(List *names, Node *defaultRaw)
Relation rel;
char *defaultValue;
Node *defaultExpr = NULL; /* NULL if no default specified */
- Acl *typacl;
- Datum aclDatum;
- bool isNull;
Datum new_record[Natts_pg_type];
bool new_record_nulls[Natts_pg_type];
bool new_record_repl[Natts_pg_type];
@@ -2141,6 +2178,7 @@ AlterDomainDefault(List *names, Node *defaultRaw)
(IsA(defaultExpr, Const) &&((Const *) defaultExpr)->constisnull))
{
/* Default is NULL, drop it */
+ defaultExpr = NULL;
new_record_nulls[Anum_pg_type_typdefaultbin - 1] = true;
new_record_repl[Anum_pg_type_typdefaultbin - 1] = true;
new_record_nulls[Anum_pg_type_typdefault - 1] = true;
@@ -2181,19 +2219,11 @@ AlterDomainDefault(List *names, Node *defaultRaw)
CatalogTupleUpdate(rel, &tup->t_self, newtuple);
- /* Must extract ACL for use of GenerateTypeDependencies */
- aclDatum = heap_getattr(newtuple, Anum_pg_type_typacl,
- RelationGetDescr(rel), &isNull);
- if (isNull)
- typacl = NULL;
- else
- typacl = DatumGetAclPCopy(aclDatum);
-
/* Rebuild dependencies */
- GenerateTypeDependencies(domainoid,
- (Form_pg_type) GETSTRUCT(newtuple),
+ GenerateTypeDependencies(newtuple,
+ rel,
defaultExpr,
- typacl,
+ NULL, /* don't have typacl handy */
0, /* relation kind is n/a */
false, /* a domain isn't an implicit array */
false, /* nor is it any kind of dependent type */
@@ -3609,3 +3639,351 @@ AlterTypeNamespaceInternal(Oid typeOid, Oid nspOid,
return oldNspOid;
}
+
+/*
+ * AlterType
+ * ALTER TYPE <type> SET (option = ...)
+ *
+ * NOTE: the set of changes that can be allowed here is constrained by many
+ * non-obvious implementation restrictions. Tread carefully when considering
+ * adding new flexibility.
+ */
+ObjectAddress
+AlterType(AlterTypeStmt *stmt)
+{
+ ObjectAddress address;
+ Relation catalog;
+ TypeName *typename;
+ HeapTuple tup;
+ Oid typeOid;
+ Form_pg_type typForm;
+ bool requireSuper = false;
+ AlterTypeRecurseParams atparams;
+ ListCell *pl;
+
+ catalog = table_open(TypeRelationId, RowExclusiveLock);
+
+ /* Make a TypeName so we can use standard type lookup machinery */
+ typename = makeTypeNameFromNameList(stmt->typeName);
+ tup = typenameType(NULL, typename, NULL);
+
+ typeOid = typeTypeId(tup);
+ typForm = (Form_pg_type) GETSTRUCT(tup);
+
+ /* Process options */
+ memset(&atparams, 0, sizeof(atparams));
+ foreach(pl, stmt->options)
+ {
+ DefElem *defel = (DefElem *) lfirst(pl);
+
+ if (strcmp(defel->defname, "storage") == 0)
+ {
+ char *a = defGetString(defel);
+
+ if (pg_strcasecmp(a, "plain") == 0)
+ atparams.storage = TYPSTORAGE_PLAIN;
+ else if (pg_strcasecmp(a, "external") == 0)
+ atparams.storage = TYPSTORAGE_EXTERNAL;
+ else if (pg_strcasecmp(a, "extended") == 0)
+ atparams.storage = TYPSTORAGE_EXTENDED;
+ else if (pg_strcasecmp(a, "main") == 0)
+ atparams.storage = TYPSTORAGE_MAIN;
+ else
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("storage \"%s\" not recognized", a)));
+
+ /*
+ * Validate the storage request. If the type isn't varlena, it
+ * certainly doesn't support non-PLAIN storage.
+ */
+ if (atparams.storage != TYPSTORAGE_PLAIN && typForm->typlen != -1)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("fixed-size types must have storage PLAIN")));
+
+ /*
+ * Switching from PLAIN to non-PLAIN is allowed, but it requires
+ * superuser, since we can't validate that the type's C functions
+ * will support it. Switching from non-PLAIN to PLAIN is
+ * disallowed outright, because it's not practical to ensure that
+ * no tables have toasted values of the type. Switching among
+ * different non-PLAIN settings is OK, since it just constitutes a
+ * change in the strategy requested for columns created in the
+ * future.
+ */
+ if (atparams.storage != TYPSTORAGE_PLAIN &&
+ typForm->typstorage == TYPSTORAGE_PLAIN)
+ requireSuper = true;
+ else if (atparams.storage == TYPSTORAGE_PLAIN &&
+ typForm->typstorage != TYPSTORAGE_PLAIN)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("cannot change type's storage to PLAIN")));
+
+ atparams.updateStorage = true;
+ }
+ else if (strcmp(defel->defname, "receive") == 0)
+ {
+ if (defel->arg != NULL)
+ atparams.receiveOid =
+ findTypeReceiveFunction(defGetQualifiedName(defel),
+ typeOid);
+ else
+ atparams.receiveOid = InvalidOid; /* NONE, remove function */
+ atparams.updateReceive = true;
+ /* Replacing an I/O function requires superuser. */
+ requireSuper = true;
+ }
+ else if (strcmp(defel->defname, "send") == 0)
+ {
+ if (defel->arg != NULL)
+ atparams.sendOid =
+ findTypeSendFunction(defGetQualifiedName(defel),
+ typeOid);
+ else
+ atparams.sendOid = InvalidOid; /* NONE, remove function */
+ atparams.updateSend = true;
+ /* Replacing an I/O function requires superuser. */
+ requireSuper = true;
+ }
+ else if (strcmp(defel->defname, "typmod_in") == 0)
+ {
+ if (defel->arg != NULL)
+ atparams.typmodinOid =
+ findTypeTypmodinFunction(defGetQualifiedName(defel));
+ else
+ atparams.typmodinOid = InvalidOid; /* NONE, remove function */
+ atparams.updateTypmodin = true;
+ /* Replacing an I/O function requires superuser. */
+ requireSuper = true;
+ }
+ else if (strcmp(defel->defname, "typmod_out") == 0)
+ {
+ if (defel->arg != NULL)
+ atparams.typmodoutOid =
+ findTypeTypmodoutFunction(defGetQualifiedName(defel));
+ else
+ atparams.typmodoutOid = InvalidOid; /* NONE, remove function */
+ atparams.updateTypmodout = true;
+ /* Replacing an I/O function requires superuser. */
+ requireSuper = true;
+ }
+ else if (strcmp(defel->defname, "analyze") == 0)
+ {
+ if (defel->arg != NULL)
+ atparams.analyzeOid =
+ findTypeAnalyzeFunction(defGetQualifiedName(defel),
+ typeOid);
+ else
+ atparams.analyzeOid = InvalidOid; /* NONE, remove function */
+ atparams.updateAnalyze = true;
+ /* Replacing an analyze function requires superuser. */
+ requireSuper = true;
+ }
+
+ /*
+ * The rest of the options that CREATE accepts cannot be changed.
+ * Check for them so that we can give a meaningful error message.
+ */
+ else if (strcmp(defel->defname, "input") == 0 ||
+ strcmp(defel->defname, "output") == 0 ||
+ strcmp(defel->defname, "internallength") == 0 ||
+ strcmp(defel->defname, "passedbyvalue") == 0 ||
+ strcmp(defel->defname, "alignment") == 0 ||
+ strcmp(defel->defname, "like") == 0 ||
+ strcmp(defel->defname, "category") == 0 ||
+ strcmp(defel->defname, "preferred") == 0 ||
+ strcmp(defel->defname, "default") == 0 ||
+ strcmp(defel->defname, "element") == 0 ||
+ strcmp(defel->defname, "delimiter") == 0 ||
+ strcmp(defel->defname, "collatable") == 0)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("type attribute \"%s\" cannot be changed",
+ defel->defname)));
+ else
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("type attribute \"%s\" not recognized",
+ defel->defname)));
+ }
+
+ /*
+ * Permissions check. Require superuser if we decided the command
+ * requires that, else must own the type.
+ */
+ if (requireSuper)
+ {
+ if (!superuser())
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("must be superuser to alter a type")));
+ }
+ else
+ {
+ if (!pg_type_ownercheck(typeOid, GetUserId()))
+ aclcheck_error_type(ACLCHECK_NOT_OWNER, typeOid);
+ }
+
+ /*
+ * We disallow all forms of ALTER TYPE SET on types that aren't plain base
+ * types. It would for example be highly unsafe, not to mention
+ * pointless, to change the send/receive functions for a composite type.
+ * Moreover, pg_dump has no support for changing these properties on
+ * non-base types. We might weaken this someday, but not now.
+ *
+ * Note: if you weaken this enough to allow composite types, be sure to
+ * adjust the GenerateTypeDependencies call in AlterTypeRecurse.
+ */
+ if (typForm->typtype != TYPTYPE_BASE)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("%s is not a base type",
+ format_type_be(typeOid))));
+
+ /*
+ * For the same reasons, don't allow direct alteration of array types.
+ */
+ if (OidIsValid(typForm->typelem) &&
+ get_array_type(typForm->typelem) == typeOid)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("%s is not a base type",
+ format_type_be(typeOid))));
+
+ /* OK, recursively update this type and any domains over it */
+ AlterTypeRecurse(typeOid, tup, catalog, &atparams);
+
+ /* Clean up */
+ ReleaseSysCache(tup);
+
+ table_close(catalog, RowExclusiveLock);
+
+ ObjectAddressSet(address, TypeRelationId, typeOid);
+
+ return address;
+}
+
+/*
+ * AlterTypeRecurse: one recursion step for AlterType()
+ *
+ * Apply the changes specified by "atparams" to the type identified by
+ * "typeOid", whose existing pg_type tuple is "tup". Then search for any
+ * domains over this type, and recursively apply (most of) the same changes
+ * to those domains.
+ *
+ * We need this because the system generally assumes that a domain inherits
+ * many properties from its base type. See DefineDomain() above for details
+ * of what is inherited.
+ *
+ * There's a race condition here, in that some other transaction could
+ * concurrently add another domain atop this base type; we'd miss updating
+ * that one. Hence, be wary of allowing ALTER TYPE to change properties for
+ * which it'd be really fatal for a domain to be out of sync with its base
+ * type (typlen, for example). In practice, races seem unlikely to be an
+ * issue for plausible use-cases for ALTER TYPE. If one does happen, it could
+ * be fixed by re-doing the same ALTER TYPE once all prior transactions have
+ * committed.
+ */
+static void
+AlterTypeRecurse(Oid typeOid, HeapTuple tup, Relation catalog,
+ AlterTypeRecurseParams *atparams)
+{
+ Datum values[Natts_pg_type];
+ bool nulls[Natts_pg_type];
+ bool replaces[Natts_pg_type];
+ HeapTuple newtup;
+ SysScanDesc scan;
+ ScanKeyData key[1];
+ HeapTuple domainTup;
+
+ /* Since this function recurses, it could be driven to stack overflow */
+ check_stack_depth();
+
+ /* Update the current type's tuple */
+ memset(values, 0, sizeof(values));
+ memset(nulls, 0, sizeof(nulls));
+ memset(replaces, 0, sizeof(replaces));
+
+ if (atparams->updateStorage)
+ {
+ replaces[Anum_pg_type_typstorage - 1] = true;
+ values[Anum_pg_type_typstorage - 1] = CharGetDatum(atparams->storage);
+ }
+ if (atparams->updateReceive)
+ {
+ replaces[Anum_pg_type_typreceive - 1] = true;
+ values[Anum_pg_type_typreceive - 1] = ObjectIdGetDatum(atparams->receiveOid);
+ }
+ if (atparams->updateSend)
+ {
+ replaces[Anum_pg_type_typsend - 1] = true;
+ values[Anum_pg_type_typsend - 1] = ObjectIdGetDatum(atparams->sendOid);
+ }
+ if (atparams->updateTypmodin)
+ {
+ replaces[Anum_pg_type_typmodin - 1] = true;
+ values[Anum_pg_type_typmodin - 1] = ObjectIdGetDatum(atparams->typmodinOid);
+ }
+ if (atparams->updateTypmodout)
+ {
+ replaces[Anum_pg_type_typmodout - 1] = true;
+ values[Anum_pg_type_typmodout - 1] = ObjectIdGetDatum(atparams->typmodoutOid);
+ }
+ if (atparams->updateAnalyze)
+ {
+ replaces[Anum_pg_type_typanalyze - 1] = true;
+ values[Anum_pg_type_typanalyze - 1] = ObjectIdGetDatum(atparams->analyzeOid);
+ }
+
+ newtup = heap_modify_tuple(tup, RelationGetDescr(catalog),
+ values, nulls, replaces);
+
+ CatalogTupleUpdate(catalog, &newtup->t_self, newtup);
+
+ /* Rebuild dependencies for this type */
+ GenerateTypeDependencies(newtup,
+ catalog,
+ NULL, /* don't have defaultExpr handy */
+ NULL, /* don't have typacl handy */
+ 0, /* we rejected composite types above */
+ false, /* and we rejected implicit arrays above */
+ false, /* so it can't be a dependent type */
+ true);
+
+ InvokeObjectPostAlterHook(TypeRelationId, typeOid, 0);
+
+ /*
+ * Now we need to recurse to domains. However, some properties are not
+ * inherited by domains, so clear the update flags for those.
+ */
+ atparams->updateReceive = false; /* domains use F_DOMAIN_RECV */
+ atparams->updateTypmodin = false; /* domains don't have typmods */
+ atparams->updateTypmodout = false;
+
+ /* Search pg_type for possible domains over this type */
+ ScanKeyInit(&key[0],
+ Anum_pg_type_typbasetype,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(typeOid));
+
+ scan = systable_beginscan(catalog, InvalidOid, false,
+ NULL, 1, key);
+
+ while ((domainTup = systable_getnext(scan)) != NULL)
+ {
+ Form_pg_type domainForm = (Form_pg_type) GETSTRUCT(domainTup);
+
+ /*
+ * Shouldn't have a nonzero typbasetype in a non-domain, but let's
+ * check
+ */
+ if (domainForm->typtype != TYPTYPE_DOMAIN)
+ continue;
+
+ AlterTypeRecurse(domainForm->oid, domainTup, catalog, atparams);
+ }
+
+ systable_endscan(scan);
+}
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index e04c33e..eaab97f 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -3636,6 +3636,17 @@ _copyAlterOperatorStmt(const AlterOperatorStmt *from)
return newnode;
}
+static AlterTypeStmt *
+_copyAlterTypeStmt(const AlterTypeStmt *from)
+{
+ AlterTypeStmt *newnode = makeNode(AlterTypeStmt);
+
+ COPY_NODE_FIELD(typeName);
+ COPY_NODE_FIELD(options);
+
+ return newnode;
+}
+
static RuleStmt *
_copyRuleStmt(const RuleStmt *from)
{
@@ -5263,6 +5274,9 @@ copyObjectImpl(const void *from)
case T_AlterOperatorStmt:
retval = _copyAlterOperatorStmt(from);
break;
+ case T_AlterTypeStmt:
+ retval = _copyAlterTypeStmt(from);
+ break;
case T_RuleStmt:
retval = _copyRuleStmt(from);
break;
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 5b1ba14..88b9129 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -1482,6 +1482,15 @@ _equalAlterOperatorStmt(const AlterOperatorStmt *a, const AlterOperatorStmt *b)
}
static bool
+_equalAlterTypeStmt(const AlterTypeStmt *a, const AlterTypeStmt *b)
+{
+ COMPARE_NODE_FIELD(typeName);
+ COMPARE_NODE_FIELD(options);
+
+ return true;
+}
+
+static bool
_equalRuleStmt(const RuleStmt *a, const RuleStmt *b)
{
COMPARE_NODE_FIELD(relation);
@@ -3359,6 +3368,9 @@ equal(const void *a, const void *b)
case T_AlterOperatorStmt:
retval = _equalAlterOperatorStmt(a, b);
break;
+ case T_AlterTypeStmt:
+ retval = _equalAlterTypeStmt(a, b);
+ break;
case T_RuleStmt:
retval = _equalRuleStmt(a, b);
break;
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 96e7fdb..7e384f9 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -249,7 +249,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
AlterDatabaseStmt AlterDatabaseSetStmt AlterDomainStmt AlterEnumStmt
AlterFdwStmt AlterForeignServerStmt AlterGroupStmt
AlterObjectDependsStmt AlterObjectSchemaStmt AlterOwnerStmt
- AlterOperatorStmt AlterSeqStmt AlterSystemStmt AlterTableStmt
+ AlterOperatorStmt AlterTypeStmt AlterSeqStmt AlterSystemStmt AlterTableStmt
AlterTblSpcStmt AlterExtensionStmt AlterExtensionContentsStmt AlterForeignTableStmt
AlterCompositeTypeStmt AlterUserMappingStmt
AlterRoleStmt AlterRoleSetStmt AlterPolicyStmt AlterStatsStmt
@@ -847,6 +847,7 @@ stmt :
| AlterObjectSchemaStmt
| AlterOwnerStmt
| AlterOperatorStmt
+ | AlterTypeStmt
| AlterPolicyStmt
| AlterSeqStmt
| AlterSystemStmt
@@ -9367,6 +9368,24 @@ operator_def_arg:
/*****************************************************************************
*
+ * ALTER TYPE name SET define
+ *
+ * We repurpose ALTER OPERATOR's version of "definition" here
+ *
+ *****************************************************************************/
+
+AlterTypeStmt:
+ ALTER TYPE_P any_name SET '(' operator_def_list ')'
+ {
+ AlterTypeStmt *n = makeNode(AlterTypeStmt);
+ n->typeName = $3;
+ n->options = $6;
+ $$ = (Node *)n;
+ }
+ ;
+
+/*****************************************************************************
+ *
* ALTER THING name OWNER TO newname
*
*****************************************************************************/
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 1b460a2..b1f7f6e 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -162,6 +162,7 @@ ClassifyUtilityCommandAsReadOnly(Node *parsetree)
case T_AlterTableMoveAllStmt:
case T_AlterTableSpaceOptionsStmt:
case T_AlterTableStmt:
+ case T_AlterTypeStmt:
case T_AlterUserMappingStmt:
case T_CommentStmt:
case T_CompositeTypeStmt:
@@ -1713,6 +1714,10 @@ ProcessUtilitySlow(ParseState *pstate,
address = AlterOperator((AlterOperatorStmt *) parsetree);
break;
+ case T_AlterTypeStmt:
+ address = AlterType((AlterTypeStmt *) parsetree);
+ break;
+
case T_CommentStmt:
address = CommentObject((CommentStmt *) parsetree);
break;
@@ -2895,6 +2900,10 @@ CreateCommandTag(Node *parsetree)
tag = CMDTAG_ALTER_OPERATOR;
break;
+ case T_AlterTypeStmt:
+ tag = CMDTAG_ALTER_TYPE;
+ break;
+
case T_AlterTSDictionaryStmt:
tag = CMDTAG_ALTER_TEXT_SEARCH_DICTIONARY;
break;
@@ -3251,6 +3260,10 @@ GetCommandLogLevel(Node *parsetree)
lev = LOGSTMT_DDL;
break;
+ case T_AlterTypeStmt:
+ lev = LOGSTMT_DDL;
+ break;
+
case T_AlterTableMoveAllStmt:
case T_AlterTableStmt:
lev = LOGSTMT_DDL;
diff --git a/src/backend/utils/cache/typcache.c b/src/backend/utils/cache/typcache.c
index cdf6331..4213ecf 100644
--- a/src/backend/utils/cache/typcache.c
+++ b/src/backend/utils/cache/typcache.c
@@ -23,11 +23,12 @@
* permanently allows caching pointers to them in long-lived places.
*
* We have some provisions for updating cache entries if the stored data
- * becomes obsolete. Information dependent on opclasses is cleared if we
- * detect updates to pg_opclass. We also support clearing the tuple
- * descriptor and operator/function parts of a rowtype's cache entry,
- * since those may need to change as a consequence of ALTER TABLE.
- * Domain constraint changes are also tracked properly.
+ * becomes obsolete. Core data extracted from the pg_type row is updated
+ * when we detect updates to pg_type. Information dependent on opclasses is
+ * cleared if we detect updates to pg_opclass. We also support clearing the
+ * tuple descriptor and operator/function parts of a rowtype's cache entry,
+ * since those may need to change as a consequence of ALTER TABLE. Domain
+ * constraint changes are also tracked properly.
*
*
* Portions Copyright (c) 1996-2020, PostgreSQL Global Development Group
@@ -80,6 +81,7 @@ static HTAB *TypeCacheHash = NULL;
static TypeCacheEntry *firstDomainTypeEntry = NULL;
/* Private flag bits in the TypeCacheEntry.flags field */
+#define TCFLAGS_HAVE_PG_TYPE_DATA 0x040000
#define TCFLAGS_CHECKED_BTREE_OPCLASS 0x000001
#define TCFLAGS_CHECKED_HASH_OPCLASS 0x000002
#define TCFLAGS_CHECKED_EQ_OPR 0x000004
@@ -99,6 +101,12 @@ static TypeCacheEntry *firstDomainTypeEntry = NULL;
#define TCFLAGS_CHECKED_DOMAIN_CONSTRAINTS 0x010000
#define TCFLAGS_DOMAIN_BASE_IS_COMPOSITE 0x020000
+/* The flags associated with equality/comparison/hashing are all but these: */
+#define TCFLAGS_OPERATOR_FLAGS \
+ (~(TCFLAGS_HAVE_PG_TYPE_DATA | \
+ TCFLAGS_CHECKED_DOMAIN_CONSTRAINTS | \
+ TCFLAGS_DOMAIN_BASE_IS_COMPOSITE))
+
/*
* Data stored about a domain type's constraints. Note that we do not create
* this struct for the common case of a constraint-less domain; we just set
@@ -295,6 +303,7 @@ static bool range_element_has_hashing(TypeCacheEntry *typentry);
static bool range_element_has_extended_hashing(TypeCacheEntry *typentry);
static void cache_range_element_properties(TypeCacheEntry *typentry);
static void TypeCacheRelCallback(Datum arg, Oid relid);
+static void TypeCacheTypCallback(Datum arg, int cacheid, uint32 hashvalue);
static void TypeCacheOpcCallback(Datum arg, int cacheid, uint32 hashvalue);
static void TypeCacheConstrCallback(Datum arg, int cacheid, uint32 hashvalue);
static void load_enum_cache_data(TypeCacheEntry *tcache);
@@ -337,9 +346,9 @@ lookup_type_cache(Oid type_id, int flags)
/* Also set up callbacks for SI invalidations */
CacheRegisterRelcacheCallback(TypeCacheRelCallback, (Datum) 0);
+ CacheRegisterSyscacheCallback(TYPEOID, TypeCacheTypCallback, (Datum) 0);
CacheRegisterSyscacheCallback(CLAOID, TypeCacheOpcCallback, (Datum) 0);
CacheRegisterSyscacheCallback(CONSTROID, TypeCacheConstrCallback, (Datum) 0);
- CacheRegisterSyscacheCallback(TYPEOID, TypeCacheConstrCallback, (Datum) 0);
/* Also make sure CacheMemoryContext exists */
if (!CacheMemoryContext)
@@ -381,7 +390,13 @@ lookup_type_cache(Oid type_id, int flags)
Assert(!found); /* it wasn't there a moment ago */
MemSet(typentry, 0, sizeof(TypeCacheEntry));
+
+ /* These fields can never change, by definition */
typentry->type_id = type_id;
+ typentry->type_id_hash = GetSysCacheHashValue1(TYPEOID,
+ ObjectIdGetDatum(type_id));
+
+ /* Keep this part in sync with the code below */
typentry->typlen = typtup->typlen;
typentry->typbyval = typtup->typbyval;
typentry->typalign = typtup->typalign;
@@ -390,6 +405,7 @@ lookup_type_cache(Oid type_id, int flags)
typentry->typrelid = typtup->typrelid;
typentry->typelem = typtup->typelem;
typentry->typcollation = typtup->typcollation;
+ typentry->flags |= TCFLAGS_HAVE_PG_TYPE_DATA;
/* If it's a domain, immediately thread it into the domain cache list */
if (typentry->typtype == TYPTYPE_DOMAIN)
@@ -400,6 +416,43 @@ lookup_type_cache(Oid type_id, int flags)
ReleaseSysCache(tp);
}
+ else if (!(typentry->flags & TCFLAGS_HAVE_PG_TYPE_DATA))
+ {
+ /*
+ * We have an entry, but its pg_type row got changed, so reload the
+ * data obtained directly from pg_type.
+ */
+ HeapTuple tp;
+ Form_pg_type typtup;
+
+ tp = SearchSysCache1(TYPEOID, ObjectIdGetDatum(type_id));
+ if (!HeapTupleIsValid(tp))
+ ereport(ERROR,
+ (errcode(ERRCODE_UNDEFINED_OBJECT),
+ errmsg("type with OID %u does not exist", type_id)));
+ typtup = (Form_pg_type) GETSTRUCT(tp);
+ if (!typtup->typisdefined)
+ ereport(ERROR,
+ (errcode(ERRCODE_UNDEFINED_OBJECT),
+ errmsg("type \"%s\" is only a shell",
+ NameStr(typtup->typname))));
+
+ /*
+ * Keep this part in sync with the code above. Many of these fields
+ * shouldn't ever change, particularly typtype, but copy 'em anyway.
+ */
+ typentry->typlen = typtup->typlen;
+ typentry->typbyval = typtup->typbyval;
+ typentry->typalign = typtup->typalign;
+ typentry->typstorage = typtup->typstorage;
+ typentry->typtype = typtup->typtype;
+ typentry->typrelid = typtup->typrelid;
+ typentry->typelem = typtup->typelem;
+ typentry->typcollation = typtup->typcollation;
+ typentry->flags |= TCFLAGS_HAVE_PG_TYPE_DATA;
+
+ ReleaseSysCache(tp);
+ }
/*
* Look up opclasses if we haven't already and any dependent info is
@@ -750,12 +803,17 @@ lookup_type_cache(Oid type_id, int flags)
/*
* If requested, get information about a range type
+ *
+ * This includes making sure that the basic info about the range element
+ * type is up-to-date.
*/
if ((flags & TYPECACHE_RANGE_INFO) &&
- typentry->rngelemtype == NULL &&
typentry->typtype == TYPTYPE_RANGE)
{
- load_rangetype_info(typentry);
+ if (typentry->rngelemtype == NULL)
+ load_rangetype_info(typentry);
+ else if (!(typentry->rngelemtype->flags & TCFLAGS_HAVE_PG_TYPE_DATA))
+ (void) lookup_type_cache(typentry->rngelemtype->type_id, 0);
}
/*
@@ -2129,7 +2187,7 @@ TypeCacheRelCallback(Datum arg, Oid relid)
}
/* Reset equality/comparison/hashing validity information */
- typentry->flags = 0;
+ typentry->flags &= ~TCFLAGS_OPERATOR_FLAGS;
}
else if (typentry->typtype == TYPTYPE_DOMAIN)
{
@@ -2140,7 +2198,39 @@ TypeCacheRelCallback(Datum arg, Oid relid)
* type is composite, we don't need to reset anything.
*/
if (typentry->flags & TCFLAGS_DOMAIN_BASE_IS_COMPOSITE)
- typentry->flags = 0;
+ typentry->flags &= ~TCFLAGS_OPERATOR_FLAGS;
+ }
+ }
+}
+
+/*
+ * TypeCacheTypCallback
+ * Syscache inval callback function
+ *
+ * This is called when a syscache invalidation event occurs for any
+ * pg_type row. If we have information cached about that type, mark
+ * it as needing to be reloaded.
+ */
+static void
+TypeCacheTypCallback(Datum arg, int cacheid, uint32 hashvalue)
+{
+ HASH_SEQ_STATUS status;
+ TypeCacheEntry *typentry;
+
+ /* TypeCacheHash must exist, else this callback wouldn't be registered */
+ hash_seq_init(&status, TypeCacheHash);
+ while ((typentry = (TypeCacheEntry *) hash_seq_search(&status)) != NULL)
+ {
+ /* Is this the targeted type row (or it's a total cache flush)? */
+ if (hashvalue == 0 || typentry->type_id_hash == hashvalue)
+ {
+ /*
+ * Mark the data obtained directly from pg_type as invalid. Also,
+ * if it's a domain, typnotnull might've changed, so we'll need to
+ * recalculate its constraints.
+ */
+ typentry->flags &= ~(TCFLAGS_HAVE_PG_TYPE_DATA |
+ TCFLAGS_CHECKED_DOMAIN_CONSTRAINTS);
}
}
}
@@ -2172,7 +2262,7 @@ TypeCacheOpcCallback(Datum arg, int cacheid, uint32 hashvalue)
while ((typentry = (TypeCacheEntry *) hash_seq_search(&status)) != NULL)
{
/* Reset equality/comparison/hashing validity information */
- typentry->flags = 0;
+ typentry->flags &= ~TCFLAGS_OPERATOR_FLAGS;
}
}
@@ -2181,8 +2271,8 @@ TypeCacheOpcCallback(Datum arg, int cacheid, uint32 hashvalue)
* Syscache inval callback function
*
* This is called when a syscache invalidation event occurs for any
- * pg_constraint or pg_type row. We flush information about domain
- * constraints when this happens.
+ * pg_constraint row. We flush information about domain constraints
+ * when this happens.
*
* It's slightly annoying that we can't tell whether the inval event was for a
* domain constraint/type record or not; there's usually more update traffic
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index b6b08d0..54d0317 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -2150,7 +2150,7 @@ psql_completion(const char *text, int start, int end)
else if (Matches("ALTER", "TYPE", MatchAny))
COMPLETE_WITH("ADD ATTRIBUTE", "ADD VALUE", "ALTER ATTRIBUTE",
"DROP ATTRIBUTE",
- "OWNER TO", "RENAME", "SET SCHEMA");
+ "OWNER TO", "RENAME", "SET SCHEMA", "SET (");
/* complete ALTER TYPE <foo> ADD with actions */
else if (Matches("ALTER", "TYPE", MatchAny, "ADD"))
COMPLETE_WITH("ATTRIBUTE", "VALUE");
diff --git a/src/include/catalog/pg_type.h b/src/include/catalog/pg_type.h
index f972f94..9789094 100644
--- a/src/include/catalog/pg_type.h
+++ b/src/include/catalog/pg_type.h
@@ -340,8 +340,8 @@ extern ObjectAddress TypeCreate(Oid newTypeOid,
bool typeNotNull,
Oid typeCollation);
-extern void GenerateTypeDependencies(Oid typeObjectId,
- Form_pg_type typeForm,
+extern void GenerateTypeDependencies(HeapTuple typeTuple,
+ Relation typeCatalog,
Node *defaultExpr,
void *typacl,
char relationKind, /* only for relation
diff --git a/src/include/commands/typecmds.h b/src/include/commands/typecmds.h
index fc18d64..0162bc2 100644
--- a/src/include/commands/typecmds.h
+++ b/src/include/commands/typecmds.h
@@ -54,4 +54,6 @@ extern Oid AlterTypeNamespaceInternal(Oid typeOid, Oid nspOid,
bool errorOnTableType,
ObjectAddresses *objsMoved);
+extern ObjectAddress AlterType(AlterTypeStmt *stmt);
+
#endif /* TYPECMDS_H */
diff --git a/src/include/nodes/nodes.h b/src/include/nodes/nodes.h
index baced7e..8a76afe 100644
--- a/src/include/nodes/nodes.h
+++ b/src/include/nodes/nodes.h
@@ -380,6 +380,7 @@ typedef enum NodeTag
T_AlterObjectSchemaStmt,
T_AlterOwnerStmt,
T_AlterOperatorStmt,
+ T_AlterTypeStmt,
T_DropOwnedStmt,
T_ReassignOwnedStmt,
T_CompositeTypeStmt,
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index da0706a..2039b42 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -2959,9 +2959,8 @@ typedef struct AlterOwnerStmt
RoleSpec *newowner; /* the new owner */
} AlterOwnerStmt;
-
/* ----------------------
- * Alter Operator Set Restrict, Join
+ * Alter Operator Set ( this-n-that )
* ----------------------
*/
typedef struct AlterOperatorStmt
@@ -2971,6 +2970,16 @@ typedef struct AlterOperatorStmt
List *options; /* List of DefElem nodes */
} AlterOperatorStmt;
+/* ------------------------
+ * Alter Type Set ( this-n-that )
+ * ------------------------
+ */
+typedef struct AlterTypeStmt
+{
+ NodeTag type;
+ List *typeName; /* type name (possibly qualified) */
+ List *options; /* List of DefElem nodes */
+} AlterTypeStmt;
/* ----------------------
* Create Rule Statement
diff --git a/src/include/utils/typcache.h b/src/include/utils/typcache.h
index 66ff17d..cdd20e5 100644
--- a/src/include/utils/typcache.h
+++ b/src/include/utils/typcache.h
@@ -33,6 +33,8 @@ typedef struct TypeCacheEntry
/* typeId is the hash lookup key and MUST BE FIRST */
Oid type_id; /* OID of the data type */
+ uint32 type_id_hash; /* hashed value of the OID */
+
/* some subsidiary information copied from the pg_type row */
int16 typlen;
bool typbyval;
diff --git a/src/test/regress/expected/create_type.out b/src/test/regress/expected/create_type.out
index eb55e25..86a8b65 100644
--- a/src/test/regress/expected/create_type.out
+++ b/src/test/regress/expected/create_type.out
@@ -224,3 +224,81 @@ select format_type('bpchar'::regtype, -1);
bpchar
(1 row)
+--
+-- Test CREATE/ALTER TYPE using a type that's compatible with varchar,
+-- so we can re-use those support functions
+--
+CREATE TYPE myvarchar;
+CREATE FUNCTION myvarcharin(cstring, oid, integer) RETURNS myvarchar
+LANGUAGE internal IMMUTABLE PARALLEL SAFE STRICT AS 'varcharin';
+NOTICE: return type myvarchar is only a shell
+CREATE FUNCTION myvarcharout(myvarchar) RETURNS cstring
+LANGUAGE internal IMMUTABLE PARALLEL SAFE STRICT AS 'varcharout';
+NOTICE: argument type myvarchar is only a shell
+CREATE FUNCTION myvarcharsend(myvarchar) RETURNS bytea
+LANGUAGE internal STABLE PARALLEL SAFE STRICT AS 'varcharsend';
+NOTICE: argument type myvarchar is only a shell
+CREATE FUNCTION myvarcharrecv(internal, oid, integer) RETURNS myvarchar
+LANGUAGE internal STABLE PARALLEL SAFE STRICT AS 'varcharrecv';
+NOTICE: return type myvarchar is only a shell
+-- fail, it's still a shell:
+ALTER TYPE myvarchar SET (storage = extended);
+ERROR: type "myvarchar" is only a shell
+CREATE TYPE myvarchar (
+ input = myvarcharin,
+ output = myvarcharout,
+ alignment = integer,
+ storage = main
+);
+-- want to check updating of a domain over the target type, too
+CREATE DOMAIN myvarchardom AS myvarchar;
+ALTER TYPE myvarchar SET (storage = plain); -- not allowed
+ERROR: cannot change type's storage to PLAIN
+ALTER TYPE myvarchar SET (storage = extended);
+ALTER TYPE myvarchar SET (
+ send = myvarcharsend,
+ receive = myvarcharrecv,
+ typmod_in = varchartypmodin,
+ typmod_out = varchartypmodout,
+ analyze = array_typanalyze -- bogus, but it doesn't matter
+);
+SELECT typinput, typoutput, typreceive, typsend, typmodin, typmodout,
+ typanalyze, typstorage
+FROM pg_type WHERE typname = 'myvarchar';
+ typinput | typoutput | typreceive | typsend | typmodin | typmodout | typanalyze |
typstorage
+-------------+--------------+---------------+---------------+-----------------+------------------+------------------+------------
+ myvarcharin | myvarcharout | myvarcharrecv | myvarcharsend | varchartypmodin | varchartypmodout | array_typanalyze |
x
+(1 row)
+
+SELECT typinput, typoutput, typreceive, typsend, typmodin, typmodout,
+ typanalyze, typstorage
+FROM pg_type WHERE typname = 'myvarchardom';
+ typinput | typoutput | typreceive | typsend | typmodin | typmodout | typanalyze | typstorage
+-----------+--------------+-------------+---------------+----------+-----------+------------------+------------
+ domain_in | myvarcharout | domain_recv | myvarcharsend | - | - | array_typanalyze | x
+(1 row)
+
+-- ensure dependencies are straight
+DROP FUNCTION myvarcharsend(myvarchar); -- fail
+ERROR: cannot drop function myvarcharsend(myvarchar) because other objects depend on it
+DETAIL: type myvarchar depends on function myvarcharsend(myvarchar)
+function myvarcharin(cstring,oid,integer) depends on type myvarchar
+function myvarcharout(myvarchar) depends on type myvarchar
+function myvarcharrecv(internal,oid,integer) depends on type myvarchar
+type myvarchardom depends on function myvarcharsend(myvarchar)
+HINT: Use DROP ... CASCADE to drop the dependent objects too.
+DROP TYPE myvarchar; -- fail
+ERROR: cannot drop type myvarchar because other objects depend on it
+DETAIL: function myvarcharin(cstring,oid,integer) depends on type myvarchar
+function myvarcharout(myvarchar) depends on type myvarchar
+function myvarcharsend(myvarchar) depends on type myvarchar
+function myvarcharrecv(internal,oid,integer) depends on type myvarchar
+type myvarchardom depends on type myvarchar
+HINT: Use DROP ... CASCADE to drop the dependent objects too.
+DROP TYPE myvarchar CASCADE;
+NOTICE: drop cascades to 5 other objects
+DETAIL: drop cascades to function myvarcharin(cstring,oid,integer)
+drop cascades to function myvarcharout(myvarchar)
+drop cascades to function myvarcharsend(myvarchar)
+drop cascades to function myvarcharrecv(internal,oid,integer)
+drop cascades to type myvarchardom
diff --git a/src/test/regress/sql/create_type.sql b/src/test/regress/sql/create_type.sql
index 68b04fd..5b176bb 100644
--- a/src/test/regress/sql/create_type.sql
+++ b/src/test/regress/sql/create_type.sql
@@ -166,3 +166,60 @@ select format_type('varchar'::regtype, 42);
select format_type('bpchar'::regtype, null);
-- this behavior difference is intentional
select format_type('bpchar'::regtype, -1);
+
+--
+-- Test CREATE/ALTER TYPE using a type that's compatible with varchar,
+-- so we can re-use those support functions
+--
+CREATE TYPE myvarchar;
+
+CREATE FUNCTION myvarcharin(cstring, oid, integer) RETURNS myvarchar
+LANGUAGE internal IMMUTABLE PARALLEL SAFE STRICT AS 'varcharin';
+
+CREATE FUNCTION myvarcharout(myvarchar) RETURNS cstring
+LANGUAGE internal IMMUTABLE PARALLEL SAFE STRICT AS 'varcharout';
+
+CREATE FUNCTION myvarcharsend(myvarchar) RETURNS bytea
+LANGUAGE internal STABLE PARALLEL SAFE STRICT AS 'varcharsend';
+
+CREATE FUNCTION myvarcharrecv(internal, oid, integer) RETURNS myvarchar
+LANGUAGE internal STABLE PARALLEL SAFE STRICT AS 'varcharrecv';
+
+-- fail, it's still a shell:
+ALTER TYPE myvarchar SET (storage = extended);
+
+CREATE TYPE myvarchar (
+ input = myvarcharin,
+ output = myvarcharout,
+ alignment = integer,
+ storage = main
+);
+
+-- want to check updating of a domain over the target type, too
+CREATE DOMAIN myvarchardom AS myvarchar;
+
+ALTER TYPE myvarchar SET (storage = plain); -- not allowed
+
+ALTER TYPE myvarchar SET (storage = extended);
+
+ALTER TYPE myvarchar SET (
+ send = myvarcharsend,
+ receive = myvarcharrecv,
+ typmod_in = varchartypmodin,
+ typmod_out = varchartypmodout,
+ analyze = array_typanalyze -- bogus, but it doesn't matter
+);
+
+SELECT typinput, typoutput, typreceive, typsend, typmodin, typmodout,
+ typanalyze, typstorage
+FROM pg_type WHERE typname = 'myvarchar';
+
+SELECT typinput, typoutput, typreceive, typsend, typmodin, typmodout,
+ typanalyze, typstorage
+FROM pg_type WHERE typname = 'myvarchardom';
+
+-- ensure dependencies are straight
+DROP FUNCTION myvarcharsend(myvarchar); -- fail
+DROP TYPE myvarchar; -- fail
+
+DROP TYPE myvarchar CASCADE;
Tomas Vondra <tomas.vondra@2ndquadrant.com> writes:
> Yeah, I agree #1 seems like the cleanest/best option. Are you worried
> about the overhead due to the extra complexity, or overhead due to
> cache getting invalidated for this particular reason?
The overhead is basically a hash_seq_search traversal over the typcache
each time we get a pg_type inval event, which there could be a lot of.
On the other hand we have a lot of inval overhead already, so this might
not amount to anything noticeable.
regards, tom lane
On Thu, Mar 05, 2020 at 05:46:44PM -0500, Tom Lane wrote:
>I wrote:
>> If not, we probably should bite the bullet and go for #1, since
>> I have little doubt that we'll need that someday anyway.
>> The trick will be to keep down the cache invalidation overhead...
>
>Here's a version that does it like that. I'm less worried about the
>overhead than I was before, because I realized that we already had
>a syscache callback for pg_type there. And it was being pretty
>stupid about which entries it reset, too, so this version might
>actually net out as less overhead (in some workloads anyway).
>
>For ease of review I just added the new TCFLAGS value out of
>sequence, but I'd be inclined to renumber the bits back into
>sequence before committing.
>
LGTM. If I had to nitpick, I'd say that the example in docs should be
ALTER TYPE mytype SET (
SEND = mytypesend,
RECEIVE = mytyperecv
);
i.e. with uppercase SEND/RECEIVE, because that's how we spell it in
other examples in CREATE TYPE etc.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Tomas Vondra <tomas.vondra@2ndquadrant.com> writes:
> On Thu, Mar 05, 2020 at 05:46:44PM -0500, Tom Lane wrote:
>> For ease of review I just added the new TCFLAGS value out of
>> sequence, but I'd be inclined to renumber the bits back into
>> sequence before committing.
> LGTM. If I had to nitpick, I'd say that the example in docs should be
> ALTER TYPE mytype SET (
> SEND = mytypesend,
> RECEIVE = mytyperecv
> );
> i.e. with uppercase SEND/RECEIVE, because that's how we spell it in
> other examples in CREATE TYPE etc.
OK, pushed with those changes and some other docs-polishing.
regards, tom lane
"David G. Johnston" <david.g.johnston@gmail.com> writes: > Is there also an issue with whether the type has implemented compression or > not - i.e., should the x->e and m->e paths be forbidden too? Or is it > always the case a non-plain type is compressible and the other non-plain > options just switch between preferences (so External just says "while I can > be compressed, please don't")? Yeah, the only relevant issue here is "can it be toasted, or not?". A data type doesn't have direct control of which toasting options can be applied, nor does it need to, as long as the C functions apply the correct detoast macros. > Can you please include an edit to [1] indicating that "e" is the > abbreviation for External and "x" is Extended (spelling out the other two > as well). Might be worth a comment at [2] as well. > [1] https://www.postgresql.org/docs/12/catalog-pg-type.html > [2] https://www.postgresql.org/docs/12/storage-toast.html Done in [1]; I didn't see much point in changing [2]. regards, tom lane