Thread: Upgrading an extension's extnamespace from user-specified to a defined schema breaks dump/restore
Upgrading an extension's extnamespace from user-specified to a defined schema breaks dump/restore
From
"David G. Johnston"
Date:
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.
Context:
The pg_cron project has decided they want to change from allowing the DBA to specify the schema during create extension to instead forcing pg_extension.extnamespace to be linked to the pg_catalog schema (starting in their v1.5, by accident but subsequently accepted as policy).
Description:
Users of pg_cron that cross the boundary of that decision are required to drop the extension and recreate it because extension update does not inspect the control file in order to update the schema named therein. For extension set schema it is an error to even specify set schema if there is a schema named in the control file - which requires relocate=false. Combine those two behaviors with the fact that pg_dump will always attach a schema clause to the dumped create extension command and it becomes impossible to dump and restore because the metadata cannot be made to conform to a control file now containing schema = pg_catalog.
Resolution and Reasoning:
IMO we need to fix and back-patch letting set schema accept the user specifying the name of the schema found in the control file. This at least lets the DBA make a judgement call on their database to alter the schema to what it would be if the extension were to be dropped and recreated. If the name specified does not match the one in the control file then the existing error would remain since the control file has to be defined non-relocatable for schema to even be specified. This seems even more like a bug oversight since we have already programmed create extension to behave in this very manner - allowing the specification of a schema even for an extension with a fixed control schema so long as they match.
It is arguable whether extension update is behaving sanely here but I imagine any fix here would not be back-patchable whereas fixing alter extension to go from error to useful behavior is a safe back-patchable bug fix.
David J.
P.S. The script I used here, where the extension creates a table with the same name, produces an oddity when relocation is allowed. See the final stanza of the script below.
-- testext.control
comment = 'testing extension'
default_version = '1.0'
relocatable = false
trusted = false
-- testext--1.0.sql
CREATE TABLE public.testext(id serial primary key);
-- testext--1.0--1.1.sql
CREATE TABLE public.testext2(id serial primary key);
create extension testext;
\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)
alter extension testext set schema pg_catalog;
--ERROR: testext is a table's row type
--HINT: Use ALTER TABLE instead.
-------------------------------- change testext.control ------------------------------
comment = 'testing extension'
default_version = '1.0'
relocatable = false
trusted = false
-- testext--1.0.sql
CREATE TABLE public.testext(id serial primary key);
-- testext--1.0--1.1.sql
CREATE TABLE public.testext2(id serial primary key);
create extension testext;
\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)
alter extension testext set schema pg_catalog;
--ERROR: testext is a table's row type
--HINT: Use ALTER TABLE instead.
-------------------------------- change testext.control ------------------------------
modify: default_version = '1.1'
add: schema = pg_catalog
alter extension testext update TO "1.1";
\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)
-- pg_dump ...
-- CREATE EXTENSION IF NOT EXISTS testext WITH SCHEMA public;
drop extension testext;
--DROP EXTENSION
CREATE EXTENSION IF NOT EXISTS testext WITH SCHEMA public;
--ERROR: extension "testext" must be installed in schema "pg_catalog"
\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)
-- pg_dump ...
-- CREATE EXTENSION IF NOT EXISTS testext WITH SCHEMA public;
drop extension testext;
--DROP EXTENSION
CREATE EXTENSION IF NOT EXISTS testext WITH SCHEMA public;
--ERROR: extension "testext" must be installed in schema "pg_catalog"
-- Another oddity
-- relocatable: true
\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 set schema pg_catalog;
ERROR: testext is a table's row type
HINT: Use ALTER TABLE instead.
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 set schema pg_catalog;
ERROR: testext is a table's row type
HINT: Use ALTER TABLE instead.
Re: Upgrading an extension's extnamespace from user-specified to a defined schema breaks dump/restore
From
"David G. Johnston"
Date:
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.
Thoughts?
David J.
Re: Upgrading an extension's extnamespace from user-specified to a defined schema breaks dump/restore
From
Tom Lane
Date:
"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
Re: Upgrading an extension's extnamespace from user-specified to a defined schema breaks dump/restore
From
"David G. Johnston"
Date:
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
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
================================
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.
We are being inconsistent here - especially since create extension does allow a schema to be named so long as it matches the one in the control file.
David J.
Re: Upgrading an extension's extnamespace from user-specified to a defined schema breaks dump/restore
From
"David G. Johnston"
Date:
On Wednesday, May 8, 2024, David G. Johnston <david.g.johnston@gmail.com> wrote:
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_controlpostgres=# 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.controlThe subsequent error during alter extension shows that the system is aware of the newly added schema setting.
Never mind this last bit, the error is indeed from relocatable being false. Same overall problem though, the extension is saying it’s defined schema is pg_catalog but the system chooses not to reflect that fact. This is probably the bug but one we likely need to live with. Modifying alter extension to accept the named schema and update the catalog at least gives the DBA an out from a situation we failed to prevent.
Though I can see the argument that if the extension doesn’t care about schema because everything is hard-coded, and there isn’t an option to say “null”, that pg_catalog is basically the next best choice.
David J.
Re: Upgrading an extension's extnamespace from user-specified to a defined schema breaks dump/restore
From
Tom Lane
Date:
"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
Re: Upgrading an extension's extnamespace from user-specified to a defined schema breaks dump/restore
From
"David G. Johnston"
Date:
On Wed, May 8, 2024 at 4:55 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
"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.
This doesn't work unless schema=something is in the control file in which case that very fact says the extension is expecting the alter command to work and make the catalog value equal the control file value if it isn't already. Plus, the informed DBA has to perform the manual action here.
I'm also not really inclined to assume that even if we let
SET SCHEMA happen, it would successfully move such an extension.
It wouldn't even try.
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.
Immaterial, they ignore @extschema@ already.
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.
I suppose it partly depends on how strictly one interprets:
"An extension might be relocatable during installation but not afterwards."
If "installation" includes version updates then yes we should be changing the value on-the-fly and the extension needs to determine what that means for them.
Though that seems like an impossible definition to apply without having the update scripts see both the existing name and the new name. And so if it is just creation-time only then the current behavior is technically correct.
But I don't want to take responsibility for
making the member objects match that.
Right now an extension that was erroneously configured as "relocatable during installation" when in fact it "does not support relocation at all" has no way besides drop-and-create to get into the correct configuration internally and the attempt to do so, which we silently accept but ignore, causes dump and restore to fail since the schema name present in the catalogs is part of the dump and it will not match the control file, causing create extension to fail.
A relocatable extension is unaffected here since it cannot specify a schema name in the control file.
For either version of non-relocatable extension, alter extension is not expected to relocate it. But it is still useful to update the metadata if the DBA asks us to do so explicitly, likely only at the behest of the extension author who knows that it is safe to do so for their extension.
I would leave the update path alone, leaving whatever the installation-time extension was in place.
I would give the DBA a tool to unbreak their already broken system. I'm not seeing how they can meaningfully break it more.
We thus retain the general promise that our system doesn't understand how to change from "relocate during install" to "doesn't support relocate at all" - it will not just magically do stuff if you later add a schema=something clause to your control file, knowing full well that the schema just added likely isn't the one in the existing user's system. All that, while still giving the DBA user an out via alter extension that, instead of failing, does the minimal useful thing and updates the catalog with the control file schema, and assuming that, just like with any other "does not support relocation at all" extension, everything is managed without either caring what @extschema@ is or assuming it is the value in the control file.
David J.
David J.
Re: Upgrading an extension's extnamespace from user-specified to a defined schema breaks dump/restore
From
"David G. Johnston"
Date:
On Wed, May 8, 2024 at 5:50 PM David G. Johnston <david.g.johnston@gmail.com> wrote:
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).
However:
cat testext--1.1.control
schema=pg_control
schema=pg_control
cat testext--1.0--1.1.sql
CREATE TABLE public.testtbl2(id text primary key);
INSERT INTO public.testtbl2 VALUES ('@extschema@');
CREATE TABLE public.testtbl2(id text primary key);
INSERT INTO public.testtbl2 VALUES ('@extschema@');
postgres=# create extension testext;
CREATE EXTENSION
postgres=# alter extension testext update to '1.1';
ALTER EXTENSION
postgres=# select * from testtbl2;
id
--------
public
CREATE EXTENSION
postgres=# alter extension testext update to '1.1';
ALTER EXTENSION
postgres=# select * from testtbl2;
id
--------
public
So it actually comes from pg_catalog, not the control file.
David J.
n.b.: just realized I typo'd schema pg_catalog as pg_control, though as seen that doesn't change the tests at all.
Re: Upgrading an extension's extnamespace from user-specified to a defined schema breaks dump/restore
From
"David G. Johnston"
Date:
On Wednesday, May 8, 2024, David G. Johnston <david.g.johnston@gmail.com> wrote:
On Wed, May 8, 2024 at 5:50 PM David G. Johnston <david.g.johnston@gmail.com> wrote: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.
Never mind, in the context of only create extension, which this paragraph appears in, this is perfectly fine. We simply don’t discuss the fact there is a choice between the control file and the installed value when updating an extension, and that the value if the control file is ignored. I’ve got some thoughts for some status quo documentation updates that I’ll work through, and some idea of how we’d frame this if this dba tool does get added.
David J.