I wrote:
> There's one big remaining to-do item, I think: experimentation with
> pg_upgrade proves that a binary upgrade fails to fix the extension
> membership of arrays/rowtypes. That's because pg_dump needs to
> manually reconstruct the membership dependencies, and it thinks that
> it doesn't need to do anything for implicit arrays. Normally the
> point of that is that we want to exactly reconstruct the extension's
> contents, but I think in this case we'd really like to add the
> additional pg_depend entries even if they weren't there before.
> Otherwise people wouldn't get the new behavior until they do a
> full dump/reload.
> I can see two ways we could do that:
> * add logic to pg_dump
> * modify ALTER EXTENSION ADD TYPE so that it automatically recurses
> from a base type to its array type (and I guess we'd need to add
> something for relation rowtypes and multiranges, too).
> I think I like the latter approach because it's like how we
> handle ownership: pg_dump doesn't emit any commands to explicitly
> change the ownership of dependent types, either. (But see [1].)
> We could presumably steal some logic from ALTER TYPE OWNER.
> I've not tried to code that here, though.
Now that the multirange issue is fixed (3e8235ba4), here's a
new version that includes the needed recursion in ALTER EXTENSION.
I spent some more effort on a proper test case, too.
regards, tom lane
diff --git a/src/backend/catalog/pg_type.c b/src/backend/catalog/pg_type.c
index fe47be38d0..1a0460b491 100644
--- a/src/backend/catalog/pg_type.c
+++ b/src/backend/catalog/pg_type.c
@@ -540,7 +540,7 @@ TypeCreate(Oid newTypeOid,
* etc.
*
* We make an extension-membership dependency if we're in an extension
- * script and makeExtensionDep is true (and isDependentType isn't true).
+ * script and makeExtensionDep is true.
* makeExtensionDep should be true when creating a new type or replacing a
* shell type, but not for ALTER TYPE on an existing type. Passing false
* causes the type's extension membership to be left alone.
@@ -600,7 +600,7 @@ GenerateTypeDependencies(HeapTuple typeTuple,
ObjectAddressSet(myself, TypeRelationId, typeObjectId);
/*
- * Make dependencies on namespace, owner, ACL, extension.
+ * Make dependencies on namespace, owner, ACL.
*
* Skip these for a dependent type, since it will have such dependencies
* indirectly through its depended-on type or relation. An exception is
@@ -625,11 +625,18 @@ GenerateTypeDependencies(HeapTuple typeTuple,
recordDependencyOnNewAcl(TypeRelationId, typeObjectId, 0,
typeForm->typowner, typacl);
-
- if (makeExtensionDep)
- recordDependencyOnCurrentExtension(&myself, rebuild);
}
+ /*
+ * Make extension dependency if requested.
+ *
+ * We used to skip this for dependent types, but it seems better to record
+ * their extension membership explicitly; otherwise code such as
+ * postgres_fdw's shippability test will be fooled.
+ */
+ if (makeExtensionDep)
+ recordDependencyOnCurrentExtension(&myself, rebuild);
+
/* Normal dependencies on the I/O and support functions */
if (OidIsValid(typeForm->typinput))
{
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 226f85d0e3..6772d6a8d2 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -131,6 +131,9 @@ static void ApplyExtensionUpdates(Oid extensionOid,
char *origSchemaName,
bool cascade,
bool is_create);
+static void ExecAlterExtensionContentsRecurse(AlterExtensionContentsStmt *stmt,
+ ObjectAddress extension,
+ ObjectAddress object);
static char *read_whole_file(const char *filename, int *length);
@@ -3294,7 +3297,6 @@ ExecAlterExtensionContentsStmt(AlterExtensionContentsStmt *stmt,
ObjectAddress extension;
ObjectAddress object;
Relation relation;
- Oid oldExtension;
switch (stmt->objtype)
{
@@ -3349,6 +3351,38 @@ ExecAlterExtensionContentsStmt(AlterExtensionContentsStmt *stmt,
check_object_ownership(GetUserId(), stmt->objtype, object,
stmt->object, relation);
+ /* Do the update, recursing to any dependent objects */
+ ExecAlterExtensionContentsRecurse(stmt, extension, object);
+
+ /* Finish up */
+ InvokeObjectPostAlterHook(ExtensionRelationId, extension.objectId, 0);
+
+ /*
+ * If get_object_address() opened the relation for us, we close it to keep
+ * the reference count correct - but we retain any locks acquired by
+ * get_object_address() until commit time, to guard against concurrent
+ * activity.
+ */
+ if (relation != NULL)
+ relation_close(relation, NoLock);
+
+ return extension;
+}
+
+/*
+ * ExecAlterExtensionContentsRecurse
+ * Subroutine for ExecAlterExtensionContentsStmt
+ *
+ * Do the bare alteration of object's membership in extension,
+ * without permission checks. Recurse to dependent objects, if any.
+ */
+static void
+ExecAlterExtensionContentsRecurse(AlterExtensionContentsStmt *stmt,
+ ObjectAddress extension,
+ ObjectAddress object)
+{
+ Oid oldExtension;
+
/*
* Check existing extension membership.
*/
@@ -3432,18 +3466,47 @@ ExecAlterExtensionContentsStmt(AlterExtensionContentsStmt *stmt,
removeExtObjInitPriv(object.objectId, object.classId);
}
- InvokeObjectPostAlterHook(ExtensionRelationId, extension.objectId, 0);
-
/*
- * If get_object_address() opened the relation for us, we close it to keep
- * the reference count correct - but we retain any locks acquired by
- * get_object_address() until commit time, to guard against concurrent
- * activity.
+ * Recurse to any dependent objects; currently, this includes the array
+ * type of a base type, the multirange type associated with a range type,
+ * and the rowtype of a table.
*/
- if (relation != NULL)
- relation_close(relation, NoLock);
+ if (object.classId == TypeRelationId)
+ {
+ ObjectAddress depobject;
- return extension;
+ depobject.classId = TypeRelationId;
+ depobject.objectSubId = 0;
+
+ /* If it has an array type, update that too */
+ depobject.objectId = get_array_type(object.objectId);
+ if (OidIsValid(depobject.objectId))
+ ExecAlterExtensionContentsRecurse(stmt, extension, depobject);
+
+ /* If it is a range type, update the associated multirange too */
+ if (type_is_range(object.objectId))
+ {
+ depobject.objectId = get_range_multirange(object.objectId);
+ if (!OidIsValid(depobject.objectId))
+ ereport(ERROR,
+ (errcode(ERRCODE_UNDEFINED_OBJECT),
+ errmsg("could not find multirange type for data type %s",
+ format_type_be(object.objectId))));
+ ExecAlterExtensionContentsRecurse(stmt, extension, depobject);
+ }
+ }
+ if (object.classId == RelationRelationId)
+ {
+ ObjectAddress depobject;
+
+ depobject.classId = TypeRelationId;
+ depobject.objectSubId = 0;
+
+ /* It might not have a rowtype, but if it does, update that */
+ depobject.objectId = get_rel_type_id(object.objectId);
+ if (OidIsValid(depobject.objectId))
+ ExecAlterExtensionContentsRecurse(stmt, extension, depobject);
+ }
}
/*
diff --git a/src/test/modules/test_extensions/Makefile b/src/test/modules/test_extensions/Makefile
index 1388c0fb0b..7d95d6b924 100644
--- a/src/test/modules/test_extensions/Makefile
+++ b/src/test/modules/test_extensions/Makefile
@@ -4,7 +4,7 @@ MODULE = test_extensions
PGFILEDESC = "test_extensions - regression testing for EXTENSION support"
EXTENSION = test_ext1 test_ext2 test_ext3 test_ext4 test_ext5 test_ext6 \
- test_ext7 test_ext8 test_ext_cine test_ext_cor \
+ test_ext7 test_ext8 test_ext9 test_ext_cine test_ext_cor \
test_ext_cyclic1 test_ext_cyclic2 \
test_ext_extschema \
test_ext_evttrig \
@@ -13,6 +13,7 @@ EXTENSION = test_ext1 test_ext2 test_ext3 test_ext4 test_ext5 test_ext6 \
DATA = test_ext1--1.0.sql test_ext2--1.0.sql test_ext3--1.0.sql \
test_ext4--1.0.sql test_ext5--1.0.sql test_ext6--1.0.sql \
test_ext7--1.0.sql test_ext7--1.0--2.0.sql test_ext8--1.0.sql \
+ test_ext9--1.0.sql \
test_ext_cine--1.0.sql test_ext_cine--1.0--1.1.sql \
test_ext_cor--1.0.sql \
test_ext_cyclic1--1.0.sql test_ext_cyclic2--1.0.sql \
diff --git a/src/test/modules/test_extensions/expected/test_extensions.out
b/src/test/modules/test_extensions/expected/test_extensions.out
index 472627a232..a7ab244e87 100644
--- a/src/test/modules/test_extensions/expected/test_extensions.out
+++ b/src/test/modules/test_extensions/expected/test_extensions.out
@@ -53,7 +53,13 @@ Objects in extension "test_ext7"
table ext7_table1
table ext7_table2
table old_table1
-(6 rows)
+ type ext7_table1
+ type ext7_table1[]
+ type ext7_table2
+ type ext7_table2[]
+ type old_table1
+ type old_table1[]
+(12 rows)
alter extension test_ext7 update to '2.0';
\dx+ test_ext7
@@ -62,7 +68,9 @@ Objects in extension "test_ext7"
-------------------------------
sequence ext7_table2_col2_seq
table ext7_table2
-(2 rows)
+ type ext7_table2
+ type ext7_table2[]
+(4 rows)
-- test handling of temp objects created by extensions
create extension test_ext8;
@@ -79,8 +87,13 @@ order by 1;
function pg_temp.ext8_temp_even(posint)
table ext8_table1
table ext8_temp_table1
+ type ext8_table1
+ type ext8_table1[]
+ type ext8_temp_table1
+ type ext8_temp_table1[]
type posint
-(5 rows)
+ type posint[]
+(10 rows)
-- Should be possible to drop and recreate this extension
drop extension test_ext8;
@@ -97,8 +110,13 @@ order by 1;
function pg_temp.ext8_temp_even(posint)
table ext8_table1
table ext8_temp_table1
+ type ext8_table1
+ type ext8_table1[]
+ type ext8_temp_table1
+ type ext8_temp_table1[]
type posint
-(5 rows)
+ type posint[]
+(10 rows)
-- here we want to start a new session and wait till old one is gone
select pg_backend_pid() as oldpid \gset
@@ -117,11 +135,119 @@ Objects in extension "test_ext8"
----------------------------
function ext8_even(posint)
table ext8_table1
+ type ext8_table1
+ type ext8_table1[]
type posint
-(3 rows)
+ type posint[]
+(6 rows)
-- dropping it should still work
drop extension test_ext8;
+-- check handling of types as extension members
+create extension test_ext9;
+\dx+ test_ext9
+ Objects in extension "test_ext9"
+ Object description
+----------------------------------------------------
+ cast from varbitrange to varbitmultirange
+ function varbitmultirange()
+ function varbitmultirange(varbitrange)
+ function varbitmultirange(varbitrange[])
+ function varbitrange(bit varying,bit varying)
+ function varbitrange(bit varying,bit varying,text)
+ table sometable
+ type somecomposite
+ type somecomposite[]
+ type sometable
+ type sometable[]
+ type varbitmultirange
+ type varbitmultirange[]
+ type varbitrange
+ type varbitrange[]
+(15 rows)
+
+alter extension test_ext9 drop type varbitrange;
+\dx+ test_ext9
+ Objects in extension "test_ext9"
+ Object description
+----------------------------------------------------
+ cast from varbitrange to varbitmultirange
+ function varbitmultirange()
+ function varbitmultirange(varbitrange)
+ function varbitmultirange(varbitrange[])
+ function varbitrange(bit varying,bit varying)
+ function varbitrange(bit varying,bit varying,text)
+ table sometable
+ type somecomposite
+ type somecomposite[]
+ type sometable
+ type sometable[]
+(11 rows)
+
+alter extension test_ext9 add type varbitrange;
+\dx+ test_ext9
+ Objects in extension "test_ext9"
+ Object description
+----------------------------------------------------
+ cast from varbitrange to varbitmultirange
+ function varbitmultirange()
+ function varbitmultirange(varbitrange)
+ function varbitmultirange(varbitrange[])
+ function varbitrange(bit varying,bit varying)
+ function varbitrange(bit varying,bit varying,text)
+ table sometable
+ type somecomposite
+ type somecomposite[]
+ type sometable
+ type sometable[]
+ type varbitmultirange
+ type varbitmultirange[]
+ type varbitrange
+ type varbitrange[]
+(15 rows)
+
+alter extension test_ext9 drop table sometable;
+\dx+ test_ext9
+ Objects in extension "test_ext9"
+ Object description
+----------------------------------------------------
+ cast from varbitrange to varbitmultirange
+ function varbitmultirange()
+ function varbitmultirange(varbitrange)
+ function varbitmultirange(varbitrange[])
+ function varbitrange(bit varying,bit varying)
+ function varbitrange(bit varying,bit varying,text)
+ type somecomposite
+ type somecomposite[]
+ type varbitmultirange
+ type varbitmultirange[]
+ type varbitrange
+ type varbitrange[]
+(12 rows)
+
+alter extension test_ext9 add table sometable;
+\dx+ test_ext9
+ Objects in extension "test_ext9"
+ Object description
+----------------------------------------------------
+ cast from varbitrange to varbitmultirange
+ function varbitmultirange()
+ function varbitmultirange(varbitrange)
+ function varbitmultirange(varbitrange[])
+ function varbitrange(bit varying,bit varying)
+ function varbitrange(bit varying,bit varying,text)
+ table sometable
+ type somecomposite
+ type somecomposite[]
+ type sometable
+ type sometable[]
+ type varbitmultirange
+ type varbitmultirange[]
+ type varbitrange
+ type varbitrange[]
+(15 rows)
+
+drop extension test_ext9;
-- Test creation of extension in temporary schema with two-phase commit,
-- which should not work. This function wrapper is useful for portability.
-- Avoid noise caused by CONTEXT and NOTICE messages including the temporary
@@ -237,9 +363,12 @@ Objects in extension "test_ext_cor"
------------------------------
function ext_cor_func()
operator <<@@(point,polygon)
+ type ext_cor_view
+ type ext_cor_view[]
type test_ext_type
+ type test_ext_type[]
view ext_cor_view
-(4 rows)
+(7 rows)
--
-- CREATE IF NOT EXISTS is an entirely unsound thing for an extension
@@ -295,7 +424,13 @@ Objects in extension "test_ext_cine"
server ext_cine_srv
table ext_cine_tab1
table ext_cine_tab2
-(8 rows)
+ type ext_cine_mv
+ type ext_cine_mv[]
+ type ext_cine_tab1
+ type ext_cine_tab1[]
+ type ext_cine_tab2
+ type ext_cine_tab2[]
+(14 rows)
ALTER EXTENSION test_ext_cine UPDATE TO '1.1';
\dx+ test_ext_cine
@@ -311,7 +446,15 @@ Objects in extension "test_ext_cine"
table ext_cine_tab1
table ext_cine_tab2
table ext_cine_tab3
-(9 rows)
+ type ext_cine_mv
+ type ext_cine_mv[]
+ type ext_cine_tab1
+ type ext_cine_tab1[]
+ type ext_cine_tab2
+ type ext_cine_tab2[]
+ type ext_cine_tab3
+ type ext_cine_tab3[]
+(17 rows)
--
-- Test @extschema@ syntax.
diff --git a/src/test/modules/test_extensions/meson.build b/src/test/modules/test_extensions/meson.build
index aec99dc20d..9b8d0a1016 100644
--- a/src/test/modules/test_extensions/meson.build
+++ b/src/test/modules/test_extensions/meson.build
@@ -18,6 +18,8 @@ test_install_data += files(
'test_ext7.control',
'test_ext8--1.0.sql',
'test_ext8.control',
+ 'test_ext9--1.0.sql',
+ 'test_ext9.control',
'test_ext_cine--1.0.sql',
'test_ext_cine--1.0--1.1.sql',
'test_ext_cine.control',
diff --git a/src/test/modules/test_extensions/sql/test_extensions.sql
b/src/test/modules/test_extensions/sql/test_extensions.sql
index 51327cc321..c5b64f47c6 100644
--- a/src/test/modules/test_extensions/sql/test_extensions.sql
+++ b/src/test/modules/test_extensions/sql/test_extensions.sql
@@ -67,6 +67,19 @@ end';
-- dropping it should still work
drop extension test_ext8;
+-- check handling of types as extension members
+create extension test_ext9;
+\dx+ test_ext9
+alter extension test_ext9 drop type varbitrange;
+\dx+ test_ext9
+alter extension test_ext9 add type varbitrange;
+\dx+ test_ext9
+alter extension test_ext9 drop table sometable;
+\dx+ test_ext9
+alter extension test_ext9 add table sometable;
+\dx+ test_ext9
+drop extension test_ext9;
+
-- Test creation of extension in temporary schema with two-phase commit,
-- which should not work. This function wrapper is useful for portability.
diff --git a/src/test/modules/test_extensions/test_ext9--1.0.sql b/src/test/modules/test_extensions/test_ext9--1.0.sql
new file mode 100644
index 0000000000..427070bece
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext9--1.0.sql
@@ -0,0 +1,8 @@
+/* src/test/modules/test_extensions/test_ext9--1.0.sql */
+-- complain if script is sourced in psql, rather than via CREATE EXTENSION
+\echo Use "CREATE EXTENSION test_ext9" to load this file. \quit
+
+-- check handling of types as extension members
+create type varbitrange as range (subtype = varbit);
+create table sometable (f1 real, f2 real);
+create type somecomposite as (f1 float8, f2 float8);
diff --git a/src/test/modules/test_extensions/test_ext9.control b/src/test/modules/test_extensions/test_ext9.control
new file mode 100644
index 0000000000..c36eddb178
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext9.control
@@ -0,0 +1,3 @@
+comment = 'test_ext9'
+default_version = '1.0'
+relocatable = true