From 5c439f189bf4bdbb0ff75b2043ca713d76019528 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 31 Jul 2020 17:11:28 -0400 Subject: [PATCH] Fix oversight in ALTER TYPE: typmodin/typmodout must propagate to arrays. If a base type supports typmods, its array type does too, with the same interpretation. Hence changes in pg_type.typmodin/typmodout must be propagated to the array type. While here, improve AlterTypeRecurse to not recurse to domains if there is nothing we'd need to change. Oversight in fe30e7ebf. Back-patch to v13 where that came in. --- src/backend/commands/typecmds.c | 63 +++++++++++++++++++---- src/test/regress/expected/create_type.out | 16 ++++++ src/test/regress/sql/create_type.sql | 8 +++ 3 files changed, 76 insertions(+), 11 deletions(-) diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c index 9e5938b10eb..2e107ace39b 100644 --- a/src/backend/commands/typecmds.c +++ b/src/backend/commands/typecmds.c @@ -127,7 +127,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, +static void AlterTypeRecurse(Oid typeOid, bool isImplicitArray, + HeapTuple tup, Relation catalog, AlterTypeRecurseParams *atparams); @@ -3853,8 +3854,8 @@ AlterType(AlterTypeStmt *stmt) 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); + /* OK, recursively update this type and any arrays/domains over it */ + AlterTypeRecurse(typeOid, false, tup, catalog, &atparams); /* Clean up */ ReleaseSysCache(tup); @@ -3870,13 +3871,15 @@ AlterType(AlterTypeStmt *stmt) * 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. + * "typeOid", whose existing pg_type tuple is "tup". If necessary, + * recursively update its array type as well. 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. + * of what is inherited. Arrays inherit a smaller number of properties, + * but not none. * * There's a race condition here, in that some other transaction could * concurrently add another domain atop this base type; we'd miss updating @@ -3888,7 +3891,8 @@ AlterType(AlterTypeStmt *stmt) * committed. */ static void -AlterTypeRecurse(Oid typeOid, HeapTuple tup, Relation catalog, +AlterTypeRecurse(Oid typeOid, bool isImplicitArray, + HeapTuple tup, Relation catalog, AlterTypeRecurseParams *atparams) { Datum values[Natts_pg_type]; @@ -3949,12 +3953,43 @@ AlterTypeRecurse(Oid typeOid, HeapTuple tup, Relation 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 */ + isImplicitArray, /* it might be an array */ + isImplicitArray, /* dependent iff it's array */ true); InvokeObjectPostAlterHook(TypeRelationId, typeOid, 0); + /* + * Arrays inherit their base type's typmodin and typmodout, but none of + * the other properties we're concerned with here. Recurse to the array + * type if needed. + */ + if (!isImplicitArray && + (atparams->updateTypmodin || atparams->updateTypmodout)) + { + Oid arrtypoid = ((Form_pg_type) GETSTRUCT(newtup))->typarray; + + if (OidIsValid(arrtypoid)) + { + HeapTuple arrtup; + AlterTypeRecurseParams arrparams; + + arrtup = SearchSysCache1(TYPEOID, ObjectIdGetDatum(arrtypoid)); + if (!HeapTupleIsValid(arrtup)) + elog(ERROR, "cache lookup failed for type %u", arrtypoid); + + memset(&arrparams, 0, sizeof(arrparams)); + arrparams.updateTypmodin = atparams->updateTypmodin; + arrparams.updateTypmodout = atparams->updateTypmodout; + arrparams.typmodinOid = atparams->typmodinOid; + arrparams.typmodoutOid = atparams->typmodoutOid; + + AlterTypeRecurse(arrtypoid, true, arrtup, catalog, &arrparams); + + ReleaseSysCache(arrtup); + } + } + /* * Now we need to recurse to domains. However, some properties are not * inherited by domains, so clear the update flags for those. @@ -3963,6 +3998,12 @@ AlterTypeRecurse(Oid typeOid, HeapTuple tup, Relation catalog, atparams->updateTypmodin = false; /* domains don't have typmods */ atparams->updateTypmodout = false; + /* Skip the scan if nothing remains to be done */ + if (!(atparams->updateStorage || + atparams->updateSend || + atparams->updateAnalyze)) + return; + /* Search pg_type for possible domains over this type */ ScanKeyInit(&key[0], Anum_pg_type_typbasetype, @@ -3983,7 +4024,7 @@ AlterTypeRecurse(Oid typeOid, HeapTuple tup, Relation catalog, if (domainForm->typtype != TYPTYPE_DOMAIN) continue; - AlterTypeRecurse(domainForm->oid, domainTup, catalog, atparams); + AlterTypeRecurse(domainForm->oid, false, domainTup, catalog, atparams); } systable_endscan(scan); diff --git a/src/test/regress/expected/create_type.out b/src/test/regress/expected/create_type.out index 86a8b65450f..f85afcb31ed 100644 --- a/src/test/regress/expected/create_type.out +++ b/src/test/regress/expected/create_type.out @@ -270,6 +270,14 @@ FROM pg_type WHERE typname = 'myvarchar'; 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 = '_myvarchar'; + typinput | typoutput | typreceive | typsend | typmodin | typmodout | typanalyze | typstorage +----------+-----------+------------+------------+-----------------+------------------+------------------+------------ + array_in | array_out | array_recv | array_send | varchartypmodin | varchartypmodout | array_typanalyze | x +(1 row) + SELECT typinput, typoutput, typreceive, typsend, typmodin, typmodout, typanalyze, typstorage FROM pg_type WHERE typname = 'myvarchardom'; @@ -278,6 +286,14 @@ FROM pg_type WHERE typname = 'myvarchardom'; domain_in | myvarcharout | domain_recv | myvarcharsend | - | - | 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 +----------+-----------+------------+------------+----------+-----------+------------------+------------ + array_in | array_out | array_recv | array_send | - | - | 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 diff --git a/src/test/regress/sql/create_type.sql b/src/test/regress/sql/create_type.sql index 5b176bb2aed..584ece06701 100644 --- a/src/test/regress/sql/create_type.sql +++ b/src/test/regress/sql/create_type.sql @@ -214,10 +214,18 @@ 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 = '_myvarchar'; + SELECT typinput, typoutput, typreceive, typsend, typmodin, typmodout, typanalyze, typstorage FROM pg_type WHERE typname = 'myvarchardom'; +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