On 29/06/2023 02:36, Michael Paquier wrote:
Hi all,
While working on a different patch, I have noted three code paths that
call changeDependencyFor() but don't check that they do not return
errors. In all the three cases (support function, extension/schema
and object/schema), it seems to me that only one dependency update is
expected.
Makes sense.
/* update dependencies to point to the new schema */
Suggest: "update dependency ..." in singular, as there should be only one.
if (changeDependencyFor(ExtensionRelationId, extensionOid,
NamespaceRelationId,
oldNspOid, nspOid) != 1)
elog(ERROR, "failed to change schema dependency for extension
%s",
NameStr(extForm->extname));
The error messages like "failed to change schema dependency for
extension" don't conform to the usual error message style. "could not
change schema dependency for extension" would be more conformant. I see
that you copy-pasted that from existing messages, and we have a bunch of
other "failed to" messages in the repository too, so I'm OK with leaving
it as it is for now. Or maybe change the wording of all the
changeDependencyFor() callers now, and consider all the other "failed
to" messages separately later.
If changeDependencyFor() returns >= 2, the message is a bit misleading.
That's what the existing callers did too, so maybe that's fine.
I can hit the above error with the attached test case. That seems wrong,
although I don't know if it means that the check is wrong or it exposed
a long-standing bug.
--
Heikki Linnakangas
Neon (https://neon.tech)
diff --git a/src/test/modules/test_extensions/sql/test_extensions.sql b/src/test/modules/test_extensions/sql/test_extensions.sql
index f4947e7da6..b6b8d9d31f 100644
--- a/src/test/modules/test_extensions/sql/test_extensions.sql
+++ b/src/test/modules/test_extensions/sql/test_extensions.sql
@@ -210,6 +210,17 @@ ALTER EXTENSION test_ext_cine UPDATE TO '1.1';
\dx+ test_ext_cine
+
+--
+CREATE SCHEMA test_func_dep1;
+CREATE SCHEMA test_func_dep2;
+CREATE EXTENSION test_ext_req_schema1 SCHEMA test_func_dep1;
+ALTER FUNCTION test_func_dep1.dep_req1() SET SCHEMA test_func_dep2;
+
+ALTER EXTENSION test_ext_req_schema1 SET SCHEMA test_func_dep2;
+
+DROP EXTENSION test_ext_req_schema1 CASCADE;
+
--
-- Test @extschema:extname@ syntax and no_relocate option
--
