Re: Allowing ALTER TYPE to change storage strategy - Mailing list pgsql-hackers
| From | Tom Lane |
|---|---|
| Subject | Re: Allowing ALTER TYPE to change storage strategy |
| Date | |
| Msg-id | 10968.1583176270@sss.pgh.pa.us Whole thread Raw |
| In response to | Re: Allowing ALTER TYPE to change storage strategy (Tom Lane <tgl@sss.pgh.pa.us>) |
| Responses |
Re: Allowing ALTER TYPE to change storage strategy
|
| List | pgsql-hackers |
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;
pgsql-hackers by date: