diff options
author | Tom Lane <tgl@sss.pgh.pa.us> | 2020-03-06 12:19:29 -0500 |
---|---|---|
committer | Tom Lane <tgl@sss.pgh.pa.us> | 2020-03-06 12:19:29 -0500 |
commit | fe30e7ebfa3846416f1adeb7cf611006513a4ee0 (patch) | |
tree | 9595f013d6fee593182b0f098fbf55dd7562341e /src/backend/commands/typecmds.c | |
parent | addd034ae1795d0a99305b294e4dce44c6b1dfd8 (diff) | |
download | postgresql-fe30e7ebfa3846416f1adeb7cf611006513a4ee0.tar.gz postgresql-fe30e7ebfa3846416f1adeb7cf611006513a4ee0.zip |
Allow ALTER TYPE to change some properties of a base type.
Specifically, this patch allows ALTER TYPE to:
* Change the default TOAST strategy for a toastable base type;
* Promote a non-toastable type to toastable;
* Add/remove binary I/O functions for a type;
* Add/remove typmod I/O functions for a type;
* Add/remove a custom ANALYZE statistics functions for a type.
The first of these can be done by the type's owner; all the others
require superuser privilege since misuse could cause problems.
The main motivation for this patch is to allow extensions to
upgrade the feature sets of their data types, so the set of
alterable properties is biased towards that use-case. However
it's also true that changing some other properties would be
a lot harder, as they get baked into physical storage and/or
stored expressions that depend on the type.
Along the way, refactor GenerateTypeDependencies() to make it easier
to call, refactor DefineType's volatility checks so they can be shared
by AlterType, and teach typcache.c that it might have to reload data
from the type's pg_type row, a scenario it never handled before.
Also rearrange alter_type.sgml a bit for clarity (put the
composite-type operations together).
Tomas Vondra and Tom Lane
Discussion: https://postgr.es/m/20200228004440.b23ein4qvmxnlpht@development
Diffstat (limited to 'src/backend/commands/typecmds.c')
-rw-r--r-- | src/backend/commands/typecmds.c | 490 |
1 files changed, 434 insertions, 56 deletions
diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c index d732a3af450..b088ca848d3 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); +} |