Обсуждение: Upgrading an extension's extnamespace from user-specified to a defined schema breaks dump/restore

"David G. Johnston" <david.g.johnston@gmail.com> writes:
> On Mon, Apr 15, 2024 at 8:15 PM David G. Johnston <
> david.g.johnston@gmail.com> wrote:
>> tl/dr - alter extension ... set schema ...  needs to update
>> pg_extension.extnamespace if the named schema matches the current value in
>> the control file.  Otherwise, extension authors can and have introduced a
>> dump/restore failure mode that the DBA can only fix via direct catalog
>> manipulation.

> Tom's recent bug regarding alter extension reminded me that no has
> expressed an opinion on this one.

A quick test says that ALTER EXTENSION SET SCHEMA *does* update
pg_extension.extnamespace to the new schema.

Re-reading your original message, I'm of the opinion that pg_cron's
control files are probably broken.  If they are changing from
relocatable to not, then they need to specify that property in
a version-specific control file not the main one.  Maybe there's
something else that needs to happen in our code, but updating
extnamespace doesn't seem to be it.  Also, I do see code that
purports to cope with a version-related update of the relocatable
flag --- whether we test that, I'm not sure, but there's not
something obviously missing.

            regards, tom lane

On Wed, May 8, 2024 at 3:29 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

"David G. Johnston" <david.g.johnston@gmail.com> writes:
> On Mon, Apr 15, 2024 at 8:15 PM David G. Johnston <
> david.g.johnston@gmail.com> wrote:
>> tl/dr - alter extension ... set schema ...  needs to update
>> pg_extension.extnamespace if the named schema matches the current value in
>> the control file.  Otherwise, extension authors can and have introduced a
>> dump/restore failure mode that the DBA can only fix via direct catalog
>> manipulation.

> Tom's recent bug regarding alter extension reminded me that no has
> expressed an opinion on this one.

A quick test says that ALTER EXTENSION SET SCHEMA *does* update
pg_extension.extnamespace to the new schema.

Re-reading your original message, I'm of the opinion that pg_cron's
control files are probably broken.  If they are changing from
relocatable to not, then they need to specify that property in
a version-specific control file not the main one.  Maybe there's
something else that needs to happen in our code, but updating
extnamespace doesn't seem to be it.  Also, I do see code that
purports to cope with a version-related update of the relocatable
flag --- whether we test that, I'm not sure, but there's not
something obviously missing.

Let me simplify this a bit.  But the parameter "relocatable" has nothing to do with this, "schema" does.

===========================

> cat testext.control
comment = 'testing extension'
default_version = '1.0'
relocatable = false
trusted = false

> cat testext--1.1.control
schema=pg_control

postgres=# create extension testext;

CREATE EXTENSION
postgres=# \dx
                 List of installed extensions
  Name   | Version |   Schema   |         Description          
---------+---------+------------+------------------------------
 plpgsql | 1.0     | pg_catalog | PL/pgSQL procedural language
 testext | 1.0     | public     | testing extension
(2 rows)

postgres=# alter extension testext update to '1.1';
ALTER EXTENSION
postgres=# \dx
                 List of installed extensions
  Name   | Version |   Schema   |         Description          
---------+---------+------------+------------------------------
 plpgsql | 1.0     | pg_catalog | PL/pgSQL procedural language
 testext | 1.1     | public     | testing extension
(2 rows)

postgres=# alter extension testext set schema pg_catalog;
ERROR:  extension "testext" does not support SET SCHEMA
================================

The update succeeds but the value for schema does not change ignoring the explicit schema now present in testext--1.1.control

The subsequent error during alter extension shows that the system is aware of the newly added schema setting.

"David G. Johnston" <david.g.johnston@gmail.com> writes:
> Let me simplify this a bit.  But the parameter "relocatable" has nothing to
> do with this, "schema" does.

> ===========================
>> cat testext.control
> comment = 'testing extension'
> default_version = '1.0'
> relocatable = false
> trusted = false

>> cat testext--1.1.control
> schema=pg_control

So the actual situation with pg_cron is that they've *never* been
relocatable, but now they want to bind a particular schema where
before they let the user choose?

I absolutely won't accept a suggestion that we should allow ALTER
SET SCHEMA if the extension is saying it's not relocatable.
Potentially breaking other non-relocatable extensions is not
an okay tradeoff for coping with an extremely questionable
decision on pg_cron's part.

I'm also not really inclined to assume that even if we let
SET SCHEMA happen, it would successfully move such an extension.
I think a reasonable penalty on pg_cron for this is that they
should have to supply an extension upgrade script that does
per-object ALTER XXX SET SCHEMA to get everything (or at least
as much as they want) into the newly selected schema.

I do see a missing bit, which is that if the new control
file says schema = whatever, we should probably update
extnamespace to match --- which I guess was your point
as well.  But I don't want to take responsibility for
making the member objects match that.

            regards, tom lane

On Wed, May 8, 2024 at 5:50 PM David G. Johnston <david.g.johnston@gmail.com> wrote:

Immaterial, they ignore @extschema@ already.

On a related note, our documentation says:

The target schema is determined by the schema parameter in the control file if that is given, otherwise by the SCHEMA option of CREATE EXTENSION if that is given, otherwise the current default object creation schema (the first one in the caller's search_path).

So it actually comes from pg_catalog, not the control file.