From c2ae83420f8bc4b9b80bfbad313b240bc406a60c Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Wed, 5 Jul 2023 15:30:01 +0900 Subject: [PATCH v2] Fix dependency handling with ALTER EXTENSION .. SET SCHEMA The error message rewordings related to changeDependencyFor() should be split into their own patch.. --- src/backend/commands/alter.c | 6 ++- src/backend/commands/cluster.c | 4 +- src/backend/commands/extension.c | 17 ++++---- src/backend/commands/functioncmds.c | 10 +++-- src/backend/commands/tablecmds.c | 2 +- src/backend/commands/typecmds.c | 2 +- .../expected/test_extensions.out | 43 +++++++++++++++++++ .../test_extensions/sql/test_extensions.sql | 32 ++++++++++++++ 8 files changed, 98 insertions(+), 18 deletions(-) diff --git a/src/backend/commands/alter.c b/src/backend/commands/alter.c index e95dc31bde..e12db795b4 100644 --- a/src/backend/commands/alter.c +++ b/src/backend/commands/alter.c @@ -848,8 +848,10 @@ AlterObjectNamespace_internal(Relation rel, Oid objid, Oid nspOid) pfree(replaces); /* update dependencies to point to the new schema */ - changeDependencyFor(classId, objid, - NamespaceRelationId, oldNspOid, nspOid); + if (changeDependencyFor(classId, objid, + NamespaceRelationId, oldNspOid, nspOid) != 1) + elog(ERROR, "could not change schema dependency for object %u", + objid); InvokeObjectPostAlterHook(classId, objid, 0); diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index 03b24ab90f..4d10cc0483 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -1273,7 +1273,7 @@ swap_relation_files(Oid r1, Oid r2, bool target_is_pg_class, AccessMethodRelationId, relam1, relam2) != 1) - elog(ERROR, "failed to change access method dependency for relation \"%s.%s\"", + elog(ERROR, "could not change access method dependency for relation \"%s.%s\"", get_namespace_name(get_rel_namespace(r1)), get_rel_name(r1)); if (changeDependencyFor(RelationRelationId, @@ -1281,7 +1281,7 @@ swap_relation_files(Oid r1, Oid r2, bool target_is_pg_class, AccessMethodRelationId, relam2, relam1) != 1) - elog(ERROR, "failed to change access method dependency for relation \"%s.%s\"", + elog(ERROR, "could not change access method dependency for relation \"%s.%s\"", get_namespace_name(get_rel_namespace(r2)), get_rel_name(r2)); } diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c index 0eabe18335..db3d860e38 100644 --- a/src/backend/commands/extension.c +++ b/src/backend/commands/extension.c @@ -2750,7 +2750,7 @@ AlterExtensionNamespace(const char *extensionName, const char *newschema, Oid *o { Oid extensionOid; Oid nspOid; - Oid oldNspOid = InvalidOid; + Oid oldNspOid; AclResult aclresult; Relation extRel; ScanKeyData key[2]; @@ -2833,6 +2833,9 @@ AlterExtensionNamespace(const char *extensionName, const char *newschema, Oid *o objsMoved = new_object_addresses(); + /* store the OID of the namespace to-be-changed */ + oldNspOid = extForm->extnamespace; + /* * Scan pg_depend to find objects that depend directly on the extension, * and alter each one's schema. @@ -2912,12 +2915,6 @@ AlterExtensionNamespace(const char *extensionName, const char *newschema, Oid *o nspOid, objsMoved); - /* - * Remember previous namespace of first object that has one - */ - if (oldNspOid == InvalidOid && dep_oldNspOid != InvalidOid) - oldNspOid = dep_oldNspOid; - /* * If not all the objects had the same old namespace (ignoring any * that are not in namespaces), complain. @@ -2948,8 +2945,10 @@ AlterExtensionNamespace(const char *extensionName, const char *newschema, Oid *o table_close(extRel, RowExclusiveLock); /* update dependencies to point to the new schema */ - changeDependencyFor(ExtensionRelationId, extensionOid, - NamespaceRelationId, oldNspOid, nspOid); + if (changeDependencyFor(ExtensionRelationId, extensionOid, + NamespaceRelationId, oldNspOid, nspOid) != 1) + elog(ERROR, "could not change schema dependency for extension %s", + NameStr(extForm->extname)); InvokeObjectPostAlterHook(ExtensionRelationId, extensionOid, 0); diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c index 49c7864c7c..7ba6a86ebe 100644 --- a/src/backend/commands/functioncmds.c +++ b/src/backend/commands/functioncmds.c @@ -1450,9 +1450,13 @@ AlterFunction(ParseState *pstate, AlterFunctionStmt *stmt) /* Add or replace dependency on support function */ if (OidIsValid(procForm->prosupport)) - changeDependencyFor(ProcedureRelationId, funcOid, - ProcedureRelationId, procForm->prosupport, - newsupport); + { + if (changeDependencyFor(ProcedureRelationId, funcOid, + ProcedureRelationId, procForm->prosupport, + newsupport) != 1) + elog(ERROR, "could not change support dependency for function %s", + get_func_name(funcOid)); + } else { ObjectAddress referenced; diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index fce5e6f220..bb90aedbac 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -16608,7 +16608,7 @@ AlterRelationNamespaceInternal(Relation classRel, Oid relOid, NamespaceRelationId, oldNspOid, newNspOid) != 1) - elog(ERROR, "failed to change schema dependency for relation \"%s\"", + elog(ERROR, "could not change schema dependency for relation \"%s\"", NameStr(classForm->relname)); } if (!already_done) diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c index 216482095d..682332bdfb 100644 --- a/src/backend/commands/typecmds.c +++ b/src/backend/commands/typecmds.c @@ -4059,7 +4059,7 @@ AlterTypeNamespaceInternal(Oid typeOid, Oid nspOid, !isImplicitArray) if (changeDependencyFor(TypeRelationId, typeOid, NamespaceRelationId, oldNspOid, nspOid) != 1) - elog(ERROR, "failed to change schema dependency for type %s", + elog(ERROR, "could not change schema dependency for type %s", format_type_be(typeOid)); InvokeObjectPostAlterHook(TypeRelationId, typeOid, 0); diff --git a/src/test/modules/test_extensions/expected/test_extensions.out b/src/test/modules/test_extensions/expected/test_extensions.out index a31775a260..254bd938be 100644 --- a/src/test/modules/test_extensions/expected/test_extensions.out +++ b/src/test/modules/test_extensions/expected/test_extensions.out @@ -312,6 +312,49 @@ Objects in extension "test_ext_cine" table ext_cine_tab3 (9 rows) +-- +-- Test extension with objects outside the extension's schema. +-- +CREATE SCHEMA test_func_dep1; +CREATE SCHEMA test_func_dep2; +CREATE SCHEMA test_func_dep3; +CREATE EXTENSION test_ext_req_schema1 SCHEMA test_func_dep1; +ALTER FUNCTION test_func_dep1.dep_req1() SET SCHEMA test_func_dep2; +SELECT pg_describe_object(classid, objid, objsubid) as obj, + pg_describe_object(refclassid, refobjid, refobjsubid) as objref, + deptype + FROM pg_depend + WHERE classid = 'pg_extension'::regclass AND + objid = (SELECT oid FROM pg_extension WHERE extname = 'test_ext_req_schema1') + ORDER BY 1, 2; + obj | objref | deptype +--------------------------------+-----------------------+--------- + extension test_ext_req_schema1 | schema test_func_dep1 | n +(1 row) + +-- fails, as function dep_req1 is not in the same schema as the extension. +ALTER EXTENSION test_ext_req_schema1 SET SCHEMA test_func_dep3; +ERROR: extension "test_ext_req_schema1" does not support SET SCHEMA +DETAIL: function test_func_dep2.dep_req1() is not in the extension's schema "test_func_dep1" +-- Move back the function, and the extension can be moved. +ALTER FUNCTION test_func_dep2.dep_req1() SET SCHEMA test_func_dep1; +ALTER EXTENSION test_ext_req_schema1 SET SCHEMA test_func_dep3; +SELECT pg_describe_object(classid, objid, objsubid) as obj, + pg_describe_object(refclassid, refobjid, refobjsubid) as objref, + deptype + FROM pg_depend + WHERE classid = 'pg_extension'::regclass AND + objid = (SELECT oid FROM pg_extension WHERE extname = 'test_ext_req_schema1') + ORDER BY 1, 2; + obj | objref | deptype +--------------------------------+-----------------------+--------- + extension test_ext_req_schema1 | schema test_func_dep3 | n +(1 row) + +DROP EXTENSION test_ext_req_schema1 CASCADE; +DROP SCHEMA test_func_dep1; +DROP SCHEMA test_func_dep2; +DROP SCHEMA test_func_dep3; -- -- Test @extschema:extname@ syntax and no_relocate option -- diff --git a/src/test/modules/test_extensions/sql/test_extensions.sql b/src/test/modules/test_extensions/sql/test_extensions.sql index f4947e7da6..5011086183 100644 --- a/src/test/modules/test_extensions/sql/test_extensions.sql +++ b/src/test/modules/test_extensions/sql/test_extensions.sql @@ -210,6 +210,38 @@ ALTER EXTENSION test_ext_cine UPDATE TO '1.1'; \dx+ test_ext_cine +-- +-- Test extension with objects outside the extension's schema. +-- +CREATE SCHEMA test_func_dep1; +CREATE SCHEMA test_func_dep2; +CREATE SCHEMA test_func_dep3; +CREATE EXTENSION test_ext_req_schema1 SCHEMA test_func_dep1; +ALTER FUNCTION test_func_dep1.dep_req1() SET SCHEMA test_func_dep2; +SELECT pg_describe_object(classid, objid, objsubid) as obj, + pg_describe_object(refclassid, refobjid, refobjsubid) as objref, + deptype + FROM pg_depend + WHERE classid = 'pg_extension'::regclass AND + objid = (SELECT oid FROM pg_extension WHERE extname = 'test_ext_req_schema1') + ORDER BY 1, 2; +-- fails, as function dep_req1 is not in the same schema as the extension. +ALTER EXTENSION test_ext_req_schema1 SET SCHEMA test_func_dep3; +-- Move back the function, and the extension can be moved. +ALTER FUNCTION test_func_dep2.dep_req1() SET SCHEMA test_func_dep1; +ALTER EXTENSION test_ext_req_schema1 SET SCHEMA test_func_dep3; +SELECT pg_describe_object(classid, objid, objsubid) as obj, + pg_describe_object(refclassid, refobjid, refobjsubid) as objref, + deptype + FROM pg_depend + WHERE classid = 'pg_extension'::regclass AND + objid = (SELECT oid FROM pg_extension WHERE extname = 'test_ext_req_schema1') + ORDER BY 1, 2; +DROP EXTENSION test_ext_req_schema1 CASCADE; +DROP SCHEMA test_func_dep1; +DROP SCHEMA test_func_dep2; +DROP SCHEMA test_func_dep3; + -- -- Test @extschema:extname@ syntax and no_relocate option -- -- 2.40.1