Re: [PATCH] Support % wildcard in extension upgrade filenames
On Thu, Feb 01, 2024 at 04:31:26PM +0530, vignesh C wrote: > > With no update to the thread and the tests still failing I'm marking > this as returned with feedback. Please feel free to resubmit to the > next CF when there is a new version of the patch. Ok, no problem. Thank you. --strk; signature.asc Description: PGP signature
Re: [PATCH] Support % wildcard in extension upgrade filenames
On Tue, Aug 01, 2023 at 08:24:15PM +0200, Daniel Gustafsson wrote:
> > On 28 Jun 2023, at 10:29, Daniel Gustafsson wrote:
> >
> >> On 31 May 2023, at 21:07, Sandro Santilli wrote:
> >> On Thu, Apr 27, 2023 at 12:49:57PM +0200, Sandro Santilli wrote:
> >
> >>> I'm happy to bring back the control-file switch if there's an
> >>> agreement about that.
> >>
> >> I'm attaching an up-to-date version of the patch with the control-file
> >> switch back in, so there's an explicit choice by extension developers.
> >
> > This version fails the extension test since the comment from the .sql file
> > is
> > missing from test_extensions.out. Can you fix that in a rebase such that
> > the
> > CFBot can have a green build of this patch?
>
> With no update to the thread and the patch still not applying I'm marking this
> returned with feedback. Please feel free to resubmit to a future CF when
> there
> is a new version of the patch.
Updated version of the patch is attached, regresses cleanly.
Will try to submit this to future CF from the website, let me know
if I'm doing it wrong.
--strk;
>From 9b8d1f45e5cd4b1e286d1c82d8ebca4fcc25eb92 Mon Sep 17 00:00:00 2001
From: Sandro Santilli
Date: Wed, 14 Sep 2022 11:10:10 +0200
Subject: [PATCH v4] Allow wildcard (%) in extension upgrade paths
A wildcard character "%" will be accepted in the
"source" side of the upgrade script and be considered
usable to upgrade any version to the "target" side.
Using wildcards needs to be explicitly requested by
extensions via a "wildcard_upgrades" setting in their
control file.
Includes regression test and documentation.
---
doc/src/sgml/extend.sgml | 14 +
src/backend/commands/extension.c | 58 +--
src/test/modules/test_extensions/Makefile | 7 ++-
.../expected/test_extensions.out | 18 ++
.../test_extensions/sql/test_extensions.sql | 9 +++
.../test_ext_wildcard1--%--2.0.sql| 6 ++
.../test_ext_wildcard1--1.0.sql | 6 ++
.../test_ext_wildcard1.control| 4 ++
8 files changed, 113 insertions(+), 9 deletions(-)
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1.control
diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml
index 218940ee5c..3d4003eaef 100644
--- a/doc/src/sgml/extend.sgml
+++ b/doc/src/sgml/extend.sgml
@@ -822,6 +822,20 @@ RETURNS anycompatible AS ...
+
+
+ wildcard_upgrades (boolean)
+
+
+This parameter, if set to true (which is not the
+default), allows ALTER EXTENSION to consider
+a wildcard character % as matching any version of
+the extension. Such wildcard match will only be used when no
+perfect match is found for a version.
+
+
+
+
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 535072d181..c05055f5a0 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -88,6 +88,7 @@ typedef struct ExtensionControlFile
bool relocatable; /* is ALTER EXTENSION SET SCHEMA supported? */
bool superuser; /* must be superuser to install? */
bool trusted; /* allow becoming superuser on the fly? */
+ bool wildcard_upgrades; /* allow using wildcards in upgrade scripts */
int encoding; /* encoding of the script file, or -1 */
List *requires; /* names of prerequisite extensions */
List *no_relocate; /* names of prerequisite extensions that
@@ -132,6 +133,7 @@ static void ApplyExtensionUpdates(Oid extensionOid,
bool cascade,
bool is_create);
static char *read_whole_file(const char *filename, int *length);
+static bool file_exists(const char *name);
/*
@@ -584,6 +586,14 @@ parse_extension_control_file(ExtensionControlFile *control,
errmsg("parameter \"%s\" requires a Boolean value",
item->name)));
}
+ else if (strcmp(item->name, "wildcard_upgrades") == 0)
+ {
+ if (!parse_bool(item->value, &control->wildcard_upgrades))
+ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("parameter \"%s\" requires a Boolean value",
+item->name)));
+ }
else if (strcmp(item->name, "encoding") == 0)
{
control->encoding = pg_valid_server_encoding(item->value);
@@ -656,6 +666,7 @@ read_extension_control_file(const char *extname)
control->relocatable = false;
control->superuser = true;
control->trusted = fals
Re: [PATCH] Support % wildcard in extension upgrade filenames
On Thu, Apr 27, 2023 at 12:49:57PM +0200, Sandro Santilli wrote:
> On Mon, Apr 24, 2023 at 01:06:24PM -0400, Mat Arye wrote:
> > Hi All,
> >
> > I've done upgrade maintenance for multiple extensions now (TimescaleDB
> > core, Promscale) and I think the original suggestion (wildcard filenames
> > with control-file switch to enable) here is a good one.
>
> Thanks for your comment, Mat.
>
> I'm happy to bring back the control-file switch if there's an
> agreement about that.
I'm attaching an up-to-date version of the patch with the control-file
switch back in, so there's an explicit choice by extension developers.
--strk;
>From 466338401ce8305b7ac9aa59386816c3a6884f02 Mon Sep 17 00:00:00 2001
From: Sandro Santilli
Date: Wed, 14 Sep 2022 11:10:10 +0200
Subject: [PATCH v3] Allow wildcard (%) in extension upgrade paths
A wildcard character "%" will be accepted in the
"source" side of the upgrade script and be considered
usable to upgrade any version to the "target" side.
Using wildcards needs to be explicitly requested by
extensions via a "wildcard_upgrades" setting in their
control file.
Includes regression test and documentation.
---
doc/src/sgml/extend.sgml | 14 +
src/backend/commands/extension.c | 58 +--
src/test/modules/test_extensions/Makefile | 7 ++-
.../expected/test_extensions.out | 15 +
.../test_extensions/sql/test_extensions.sql | 9 +++
.../test_ext_wildcard1--%--2.0.sql| 6 ++
.../test_ext_wildcard1--1.0.sql | 6 ++
.../test_ext_wildcard1.control| 4 ++
8 files changed, 110 insertions(+), 9 deletions(-)
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1.control
diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml
index 218940ee5c..3d4003eaef 100644
--- a/doc/src/sgml/extend.sgml
+++ b/doc/src/sgml/extend.sgml
@@ -822,6 +822,20 @@ RETURNS anycompatible AS ...
+
+
+ wildcard_upgrades (boolean)
+
+
+This parameter, if set to true (which is not the
+default), allows ALTER EXTENSION to consider
+a wildcard character % as matching any version of
+the extension. Such wildcard match will only be used when no
+perfect match is found for a version.
+
+
+
+
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 0eabe18335..207b4649f2 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -88,6 +88,7 @@ typedef struct ExtensionControlFile
bool relocatable; /* is ALTER EXTENSION SET SCHEMA supported? */
bool superuser; /* must be superuser to install? */
bool trusted; /* allow becoming superuser on the fly? */
+ bool wildcard_upgrades; /* allow using wildcards in upgrade scripts */
int encoding; /* encoding of the script file, or -1 */
List *requires; /* names of prerequisite extensions */
List *no_relocate; /* names of prerequisite extensions that
@@ -132,6 +133,7 @@ static void ApplyExtensionUpdates(Oid extensionOid,
bool cascade,
bool is_create);
static char *read_whole_file(const char *filename, int *length);
+static bool file_exists(const char *name);
/*
@@ -584,6 +586,14 @@ parse_extension_control_file(ExtensionControlFile *control,
errmsg("parameter \"%s\" requires a Boolean value",
item->name)));
}
+ else if (strcmp(item->name, "wildcard_upgrades") == 0)
+ {
+ if (!parse_bool(item->value, &control->wildcard_upgrades))
+ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("parameter \"%s\" requires a Boolean value",
+item->name)));
+ }
else if (strcmp(item->name, "encoding") == 0)
{
control->encoding = pg_valid_server_encoding(item->value);
@@ -656,6 +666,7 @@ read_extension_control_file(const char *extname)
control->relocatable = false;
control->superuser = true;
control->trusted = false;
+ control->wildcard_upgrades = false;
control->encoding = -1;
/*
@@ -913,7 +924,15 @@ execute_extension_script(Oid extensionOid, ExtensionControlFile *control,
if (from_version == NULL)
elog(DEBUG1, "executing extension script for \"%s\" version '%s'", control->name, version);
else
+ {
+ if ( control->wildcard_upgrades && ! file_exists(filename) )
+ {
+ elog(DEBUG1, "extension upgrade script \"%s\" does not exist, will try wildcard", filename);
+ /* if filename does not exist,
Re: [PATCH] Support % wildcard in extension upgrade filenames
On Thu, May 18, 2023 at 11:14:52PM +0200, Przemysław Sztoch wrote: > For me, it would be a big help if you could specify a simple from/to range > as the source version: > myext--1.0.0-1.9.9--2.0.0.sql > myext--2.0.0-2.9.9--3.0.0.sql > myext--3.0.0-3.2.3--3.2.4.sql > > The idea of % wildcard in my opinion is fully contained in the from/to > range. Yes, it will be once partial matching is implemented (in its current state the patch only allows the wildcard to cover the whole version, not just a portion, but the idea was to move to partial matches too). > The name "ANY" should also be allowed as the source version. It is. The only character with a special meaning, in my patch, would be the "%" character, and would ONLY be special if the extension has set the wildcard_upgrades directive to "true" in the .control file. > Some extensions are a collection of CREATE OR REPLACE FUNCTION. All upgrades > are one and the same SQL file. Yes, this is the case for PostGIS, and that's why we're looking forward to addint support for this wildcard. --strk;
Re: [PATCH] Support % wildcard in extension upgrade filenames
On Mon, Apr 24, 2023 at 01:06:24PM -0400, Mat Arye wrote: > Hi All, > > I've done upgrade maintenance for multiple extensions now (TimescaleDB > core, Promscale) and I think the original suggestion (wildcard filenames > with control-file switch to enable) here is a good one. Thanks for your comment, Mat. I'm happy to bring back the control-file switch if there's an agreement about that. The rationale for me to add that was solely to be 100% sure about not breaking any extension using "%" character as an actual version. I never considered it a security measure because the author of the control file is the same as the author of the ugprade scripts so shipping a wildcard upgrade script could be seen as a switch itself. [...] > As for Tom's concern about downgrades, I think it's valid but it's a case > that is easy to test for in Plpgsql and either handle or error. For > example, we use semver so testing for a downgrade at the top of the upgrade > script is trivial. I'd say it could be made even easier if PostgreSQL itself would provide a variable (or other way to fetch it) for the "source" version of the extension during exection of the "source"--"target" upgrade. I'm saying this because PostGIS fetches this version by calling a PostGIS function, but some extensions might have such "version" function pointing to a C file that doesn't exist enymore at the time of upgrade and thus would be left with the impossibility to rely on calling it. --strk;
Re: Order changes in PG16 since ICU introduction
On Fri, Apr 21, 2023 at 10:27:49AM -0700, Jeff Davis wrote: > On Fri, 2023-04-21 at 19:09 +0200, Sandro Santilli wrote: > > =# select version(); > > PostgreSQL 16devel on x86_64-pc-linux-gnu, compiled by gcc (Ubuntu > > 11.3.0-1ubuntu1~22.04) 11.3.0, 64-bit > > =# show lc_collate; > > C > > =# select '+' < '-'; > > f > > What is the result of: > > select datlocprovider, datcollate, daticulocale > from pg_database where datname=current_database(); datlocprovider | i datcollate | C daticulocale | en-US --strk;
Re: Order changes in PG16 since ICU introduction
On Fri, Apr 21, 2023 at 07:14:13PM +0200, Peter Eisentraut wrote:
> On 21.04.23 19:09, Sandro Santilli wrote:
> > On Fri, Apr 21, 2023 at 11:48:51AM -0400, Tom Lane wrote:
> > > "Regina Obe" writes:
> > >
> > > > https://trac.osgeo.org/postgis/ticket/5375
> > >
> > > If they actually are using locale C, I would say this is a bug.
> > > That should designate memcmp sorting and nothing else.
> >
> > Sounds like a bug to me. This is happening with a PostgreSQL cluster
> > created and served by a build of commit c04c6c5d6f :
> >
> >=# select version();
> >PostgreSQL 16devel on x86_64-pc-linux-gnu, compiled by gcc (Ubuntu
> > 11.3.0-1ubuntu1~22.04) 11.3.0, 64-bit
> >=# show lc_collate;
> >C
> >=# select '+' < '-';
> >f
>
> If the database is created with locale provider ICU, then lc_collate does
> not apply here, so the result might be correct (depending on what locale you
> have set).
The database is created by a perl script which starts like this:
$ENV{"LC_ALL"} = "C";
$ENV{"LANG"} = "C";
And then runs:
createdb --encoding=UTF-8 --template=template0 --lc-collate=C
Should we tweak anything else to make the results predictable ?
--strk;
Re: Order changes in PG16 since ICU introduction
On Fri, Apr 21, 2023 at 11:48:51AM -0400, Tom Lane wrote: > "Regina Obe" writes: > > > https://trac.osgeo.org/postgis/ticket/5375 > > If they actually are using locale C, I would say this is a bug. > That should designate memcmp sorting and nothing else. Sounds like a bug to me. This is happening with a PostgreSQL cluster created and served by a build of commit c04c6c5d6f : =# select version(); PostgreSQL 16devel on x86_64-pc-linux-gnu, compiled by gcc (Ubuntu 11.3.0-1ubuntu1~22.04) 11.3.0, 64-bit =# show lc_collate; C =# select '+' < '-'; f =# select '+' < '-' collate "C"; t I don't know if it should matter but also: =# show lc_messages; C --strk;
Re: [PATCH] Support % wildcard in extension upgrade filenames
On Tue, Apr 11, 2023 at 04:36:04PM -0400, Regina Obe wrote: > > > Hey, best would be having support for wildcard wouldn't it ? > > I'm a woman of compromise. Sure 1 file would be ideal, but > I'd rather live with a big file listing all version upgrades > than 1000 files with the same information. Personally I don't see the benefit of 1 big file vs. many 0-length files to justify the cost (time and complexity) of a PostgreSQL change, with the corresponding cost of making use of this new functionality based on PostgreSQL version. We'd still have the problem of missing upgrade paths unless we release a new version of PostGIS even if it's ONLY for the sake of updating that 1 big file (or adding a new file, in the current situation). --strk;
Re: [PATCH] Support % wildcard in extension upgrade filenames
On Mon, Apr 10, 2023 at 11:09:40PM -0400, Regina Obe wrote: > > On Mon, Apr 03, 2023 at 09:26:25AM +0700, Yurii Rashkovskii wrote: > > > I want to chime in on the issue of lower-number releases that are > > > released after higher-number releases. The way I see this particular > > > problem is that we always put upgrade SQL files in release "packages," > > > and they obviously become static resources. > > > > > > While I [intentionally] overlook some details here, what if (as a > > > convention, for projects where it matters) we shipped extensions with > > > non-upgrade SQL files only, and upgrades were available as separate > > > downloads? This way, we're not tying releases themselves to upgrade paths. > > > This also requires no changes to Postgres. > > > > This is actually something that's on the plate, and we recently added a -- > > disable-extension-upgrades-install configure switch and a > > `install-extension- > > upgrades-from-known-versions` make target in PostGIS to help going in that > > direction. I guess the ball would now be in the hands of packagers. > > > > > I know this may be a big delivery layout departure for > > > well-established projects; I also understand that this won't solve the > > > problem of having to have these files in the first place (though in > > > many cases, they can be automatically generated once, I suppose, if they > > > are > > trivial). > > > > We will now also be providing a `postgis` script for administration that > > among > > other things will support a `install-extension-upgrades` command to install > > upgrade paths from specific old versions, but will not hard-code any list of > > "known" old versions as such a list will easily become outdated. > > > > Note that all upgrade scripts installed by the Makefile target or by the > > `postgis` scripts will only be empty upgrade paths from the source version > > to > > the fake "ANY" version, as the ANY-- upgrade path will still be the > > "catch-all" upgrade script. > > Sounds like a user and packaging nightmare to me. > How is a packager to know which versions a user might have installed? Isn't this EXACTLY the same problem WE have ? The problem is we cannot know in advance how many patch-level releases will come out, and we don't want to hurry into cutting a new release in branches NOT having a bug which was fixed in a stable branch... Packager might actually know better in that they could ONLY consider the packages ever packaged by them. Hey, best would be having support for wildcard wouldn't it ? > I much preferred the idea of just listing all our upgrade targets in the > control file. Again: how would we know all upgrade targets ? > We need to come up with a convention of how to describe a micro update, > as it's really a problem with extensions that follow the pattern I think it's a problem with extensions maintaining stable branches, as if the history was linear we would possibly need less files (although at this stage any number bigger than 1 would be too much for me) --strk;
Re: [PATCH] Support % wildcard in extension upgrade filenames
On Mon, Apr 03, 2023 at 09:26:25AM +0700, Yurii Rashkovskii wrote: > I want to chime in on the issue of lower-number releases that are released > after higher-number releases. The way I see this particular problem is that > we always put upgrade SQL files in release "packages," and they obviously > become static resources. > > While I [intentionally] overlook some details here, what if (as a > convention, for projects where it matters) we shipped extensions with > non-upgrade SQL files only, and upgrades were available as separate > downloads? This way, we're not tying releases themselves to upgrade paths. > This also requires no changes to Postgres. This is actually something that's on the plate, and we recently added a --disable-extension-upgrades-install configure switch and a `install-extension-upgrades-from-known-versions` make target in PostGIS to help going in that direction. I guess the ball would now be in the hands of packagers. > I know this may be a big delivery layout departure for well-established > projects; I also understand that this won't solve the problem of having to > have these files in the first place (though in many cases, they can be > automatically generated once, I suppose, if they are trivial). We will now also be providing a `postgis` script for administration that among other things will support a `install-extension-upgrades` command to install upgrade paths from specific old versions, but will not hard-code any list of "known" old versions as such a list will easily become outdated. Note that all upgrade scripts installed by the Makefile target or by the `postgis` scripts will only be empty upgrade paths from the source version to the fake "ANY" version, as the ANY-- upgrade path will still be the "catch-all" upgrade script. --strk; > On Tue, Jan 10, 2023 at 5:52 AM Tom Lane wrote: > > > I continue to think that this is a fundamentally bad idea. It creates > > all sorts of uncertainties about what is a valid update path and what > > is not. Restrictions like > > > > + Such wildcard update > > + scripts will only be used when no explicit path is found from > > + old to target version. > > > > are just band-aids to try to cover up the worst problems. > > > > Have you considered the idea of instead inventing a "\include" facility > > for extension scripts? Then, if you want to use one-monster-script > > to handle different upgrade cases, you still need one script file for > > each supported upgrade step, but those can be one-liners including the > > common script file. Plus, such a facility could be of use to people > > who want intermediate factorization solutions (that is, some sharing > > of code without buying all the way into one-monster-script). > > > > regards, tom lane > > > > > > > > -- > Y.
Re: [PATCH] Support % wildcard in extension upgrade filenames
On Mon, Mar 13, 2023 at 02:48:56PM -0400, Regina Obe wrote:
>
> I still see the main use-case as for those that micro version and for this
> use case, they would need a way, not necessarily to have a single upgrade
> script, but a script for each minor.
>
> So something like
>
> 3.2.%--3.4.0 = 3.2--3.4.0
I could implement this too if there's an agreement about it.
For now I'm attaching an updated patch with conflicts resolved.
--strk;
>From a10a1a7200f76bbc6e2def8d4684b647a99316cd Mon Sep 17 00:00:00 2001
From: Sandro Santilli
Date: Wed, 14 Sep 2022 11:10:10 +0200
Subject: [PATCH v2] Allow wildcard (%) in extension upgrade paths
A wildcard character "%" will be accepted in the
"source" side of the upgrade script and be considered
usable to upgrade any version to the "target" side.
Includes regression test and documentation.
---
doc/src/sgml/extend.sgml | 8
src/backend/commands/extension.c | 42 ---
src/test/modules/test_extensions/Makefile | 7 ++--
.../expected/test_extensions.out | 16 +++
src/test/modules/test_extensions/meson.build | 3 ++
.../test_extensions/sql/test_extensions.sql | 9
.../test_ext_wildcard1--%--2.0.sql| 6 +++
.../test_ext_wildcard1--1.0.sql | 6 +++
.../test_ext_wildcard1.control| 3 ++
9 files changed, 91 insertions(+), 9 deletions(-)
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1.control
diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml
index 218940ee5c..bdd463b81f 100644
--- a/doc/src/sgml/extend.sgml
+++ b/doc/src/sgml/extend.sgml
@@ -1120,6 +1120,14 @@ SELECT pg_catalog.pg_extension_config_dump('my_config', 'WHERE NOT standard_entr
1.1).
+
+ The literal value % can be used as the
+ old_version component in an extension
+ update script for it to match any version. Such wildcard update
+ scripts will only be used when no explicit path is found from
+ old to target version.
+
+
Given that a suitable update script is available, the command
ALTER EXTENSION UPDATE will update an installed extension
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 0eabe18335..36b6d7e01a 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -132,6 +132,7 @@ static void ApplyExtensionUpdates(Oid extensionOid,
bool cascade,
bool is_create);
static char *read_whole_file(const char *filename, int *length);
+static bool file_exists(const char *name);
/*
@@ -913,7 +914,14 @@ execute_extension_script(Oid extensionOid, ExtensionControlFile *control,
if (from_version == NULL)
elog(DEBUG1, "executing extension script for \"%s\" version '%s'", control->name, version);
else
+ {
+ if ( ! file_exists(filename) )
+ {
+ /* if filename does not exist, try wildcard */
+ filename = get_extension_script_filename(control, "%", version);
+ }
elog(DEBUG1, "executing extension script for \"%s\" update from version '%s' to '%s'", control->name, from_version, version);
+ }
/*
* If installing a trusted extension on behalf of a non-superuser, become
@@ -1258,14 +1266,19 @@ identify_update_path(ExtensionControlFile *control,
/* Find shortest path */
result = find_update_path(evi_list, evi_start, evi_target, false, false);
+ if (result != NIL)
+ return result;
- if (result == NIL)
- ereport(ERROR,
-(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("extension \"%s\" has no update path from version \"%s\" to version \"%s\"",
- control->name, oldVersion, newVersion)));
+ /* Find wildcard path, if no explicit path was found */
+ evi_start = get_ext_ver_info("%", &evi_list);
+ result = find_update_path(evi_list, evi_start, evi_target, false, false);
+ if (result != NIL)
+ return result;
- return result;
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("extension \"%s\" has no update path from version \"%s\" to version \"%s\"",
+ control->name, oldVersion, newVersion)));
}
/*
@@ -3470,3 +3483,20 @@ read_whole_file(const char *filename, int *length)
buf[*length] = '\0';
return buf;
}
+
+static bool
+file_exists(const char *name)
+{
+ struct stat st;
+
+ Assert(name != NULL);
+
+ if (stat(name, &st) == 0)
+ return !S_ISDIR(st.st_mode);
+ else if (!(errno == ENOENT || errno == ENOTDIR || errno == EACCES))
+ ereport(ERROR,
+(errcode_for_file_access(),
+
Re: Ability to reference other extensions by schema in extension scripts
On Mon, Mar 13, 2023 at 05:57:57PM -0400, Regina Obe wrote:
>
> Attached is a slightly revised patch to fix the extra whitespace in the
> extend.gml document that Sandro noted to me.
Thanks Regina.
I've tested attached patch (md5 0b652a8271fc7e71ed5f712ac162a0ef)
against current master (hash 4ef1be5a0b676a9f030cc2e4837f4b5650ecb069).
The patch applies cleanly, builds cleanly, regresses cleanly.
I've also run my quick test and I'm satisfied with it:
test=# create extension ext2 cascade;
NOTICE: installing required extension "ext1"
CREATE EXTENSION
test=# select ext2log('h');
ext1: ext2: h
test=# alter extension ext1 set schema n1;
ERROR: cannot SET SCHEMA of extension ext1 because other extensions prevent
it
DETAIL: extension ext2 prevents relocation of extension ext1
test=# drop extension ext2;
DROP EXTENSION
test=# alter extension ext1 set schema n1;
ALTER EXTENSION
test=# create extension ext2;
CREATE EXTENSION
test=# select ext2log('h');
ext1: ext2: h
--strk;
Re: [PATCH] Support % wildcard in extension upgrade filenames
On Wed, Mar 08, 2023 at 03:18:06PM -0500, Regina Obe wrote: > > Then question arises if you have such a map file and you have files as well > (the old way). One idea I had in the past about the .control file was to advertise an executable that would take current version and next version and return a list of paths to SQL files to use to upgrade. Then our script could decide what to do, w/out Tom looking :P --strk;
Re: [PATCH] Support % wildcard in extension upgrade filenames
On Wed, Mar 08, 2023 at 08:27:29AM -0500, Robert Haas wrote: > I wonder if a solution to this problem might be to provide some kind > of a version map file. Let's suppose that the map file is a bunch of > lines of the form X Y Z, where X, Y, and Z are version numbers. The > semantics could be: we (the extension authors) promise that if you > want to upgrade from X to Z, it suffices to run the script that knows > how to upgrade from Y to Z. > This would address Tom's concern, because > if you write a master upgrade script, you have to explicitly declare > the versions to which it applies. This would just move the problem from having 1968 files to having to write 1968 lines in control files, and does not solve the problem of allowing people with an hot-patched old version not being able to upgrade to a newer (bug-free) version released before the hot-patch. We maintain multiple stable branches (5, at the moment: 2.5, 3.0, 3.1, 3.2, 3.3) and would like to allow anyone running an old patched version to still upgrade to a newer version released earlier. --strk; Libre GIS consultant/developer https://strk.kbt.io/services.html
Re: Ability to reference other extensions by schema in extension scripts
On Sat, Mar 11, 2023 at 03:18:18AM -0500, Regina Obe wrote: > Attached is a revised patch with these changes in place. I've given a try to this patch. It builds and regresses fine. My own tests also worked fine. As long as ext1 was found in the ext2's no_relocate list it could not be relocated, and proper error message is given to user trying it. Nitpicking, there are a few things that are weird to me: 1) I don't get any error/warning if I put an arbitrary string into no_relocate (there's no check to verify the no_relocate is a subset of the requires). 2) An extension can still reference extensions it depends on without putting them in no_relocate. This may be intentional, as some substitutions may not require blocking relocation, but felt inconsistent with the normal @extschema@ which is never replaced unless an extension is marked as non-relocatable. --strk; Libre GIS consultant/developer https://strk.kbt.io/services.html
Re: [PATCH] Support % wildcard in extension upgrade filenames
On Tue, Mar 07, 2023 at 02:13:07PM -0500, Tom Lane wrote: > What I am maintaining is that no extension author is actually going > to write such a script, indeed they probably won't trouble to write > any downgrade-like actions at all. Which makes the proposed design > mostly a foot-gun. What I'm maintaining is that such authors should be warned about the risk, and discouraged from installing any wildcard-containing script UNLESS they deal with downgrade protection. PostGIS does deal with that kind of protection (yes, could be helped somehow in doing that by PostgreSQL). > I'm not unsympathetic to the idea of trying to support multiple upgrade > paths in one script. I just don't like this particular design for that, > because it requires the extension author to make promises that nobody > is actually going to deliver on. Would you be ok with a stricter pattern matching ? Something like: postgis--3.3.%--3.3.ANY.sql postgis--3.3.ANY--3.4.0.sql Would that be easier to promise something about ? --strk;
Re: [PATCH] Support % wildcard in extension upgrade filenames
On Tue, Mar 07, 2023 at 12:39:34PM -0500, Robert Haas wrote: > The thing that confuses me here is why the PostGIS folks are ending up > with so many files. We want to allow PostGIS users to upgrade from ANY previous version, because different distribution or user habit may result in people upgrading from ANY older release to the newest. Now, PostGIS has released a total of 164 versions: curl -s https://download.osgeo.org/postgis/source/ | grep -c '.tar.gz<' In *addition* to these, most developers end up installing a "dev" suffixed version to distinguish it from the official releases, which means there's a total of 164*2 = 328 version *per extension*. Now, PostGIS doesn't install a single extension but multiple ones: - postgis - postgis_raster - postgis_topology - postgis_sfcgal - postgis_tiger_geocoder - address_standardizer So we'd be up to 328 * 6 = 1968 files needed to upgrade from ANY postgis extension version to CURRENT version. The numbers are slightly less because not all versions of PostGIS did support EXTENSION (wasn't even available in early PostGIS versions) > We certainly don't have that problem with the > extension that are being maintained in contrib, and I guess there is > some difference in versioning practice that is making it an issue for > them but not for us. I wish I understood what was going on there. Our extension versioning has 3 numbers, but PostgreSQL doesn't really care about this right ? It's just that we've had 328 different versions released and more are to come, and we want users to be able to upgrade from each one of them. I guess you may suggest that we do NOT increas the *EXTENSION* version number UNLESS something really changes at SQL level, but honestly I doubt we EVER released a version with no changes at the SQL level (maybe in some occasion for really bad bugs which were only fixed at the C level). --strk;
Re: [PATCH] Support % wildcard in extension upgrade filenames
On Mon, Mar 06, 2023 at 02:54:35PM -0500, Gregory Stark (as CFM) wrote:
> This patch too is conflicting on meson.build.
I'm attaching a rebased version to this email.
> Maybe it can just be solved with multiple one-line scripts
> that call to a master script?
Not really, as the problem we are trying to solve is the need
to install many files, and the need to foresee any possible
future bug-fix release we might want to support upgrades from.
PostGIS is already installing zero-line scripts to upgrade
from to a virtual "ANY" version which we then
use to have a single ANY-- upgrade path, but we
are still REQUIRED to install a file for any we
want to allow upgrade from, which is what this patch is aimed
at fixing.
--strk;
>From ffefd039f24e3def8d2216b3ac7875d0b6feb8fb Mon Sep 17 00:00:00 2001
From: Sandro Santilli
Date: Wed, 14 Sep 2022 11:10:10 +0200
Subject: [PATCH v1] Allow wildcard (%) in extension upgrade paths
A wildcard character "%" will be accepted in the
"source" side of the upgrade script and be considered
usable to upgrade any version to the "target" side.
Includes regression test and documentation.
---
doc/src/sgml/extend.sgml | 8
src/backend/commands/extension.c | 42 ---
src/test/modules/test_extensions/Makefile | 6 ++-
.../expected/test_extensions.out | 15 +++
src/test/modules/test_extensions/meson.build | 3 ++
.../test_extensions/sql/test_extensions.sql | 7
.../test_ext_wildcard1--%--2.0.sql| 6 +++
.../test_ext_wildcard1--1.0.sql | 6 +++
.../test_ext_wildcard1.control| 3 ++
9 files changed, 88 insertions(+), 8 deletions(-)
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1.control
diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml
index b70cbe83ae..f1f0ae1244 100644
--- a/doc/src/sgml/extend.sgml
+++ b/doc/src/sgml/extend.sgml
@@ -1081,6 +1081,14 @@ SELECT pg_catalog.pg_extension_config_dump('my_config', 'WHERE NOT standard_entr
1.1).
+
+ The literal value % can be used as the
+ old_version component in an extension
+ update script for it to match any version. Such wildcard update
+ scripts will only be used when no explicit path is found from
+ old to target version.
+
+
Given that a suitable update script is available, the command
ALTER EXTENSION UPDATE will update an installed extension
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 02ff4a9a7f..6df0fd403a 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -130,6 +130,7 @@ static void ApplyExtensionUpdates(Oid extensionOid,
bool cascade,
bool is_create);
static char *read_whole_file(const char *filename, int *length);
+static bool file_exists(const char *name);
/*
@@ -893,7 +894,14 @@ execute_extension_script(Oid extensionOid, ExtensionControlFile *control,
if (from_version == NULL)
elog(DEBUG1, "executing extension script for \"%s\" version '%s'", control->name, version);
else
+ {
+ if ( ! file_exists(filename) )
+ {
+ /* if filename does not exist, try wildcard */
+ filename = get_extension_script_filename(control, "%", version);
+ }
elog(DEBUG1, "executing extension script for \"%s\" update from version '%s' to '%s'", control->name, from_version, version);
+ }
/*
* If installing a trusted extension on behalf of a non-superuser, become
@@ -1217,14 +1225,19 @@ identify_update_path(ExtensionControlFile *control,
/* Find shortest path */
result = find_update_path(evi_list, evi_start, evi_target, false, false);
+ if (result != NIL)
+ return result;
- if (result == NIL)
- ereport(ERROR,
-(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("extension \"%s\" has no update path from version \"%s\" to version \"%s\"",
- control->name, oldVersion, newVersion)));
+ /* Find wildcard path, if no explicit path was found */
+ evi_start = get_ext_ver_info("%", &evi_list);
+ result = find_update_path(evi_list, evi_start, evi_target, false, false);
+ if (result != NIL)
+ return result;
- return result;
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("extension \"%s\" has no update path from version \"%s\" to version \"%s\"",
+ control->name, oldVersion, newVersion)));
}
/*
@@ -3395,3 +3408,20 @@ read_whole_file(const char *filename, int *length)
buf[*length] = '\0';
return buf;
}
+
+s
Re: Ability to reference other extensions by schema in extension scripts
The following review has been posted through the commitfest application: make installcheck-world: tested, failed Implements feature: tested, passed Spec compliant: tested, passed Documentation:tested, passed I've applied the patch attached to message https://www.postgresql.org/message-id/000401d94bc8%2448dff700%24da9fe500%24%40pcorp.us (md5sum a7d45a32c054919d94cd4a26d7d07c20) onto current tip of the master branch being 128dd9f9eca0b633b51ffcd5b0f798fbc48ec4c0 The review written in https://www.postgresql.org/message-id/20230228224608.ak7br5shev4wic5a%40c19 all still applies. The `make installcheck-world` test fails for me but the failures seem unrelated to the patch (many occurrences of "+ERROR: function pg_input_error_info(unknown, unknown) does not exist" in the regression.diff). Documentation exists for the new feature The new status of this patch is: Ready for Committer
Re: Ability to reference other extensions by schema in extension scripts
On Sun, Feb 26, 2023 at 01:39:24AM -0500, Regina Obe wrote:
> > 1) Just don't allow any extensions referenced by other
> >extensions to be relocatable.
>
> Attached is my revision 3 patch, which follows the proposed #1.
> Don't allow schema relocation of an extension if another extension
> requires it.
I've built a version of PostgreSQL with this patch applied and I
confirm it works as expected.
The "ext1" is relocatable and creates a function ext1log():
=# create extension ext1 schema n1;
CREATE EXTENSION
The "ext2" is relocatable and creates a function ext2log() relying
on the ext1log() function from "ext1" extension, referencing
it via @extschema:ext1@:
=# create extension ext2 schema n2;
CREATE EXTENSION
=# select n2.ext2log('hello'); -- things work here
ext1: ext2: hello
By creating "ext2", "ext1" becomes effectively non-relocatable:
=# alter extension ext1 set schema n2;
ERROR: cannot SET SCHEMA of extension ext1 because other extensions
require it
DETAIL: extension ext2 requires extension ext1
Drop "ext2" makes "ext1" relocatable again:
=# drop extension ext2;
DROP EXTENSION
=# alter extension ext1 set schema n2;
ALTER EXTENSION
Upon re-creating "ext2" the referenced ext1 schema will be
the correct one:
=# create extension ext2 schema n1;
CREATE EXTENSION
=# select n1.ext2log('hello');
ext1: ext2: hello
The code itself builds w/out warnings with:
mkdir build
cd build
../configure
make 2> ERR # ERR is empty
The testsuite reports all successes:
make check
[...]
===
All 213 tests passed.
===
Since I didn't see the tests for extension in there, I've also
explicitly run that portion:
make -C src/test/modules/test_extensions/ check
[...]
test test_extensions ... ok 32 ms
test test_extdepend ... ok 12 ms
[...]
=
All 2 tests passed.
=
As mentioned already the downside of this patch is that it would
not be possibile to change the schema of an otherwise relocatable
extension once other extension depend on it, but I can't think of
any good reason to allow that, as it would mean dependent code
would need to always dynamically determine the install location
of the objects in that extension, which sounds dangerous, security
wise.
--strk;
Libre GIS consultant/developer
https://strk.kbt.io/services.html
Re: Ability to reference other extensions by schema in extension scripts
On Sat, Feb 25, 2023 at 03:40:24PM -0500, Regina Obe wrote: > > On Mon, Feb 06, 2023 at 05:19:39AM -0500, Regina Obe wrote: > > > > I was thinking: how about using the "refobjsubid" to encode the "level" of > > dependency on an extension ? Right now "refobjsubid" is always 0 when the > > referenced object is an extension. > > Could we consider subid=1 to mean the dependency is not only on the > > extension but ALSO on it's schema location ? > > I like that idea. It's only been ever used for tables I think, but I don't > see why it wouldn't apply in this case as the concept is kinda the same. > Only concern if other parts rely on this being 0. This has to be verified, yes. But it feels to me like "must be 0" was mostly to _allow_ for future extensions like the proposed one. > The other question, should this just update the existing DEPENDENCY_NORMAL > extension or add a new DEPENDENCY_NORMAL between the extensions with > subid=1? I'd use the existing record. --strk; Libre GIS consultant/developer https://strk.kbt.io/services.html
Re: Ability to reference other extensions by schema in extension scripts
On Mon, Feb 06, 2023 at 05:19:39AM -0500, Regina Obe wrote: > > Attached is a revised version of the original patch. It is revised to > prevent > > ALTER EXTENSION .. SET SCHEMA if there is a dependent extension that > references the extension in their scripts using @extschema:extensionname@ > It also adds additional tests to verify that new feature. > > In going thru the code base, I was tempted to add a new dependency type > instead of using the existing DEPENDENCY_AUTO. I think this would be > cleaner, but I felt that was overstepping the area a bit, since it requires > making changes to dependency.h and dependency.c > > My main concern with using DEPENDENCY_AUTO is because it was designed for > cases where an object can be dropped without need for CASCADE. In this > case, we don't want a dependent extension to be dropped if it's required is > dropped. However since there will already exist > a DEPENDENCY_NORMAL between the 2 extensions, I figure we are protected > against that issue already. I was thinking: how about using the "refobjsubid" to encode the "level" of dependency on an extension ? Right now "refobjsubid" is always 0 when the referenced object is an extension. Could we consider subid=1 to mean the dependency is not only on the extension but ALSO on it's schema location ? Also: should we really allow extensions to rely on other extension w/out fully-qualifying calls to their functions ? Or should it be discouraged and thus forbidden ? If we wanted to forbid it we then would not need to encode any additional dependency but rather always forbid `ALTER EXTENSION .. SET SCHEMA` whenever the extension is a dependency of any other extension. On the code in the patch itself, I tried with this simple use case: - ext1, relocatable, exposes an ext1log(text) function - ext2, relocatable, exposes an ext2log(text) function calling @extschema:ext1@.ext1log() What is not good: - Drop of ext1 automatically cascades to drop of ext2 without even a notice: test=# create extension ext2 cascade; NOTICE: installing required extension "ext1" CREATE EXTENSION test=# drop extension ext1; DROP EXTENSION -- no WARNING, no NOTICE, ext2 is gone What is good: - ext1 cannot be relocated while ext2 is loaded: test=# create extension ext2 cascade; NOTICE: installing required extension "ext1" CREATE EXTENSION test=# alter extension ext1 set schema n1; ERROR: Extension can not be relocated because dependent extension references it's location test=# drop extension ext2; DROP EXTENSION test=# alter extension ext1 set schema n1; ALTER EXTENSION --strk; Libre GIS consultant/developer https://strk.kbt.io/services.html
Re: Ability to reference other extensions by schema in extension scripts
On Mon, Jan 16, 2023 at 11:57:30PM -0500, Regina Obe wrote:
> What would be more bullet-proof is having an extra column in pg_extension or
> adding an extra array element to pg_extension.extcondition[] that allows you
> to say "Hey, don't allow this to be relocatable cause other extensions
> depend on it that have explicitly referenced the schema."
I've given this some more thoughts and I think a good
compromise could be to add the safety net in ALTER EXTESION SET SCHEMA
so that it does not only check "extrelocatable" but also the presence
of any extension effectively depending on it, in which case the
operation could be prevented with a more useful message than
"extension does not support SET SCHEMA" (what is currently output).
Example query to determine those cases:
SELECT e.extname, array_agg(v.name)
FROM pg_extension e, pg_available_extension_versions v
WHERE e.extname = ANY( v.requires )
AND e.extrelocatable
AND v.installed group by e.extname;
extname|array_agg
---+--
fuzzystrmatch | {postgis_tiger_geocoder}
--strk;
Re: Ability to reference other extensions by schema in extension scripts
On Thu, Dec 15, 2022 at 08:04:22AM -0500, Regina Obe wrote: > > On Tue, Nov 22, 2022 at 11:24:19PM -0500, Regina Obe wrote: > > > > > If an extension is required by another extension and that required > > > extension schema is referenced in the extension scripts using the > > > @extschema:extensionname@ syntax, then ideally we should prevent the > > > required extension from being relocatable. This would prevent a user > > > from accidentally moving the required extension, thus breaking the > > > dependent extensions. > > > > > > I didn't add that feature cause I wasn't sure if it was overstepping > > > the bounds of what should be done, or if we leave it up to the user to > > > just know better. > > > > An alternative would be to forbid using @extschema:extensionname@ to > > reference relocatable extensions. DBA can toggle relocatability of an > > extension to allow it to be referenced. > > That would be hard to do in a DbaaS setup and not many users know they can > fiddle with extension control files. > Plus those would get overwritten with upgrades. Wouldn't this also be the case if you override relocatability ? Case: - Install fuzzystrmatch, marked as relocatable - Install ext2 depending on the former, which is them marked non-relocatable - Upgrade database -> fuzzystrmatch becomes relocatable again - Change fuzzystrmatch schema BREAKING ext2 Allowing to relocate a dependency of other extensions using the @extschema@ syntax is very dangerous. I've seen that PostgreSQL itself doesn't even bother to replace @extschema@ IF the extension using it doesn't mark itself as non-relocatable. For consistency this patch should basically refuse to expand @extschema:fuzzystrmatch@ if "fuzzystrmatch" extension is relocatable. Changing the current behaviour of PostgreSQL could be proposed but I don't think it's to be done in this thread ? So my suggestion is to start consistent (do not expand if referenced extension is relocatable). --strk;
Re: [PATCH] Support % wildcard in extension upgrade filenames
On Tue, Jan 10, 2023 at 11:09:23PM -0500, Regina Obe wrote: > The only way we can fix that in the current setup, is to move to a minor > version mode which means we can > never do micro updates. Or just not with standard PostgreSQL syntax, because we can of course do upgrades using the `SELECT postgis_extensions_upgrade()` call at the moment, which, if you ask me, sounds MORE dangerous than the wildcard upgrade approach because the _implementation_ of that function will always be the OLD implementation, never the NEW one, so if bogus cannot be fixed by a new release w/out a way to upgrade there... --strk;
Re: [PATCH] Support % wildcard in extension upgrade filenames
On Tue, Jan 10, 2023 at 06:50:31PM -0500, Tom Lane wrote: > With the proposed % feature, if foo--%--3.0.sql exists then the > system will invoke it and expect the end result to be a valid > 3.0 installation, whether or not the script actually has any > ability to do a downgrade. It is sane, for the system to expect the end result to be a valid 3.0 installation if no exception is thrown by the script itself. If we ship foo--%--3.0.sql we must have been taken care of protecting from unsupported downgrades/upgrades (and we did, having the script throw an exception if anything is unexpected). > (I suppose you could add code to look at pg_extension.extversion, We actually added code looking at our own version-extracting function (which existed since before PostgreSQL added support for extensions). Is the function not there ? Raise an exception. Is the function not owned by the extension ? Raise an exception. In other cases -> trust the output of that function to tell what version we're coming from, throw an exception if upgrade to the target version is unsupported. > to add such code is that really better than having a number of > one-liner scripts implementing the same check declaratively?) Yes, it is, because no matter how many one-liner scripts we install (we currently install 87 * 6 such scripts, we always end up missing some of them upon releasing a new bug-fix release in stable branches. > almost certainly you are > going to end with an extension containing some leftover 4.0 > objects, some 3.0 objects, and maybe some objects with properties > that don't exactly match either 3.0 or 4.0. This is already possible, depending on WHO writes those upgrade scripts. This proposal just gives more expressiveness power to those script authors. > So I really think this is a case of "be careful what you ask > for, you might get it". Even if PostGIS is willing to put in > the amount of infrastructure legwork needed to make such a > design bulletproof, I'm quite sure nobody else will manage > to use such a thing successfully. This is why I initially made this something to be explicitly enabled by the .control file, which I can do again if it feels safer for you. > I'd rather spend our > development effort on a feature that has more than one use-case. Use case is any extension willing to support more than a single stable branch while still allowing upgrading from newer-stable-bugfix-release to older-feature-release (ie: 3.2.10 -> 3.4.0 ). Does not seem so uncommon to me, for a big project. Maybe there aren't enough big extension-based projects out there ? --strk; Libre GIS consultant/developer https://strk.kbt.io/services.html
Re: [PATCH] Support % wildcard in extension upgrade filenames
On Mon, Jan 09, 2023 at 05:51:49PM -0500, Tom Lane wrote: > Have you considered the idea of instead inventing a "\include" facility [...] > cases, you still need one script file for each supported upgrade step That's exactly the problem we're trying to solve here. The include support is nice on itself, but won't solve our problem. --strk;
Re: Ability to reference other extensions by schema in extension scripts
On Tue, Nov 22, 2022 at 11:24:19PM -0500, Regina Obe wrote: > Here is first version of my patch using the @extschema:extensionname@ syntax > you proposed. > > This patch includes: > 1) Changes to replace references of @extschema:extensionname@ with the > schema of the required extension > 2) Documentation for the feature > 3) Tests for the feature. > > There is one issue I thought about that is not addressed by this. > > If an extension is required by another extension and that required extension > schema is referenced in the extension scripts using the > @extschema:extensionname@ syntax, then ideally we should prevent the > required extension from being relocatable. This would prevent a user from > accidentally moving the required extension, thus breaking the dependent > extensions. > > I didn't add that feature cause I wasn't sure if it was overstepping the > bounds of what should be done, or if we leave it up to the user to just know > better. An alternative would be to forbid using @extschema:extensionname@ to reference relocatable extensions. DBA can toggle relocatability of an extension to allow it to be referenced. --strk;
Re: [PATCH] Support % wildcard in extension upgrade filenames
On Wed, Nov 16, 2022 at 05:25:07PM -0600, Justin Pryzby wrote:
> Note that the patch is passing tests when using autoconf build but
> failing for meson builds.
Thanks for pointing me to the right direction.
I'm attaching an updated patch, will keep an eye on cirrusCI
--strk;
>From 344f2b96c172ed8c75990ecb86e78494c82df54d Mon Sep 17 00:00:00 2001
From: Sandro Santilli
Date: Wed, 14 Sep 2022 11:10:10 +0200
Subject: [PATCH] Allow wildcard (%) in extension upgrade paths
A wildcard character "%" will be accepted in the
"source" side of the upgrade script and be considered
usable to upgrade any version to the "target" side.
Includes regression test and documentation.
---
doc/src/sgml/extend.sgml | 8
src/backend/commands/extension.c | 42 ---
src/test/modules/test_extensions/Makefile | 6 ++-
.../expected/test_extensions.out | 15 +++
src/test/modules/test_extensions/meson.build | 3 ++
.../test_extensions/sql/test_extensions.sql | 7
.../test_ext_wildcard1--%--2.0.sql| 6 +++
.../test_ext_wildcard1--1.0.sql | 6 +++
.../test_ext_wildcard1.control| 3 ++
9 files changed, 88 insertions(+), 8 deletions(-)
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1.control
diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml
index 46e873a166..c79140f669 100644
--- a/doc/src/sgml/extend.sgml
+++ b/doc/src/sgml/extend.sgml
@@ -1081,6 +1081,14 @@ SELECT pg_catalog.pg_extension_config_dump('my_config', 'WHERE NOT standard_entr
1.1).
+
+ The literal value % can be used as the
+ old_version component in an extension
+ update script for it to match any version. Such wildcard update
+ scripts will only be used when no explicit path is found from
+ old to target version.
+
+
Given that a suitable update script is available, the command
ALTER EXTENSION UPDATE will update an installed extension
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 1a62e5dac5..e3ea9dba30 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -128,6 +128,7 @@ static void ApplyExtensionUpdates(Oid extensionOid,
bool cascade,
bool is_create);
static char *read_whole_file(const char *filename, int *length);
+static bool file_exists(const char *name);
/*
@@ -890,7 +891,14 @@ execute_extension_script(Oid extensionOid, ExtensionControlFile *control,
if (from_version == NULL)
elog(DEBUG1, "executing extension script for \"%s\" version '%s'", control->name, version);
else
+ {
+ if ( ! file_exists(filename) )
+ {
+ /* if filename does not exist, try wildcard */
+ filename = get_extension_script_filename(control, "%", version);
+ }
elog(DEBUG1, "executing extension script for \"%s\" update from version '%s' to '%s'", control->name, from_version, version);
+ }
/*
* If installing a trusted extension on behalf of a non-superuser, become
@@ -1214,14 +1222,19 @@ identify_update_path(ExtensionControlFile *control,
/* Find shortest path */
result = find_update_path(evi_list, evi_start, evi_target, false, false);
+ if (result != NIL)
+ return result;
- if (result == NIL)
- ereport(ERROR,
-(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("extension \"%s\" has no update path from version \"%s\" to version \"%s\"",
- control->name, oldVersion, newVersion)));
+ /* Find wildcard path, if no explicit path was found */
+ evi_start = get_ext_ver_info("%", &evi_list);
+ result = find_update_path(evi_list, evi_start, evi_target, false, false);
+ if (result != NIL)
+ return result;
- return result;
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("extension \"%s\" has no update path from version \"%s\" to version \"%s\"",
+ control->name, oldVersion, newVersion)));
}
/*
@@ -3392,3 +3405,20 @@ read_whole_file(const char *filename, int *length)
buf[*length] = '\0';
return buf;
}
+
+static bool
+file_exists(const char *name)
+{
+ struct stat st;
+
+ Assert(name != NULL);
+
+ if (stat(name, &st) == 0)
+ return !S_ISDIR(st.st_mode);
+ else if (!(errno == ENOENT || errno == ENOTDIR || errno == EACCES))
+ ereport(ERROR,
+(errcode_for_file_access(),
+ errmsg("could not access file \"%s\": %m", name)));
+
+ return false;
+}
diff --git a/src/test/modules/test_extensions/Makefile b/src/test/modules/test_extensions/Makefile
index c
Re: [PATCH] Support % wildcard in extension upgrade filenames
On Sun, Nov 13, 2022 at 11:46:50PM -0500, Regina Obe wrote:
> > Re: Sandro Santilli
> > > I'm attaching an updated version of the patch. This time the patch is
> > > tested. Nothing changes unless the .control file for the subject
> > > extension doesn't have a "wildcard_upgrades = true" statement.
> > >
> > > When wildcard upgrades are enabled, a file with a "%" symbol as the
> > > "source" part of the upgrade path will match any version and
> >
> > Fwiw I believe wildcard_upgrades isn't necessary in the .control file.
> > If there are no % files, none would be used anyway, and if there are, it's
> clear
> > it's meant as wildcard since % won't appear in any remotely sane version
> > number.
>
> I also like the idea of skipping the wildcard_upgrades syntax.
> Then there is no need to have a conditional control file for PG 16 vs. older
> versions.
Here we go. Attached a version of the patch with no
"wildcard_upgrades" controlling it.
--strk;
>From 9b138eae95e0d389bee3776247ba9d7d5144bcc5 Mon Sep 17 00:00:00 2001
From: Sandro Santilli
Date: Wed, 14 Sep 2022 11:10:10 +0200
Subject: [PATCH] Allow wildcard (%) in extension upgrade paths
A wildcard character "%" will be accepted in the
"source" side of the upgrade script and be considered
usable to upgrade any version to the "target" side.
Includes regression test and documentation.
---
doc/src/sgml/extend.sgml | 8
src/backend/commands/extension.c | 42 ---
src/test/modules/test_extensions/Makefile | 6 ++-
.../expected/test_extensions.out | 15 +++
.../test_extensions/sql/test_extensions.sql | 7
.../test_ext_wildcard1--%--2.0.sql| 6 +++
.../test_ext_wildcard1--1.0.sql | 6 +++
.../test_ext_wildcard1.control| 3 ++
8 files changed, 85 insertions(+), 8 deletions(-)
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1.control
diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml
index 46e873a166..c79140f669 100644
--- a/doc/src/sgml/extend.sgml
+++ b/doc/src/sgml/extend.sgml
@@ -1081,6 +1081,14 @@ SELECT pg_catalog.pg_extension_config_dump('my_config', 'WHERE NOT standard_entr
1.1).
+
+ The literal value % can be used as the
+ old_version component in an extension
+ update script for it to match any version. Such wildcard update
+ scripts will only be used when no explicit path is found from
+ old to target version.
+
+
Given that a suitable update script is available, the command
ALTER EXTENSION UPDATE will update an installed extension
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 1a62e5dac5..e3ea9dba30 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -128,6 +128,7 @@ static void ApplyExtensionUpdates(Oid extensionOid,
bool cascade,
bool is_create);
static char *read_whole_file(const char *filename, int *length);
+static bool file_exists(const char *name);
/*
@@ -890,7 +891,14 @@ execute_extension_script(Oid extensionOid, ExtensionControlFile *control,
if (from_version == NULL)
elog(DEBUG1, "executing extension script for \"%s\" version '%s'", control->name, version);
else
+ {
+ if ( ! file_exists(filename) )
+ {
+ /* if filename does not exist, try wildcard */
+ filename = get_extension_script_filename(control, "%", version);
+ }
elog(DEBUG1, "executing extension script for \"%s\" update from version '%s' to '%s'", control->name, from_version, version);
+ }
/*
* If installing a trusted extension on behalf of a non-superuser, become
@@ -1214,14 +1222,19 @@ identify_update_path(ExtensionControlFile *control,
/* Find shortest path */
result = find_update_path(evi_list, evi_start, evi_target, false, false);
+ if (result != NIL)
+ return result;
- if (result == NIL)
- ereport(ERROR,
-(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("extension \"%s\" has no update path from version \"%s\" to version \"%s\"",
- control->name, oldVersion, newVersion)));
+ /* Find wildcard path, if no explicit path was found */
+ evi_start = get_ext_ver_info("%", &evi_list);
+ result = find_update_path(evi_list, evi_start, evi_target, false, false);
+ if (result != NIL)
+ return result;
- return result;
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errm
Re: [PATCH] Support % wildcard in extension upgrade filenames
And now a version of the patch including documentation and regression tests.
Anything you see I should improve ?
--strk;
On Fri, Nov 04, 2022 at 06:31:58PM +0100, Sandro Santilli wrote:
> On Mon, Oct 31, 2022 at 01:55:05AM -0400, Regina Obe wrote:
> >
> > Sandro, can you submit an updated version of this patch.
> > I was testing it out and looked good first time.
>
> Attached updated version of the patch.
>
> > If everyone is okay with this patch, we'll go ahead and add tests and
> > documentation to go with it.
>
> Yes please let us know if there's any chance of getting this
> mechanism approved or if we should keep advertising works around
> this limitation (it's not just installing 100s of files but also
> still missing needed upgrade paths when doing so).
>
>
> --strk;
>From 4b10297315d2db4365e6f47e375588394b260d0d Mon Sep 17 00:00:00 2001
From: Sandro Santilli
Date: Wed, 14 Sep 2022 11:10:10 +0200
Subject: [PATCH] Allow wildcard (%) in extension upgrade paths
A wildcard character "%" will be accepted in the
"source" side of the upgrade script and be considered
usable to upgrade any version to the "target" side.
Using wildcards needs to be explicitly requested by
extensions via a "wildcard_upgrades" setting in their
control file.
Includes regression test and documentation.
---
doc/src/sgml/extend.sgml | 14 +
src/backend/commands/extension.c | 58 +--
src/test/modules/test_extensions/Makefile | 6 +-
.../expected/test_extensions.out | 15 +
.../test_extensions/sql/test_extensions.sql | 7 +++
.../test_ext_wildcard1--%--2.0.sql| 6 ++
.../test_ext_wildcard1--1.0.sql | 6 ++
.../test_ext_wildcard1.control| 4 ++
8 files changed, 108 insertions(+), 8 deletions(-)
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1.control
diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml
index 46e873a166..4012652574 100644
--- a/doc/src/sgml/extend.sgml
+++ b/doc/src/sgml/extend.sgml
@@ -807,6 +807,20 @@ RETURNS anycompatible AS ...
+
+
+ wildcard_upgrades (boolean)
+
+
+This parameter, if set to true (which is not the
+default), allows ALTER EXTENSION to consider
+a wildcard character % as matching any version of
+the extension. Such wildcard match will only be used when no
+perfect match is found for a version.
+
+
+
+
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 1a62e5dac5..ea8825fcff 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -86,6 +86,7 @@ typedef struct ExtensionControlFile
bool relocatable; /* is ALTER EXTENSION SET SCHEMA supported? */
bool superuser; /* must be superuser to install? */
bool trusted; /* allow becoming superuser on the fly? */
+ bool wildcard_upgrades; /* allow using wildcards in upgrade scripts */
int encoding; /* encoding of the script file, or -1 */
List *requires; /* names of prerequisite extensions */
} ExtensionControlFile;
@@ -128,6 +129,7 @@ static void ApplyExtensionUpdates(Oid extensionOid,
bool cascade,
bool is_create);
static char *read_whole_file(const char *filename, int *length);
+static bool file_exists(const char *name);
/*
@@ -579,6 +581,14 @@ parse_extension_control_file(ExtensionControlFile *control,
errmsg("parameter \"%s\" requires a Boolean value",
item->name)));
}
+ else if (strcmp(item->name, "wildcard_upgrades") == 0)
+ {
+ if (!parse_bool(item->value, &control->wildcard_upgrades))
+ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("parameter \"%s\" requires a Boolean value",
+item->name)));
+ }
else if (strcmp(item->name, "encoding") == 0)
{
control->encoding = pg_valid_server_encoding(item->value);
@@ -636,6 +646,7 @@ read_extension_control_file(const char *extname)
control->relocatable = false;
control->superuser = true;
control->trusted = false;
+ control->wildcard_upgrades = false;
control->encoding = -1;
/*
@@ -890,7 +901,15 @@ execute_extension_script(Oid extensionOid, ExtensionControlFile *control,
if (from_version == NULL)
elog(DEBUG1, "executing extension script for \"%s\" version '%s'", control->name, version);
else
+ {
+ if ( control->wildcard_upgrades && ! file_exists(filename) )
+ {
+ elog(DEBUG1, "ex
Re: [PATCH] Support % wildcard in extension upgrade filenames
On Mon, Oct 31, 2022 at 01:55:05AM -0400, Regina Obe wrote:
>
> Sandro, can you submit an updated version of this patch.
> I was testing it out and looked good first time.
Attached updated version of the patch.
> If everyone is okay with this patch, we'll go ahead and add tests and
> documentation to go with it.
Yes please let us know if there's any chance of getting this
mechanism approved or if we should keep advertising works around
this limitation (it's not just installing 100s of files but also
still missing needed upgrade paths when doing so).
--strk;
>From 8725c616c31a9bc25478c7128ea81ce753e51089 Mon Sep 17 00:00:00 2001
From: Sandro Santilli
Date: Wed, 14 Sep 2022 11:10:10 +0200
Subject: [PATCH] Allow wildcard (%) in extension upgrade paths
A wildcard character "%" will be accepted in the
"source" side of the upgrade script and be considered
usable to upgrade any version to the "target" side.
Using wildcards needs to be explicitly requested by
extensions via a "wildcard_upgrades" setting in their
control file.
---
src/backend/commands/extension.c | 58
1 file changed, 52 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 1a62e5dac5..ea8825fcff 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -86,6 +86,7 @@ typedef struct ExtensionControlFile
bool relocatable; /* is ALTER EXTENSION SET SCHEMA supported? */
bool superuser; /* must be superuser to install? */
bool trusted; /* allow becoming superuser on the fly? */
+ bool wildcard_upgrades; /* allow using wildcards in upgrade scripts */
int encoding; /* encoding of the script file, or -1 */
List *requires; /* names of prerequisite extensions */
} ExtensionControlFile;
@@ -128,6 +129,7 @@ static void ApplyExtensionUpdates(Oid extensionOid,
bool cascade,
bool is_create);
static char *read_whole_file(const char *filename, int *length);
+static bool file_exists(const char *name);
/*
@@ -579,6 +581,14 @@ parse_extension_control_file(ExtensionControlFile *control,
errmsg("parameter \"%s\" requires a Boolean value",
item->name)));
}
+ else if (strcmp(item->name, "wildcard_upgrades") == 0)
+ {
+ if (!parse_bool(item->value, &control->wildcard_upgrades))
+ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("parameter \"%s\" requires a Boolean value",
+item->name)));
+ }
else if (strcmp(item->name, "encoding") == 0)
{
control->encoding = pg_valid_server_encoding(item->value);
@@ -636,6 +646,7 @@ read_extension_control_file(const char *extname)
control->relocatable = false;
control->superuser = true;
control->trusted = false;
+ control->wildcard_upgrades = false;
control->encoding = -1;
/*
@@ -890,7 +901,15 @@ execute_extension_script(Oid extensionOid, ExtensionControlFile *control,
if (from_version == NULL)
elog(DEBUG1, "executing extension script for \"%s\" version '%s'", control->name, version);
else
+ {
+ if ( control->wildcard_upgrades && ! file_exists(filename) )
+ {
+ elog(DEBUG1, "extension upgrade script \"%s\" does not exist, will try wildcard", filename);
+ /* if filename does not exist, try wildcard */
+ filename = get_extension_script_filename(control, "%", version);
+ }
elog(DEBUG1, "executing extension script for \"%s\" update from version '%s' to '%s'", control->name, from_version, version);
+ }
/*
* If installing a trusted extension on behalf of a non-superuser, become
@@ -1215,13 +1234,23 @@ identify_update_path(ExtensionControlFile *control,
/* Find shortest path */
result = find_update_path(evi_list, evi_start, evi_target, false, false);
- if (result == NIL)
- ereport(ERROR,
-(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("extension \"%s\" has no update path from version \"%s\" to version \"%s\"",
- control->name, oldVersion, newVersion)));
+ if (result != NIL)
+ return result;
- return result;
+ /* Find wildcard path, if allowed by control file */
+ if ( control->wildcard_upgrades )
+ {
+ evi_start = get_ext_ver_info("%", &evi_list);
+ result = find_update_path(evi_list, evi_start, evi_target, false, false);
+
+ if (result != NIL)
+ return result;
+ }
+
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("extension \"%s\" has no update path from version \"%s\" to version \"%s\"",
+ control->name, oldVersion, newVersion)));
}
/*
@@ -3392,3 +3421,20 @@ read_whole_file(const ch
Re: [PATCH] Support % wildcard in extension upgrade filenames
And now with the actual patch attached ... (sorry)
--strk;
On Thu, Sep 15, 2022 at 12:01:04AM +0200, Sandro Santilli wrote:
> I'm attaching an updated version of the patch. This time the patch
> is tested. Nothing changes unless the .control file for the subject
> extension doesn't have a "wildcard_upgrades = true" statement.
>
> When wildcard upgrades are enabled, a file with a "%" symbol as
> the "source" part of the upgrade path will match any version and
> will be used if a specific version upgrade does not exist.
> This means that in presence of the following files:
>
> postgis--3.0.0--3.2.0.sql
> postgis--%--3.2.0.sql
>
> The first one will be used for going from 3.0.0 to 3.2.0.
>
> This is the intention. The patch lacks automated tests and can
> probably be improved.
>
> For more context, a previous (non-working) version of this patch was
> submitted to commitfest: https://commitfest.postgresql.org/38/3654/
>
> --strk;
>
> On Sat, Jun 04, 2022 at 11:20:55AM +0200, Sandro Santilli wrote:
> > On Sat, May 28, 2022 at 04:50:20PM +0200, Laurenz Albe wrote:
> > > On Fri, 2022-05-27 at 17:37 -0400, Regina Obe wrote:
> > > >
> > > > https://lists.osgeo.org/pipermail/postgis-devel/2022-February/029500.html
> > > >
> > > > Does anyone think this is such a horrible idea that we should abandon
> > > > all
> > > > hope?
> > >
> > > I don't think this idea is fundamentally wrong, but I have two worries:
> > >
> > > 1. It would be a good idea good to make sure that there is not both
> > >"extension--%--2.0.sql" and "extension--1.0--2.0.sql" present.
> > >Otherwise the behavior might be indeterministic.
> >
> > I'd make sure to use extension--1.0--2.0.sql in that case (more
> > specific first).
> >
> > > 2. What if you have a "postgis--%--3.3.sql", and somebody tries to upgrade
> > >their PostGIS 1.1 installation with it? Would that work?
> >
> > For PostGIS in particular it will NOT work as the PostGIS upgrade
> > script checks for the older version and decides if the upgrade is
> > valid or not. This is the same upgrade code used for non-extension
> > installs.
> >
> > >Having a lower bound for a matching version might be a good idea,
> > >although I have no idea how to do that.
> >
> > I was thinking of a broader pattern matching support, like:
> >
> > postgis--3.%--3.3.sql
> >
> > But it would be better to start simple and eventually if needed
> > increase the complexity ?
> >
> > Another option could be specifying something in the control file,
> > which would also probably be a good idea to still allow some
> > extensions to use '%' in the version string (for example).
> >
> > --strk;
>From 5d57d7b755c3a23ca94bf922581a417844249044 Mon Sep 17 00:00:00 2001
From: Sandro Santilli
Date: Wed, 14 Sep 2022 11:10:10 +0200
Subject: [PATCH v2] Allow wildcard (%) in extension upgrade paths
A wildcard character "%" will be accepted in the
"source" side of the upgrade script and be considered
usable to upgrade any version to the "target" side.
Using wildcards needs to be explicitly requested by
extensions via a "wildcard_upgrades" setting in their
control file.
---
src/backend/commands/extension.c | 58
1 file changed, 52 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 6b6720c690..e36a79ae75 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -86,6 +86,7 @@ typedef struct ExtensionControlFile
bool relocatable; /* is ALTER EXTENSION SET SCHEMA supported? */
bool superuser; /* must be superuser to install? */
bool trusted; /* allow becoming superuser on the fly? */
+ bool wildcard_upgrades; /* allow using wildcards in upgrade scripts */
int encoding; /* encoding of the script file, or -1 */
List *requires; /* names of prerequisite extensions */
} ExtensionControlFile;
@@ -128,6 +129,7 @@ static void ApplyExtensionUpdates(Oid extensionOid,
bool cascade,
bool is_create);
static char *read_whole_file(const char *filename, int *length);
+static bool file_exists(const char *name);
/*
@@ -579,6 +581,14 @@ parse_extension_control_file(ExtensionControlFile *control,
errmsg("parameter \"%s\" requires a Boolean value",
item->name)));
}
+ else if (strcmp(item->name, "wildcard_upgrades") == 0)
+ {
+ if (!parse_bool(item
Re: [PATCH] Support % wildcard in extension upgrade filenames
I'm attaching an updated version of the patch. This time the patch is tested. Nothing changes unless the .control file for the subject extension doesn't have a "wildcard_upgrades = true" statement. When wildcard upgrades are enabled, a file with a "%" symbol as the "source" part of the upgrade path will match any version and will be used if a specific version upgrade does not exist. This means that in presence of the following files: postgis--3.0.0--3.2.0.sql postgis--%--3.2.0.sql The first one will be used for going from 3.0.0 to 3.2.0. This is the intention. The patch lacks automated tests and can probably be improved. For more context, a previous (non-working) version of this patch was submitted to commitfest: https://commitfest.postgresql.org/38/3654/ --strk; On Sat, Jun 04, 2022 at 11:20:55AM +0200, Sandro Santilli wrote: > On Sat, May 28, 2022 at 04:50:20PM +0200, Laurenz Albe wrote: > > On Fri, 2022-05-27 at 17:37 -0400, Regina Obe wrote: > > > > > > https://lists.osgeo.org/pipermail/postgis-devel/2022-February/029500.html > > > > > > Does anyone think this is such a horrible idea that we should abandon all > > > hope? > > > > I don't think this idea is fundamentally wrong, but I have two worries: > > > > 1. It would be a good idea good to make sure that there is not both > >"extension--%--2.0.sql" and "extension--1.0--2.0.sql" present. > >Otherwise the behavior might be indeterministic. > > I'd make sure to use extension--1.0--2.0.sql in that case (more > specific first). > > > 2. What if you have a "postgis--%--3.3.sql", and somebody tries to upgrade > >their PostGIS 1.1 installation with it? Would that work? > > For PostGIS in particular it will NOT work as the PostGIS upgrade > script checks for the older version and decides if the upgrade is > valid or not. This is the same upgrade code used for non-extension > installs. > > >Having a lower bound for a matching version might be a good idea, > >although I have no idea how to do that. > > I was thinking of a broader pattern matching support, like: > > postgis--3.%--3.3.sql > > But it would be better to start simple and eventually if needed > increase the complexity ? > > Another option could be specifying something in the control file, > which would also probably be a good idea to still allow some > extensions to use '%' in the version string (for example). > > --strk; > > Libre GIS consultant/developer > https://strk.kbt.io/services.html > >
Re: [PATCH] Support % wildcard in extension upgrade filenames
On Sat, May 28, 2022 at 11:37:30AM -0400, Tom Lane wrote: > Laurenz Albe writes: > > 2. What if you have a "postgis--%--3.3.sql", and somebody tries to upgrade > >their PostGIS 1.1 installation with it? Would that work? > >Having a lower bound for a matching version might be a good idea, > >although I have no idea how to do that. > > The lack of upper bound is a problem too: what stops the system from > trying to use this to get from (say) 4.2 to 3.3, and if it does try that, > will the script produce a sane result? This is a very old problem we had before EXTENSION was even available in PostgreSQL, and so we solved this internally. The upgrade script for PostGIS checks the version of the existing code and refuses to downgrade (and refuses to upgrade if a dump/restore is required). > I'm frankly skeptical that this is a good idea at all. It seems > to have come out of someone's willful refusal to use the system as > designed, ie as a series of small upgrade scripts that each do just > one step. I don't feel an urgent need to cater to the > one-monster-script-that-handles-all-cases approach, because no one > has offered any evidence that that's really a better way. How would > you even write the conditional logic needed ... plpgsql DO blocks? > Testing what? IIRC we don't expose any explicit knowledge of the > old extension version number to the script. We (PostGIS) do expose explicit knowledge of the old extension, and for this reason I think the pattern-based logic should be enabled explicitly in the postgis.control file. It could be even less generic and more specific to a given extension need, if done completely inside the control file. For PostGIS all we need at the moment is something like (in the control file): one_monster_upgrade_script = postgis--ANY--3.3.0.sql --strk;
Re: [PATCH] Support % wildcard in extension upgrade filenames
On Sat, May 28, 2022 at 05:26:05PM +0200, Daniel Gustafsson wrote: > > On 28 May 2022, at 16:50, Laurenz Albe wrote: > > > I don't think this idea is fundamentally wrong, but I have two worries: > > > > 1. It would be a good idea good to make sure that there is not both > > "extension--%--2.0.sql" and "extension--1.0--2.0.sql" present. > > Otherwise the behavior might be indeterministic. > > > > 2. What if you have a "postgis--%--3.3.sql", and somebody tries to upgrade > > their PostGIS 1.1 installation with it? Would that work? > > Having a lower bound for a matching version might be a good idea, > > although I have no idea how to do that. > > Following that reasoning, couldn't a rogue actor inject a fake file (perhaps > bundled with another innocent looking extension) which takes precedence in > wildcard matching? I think whoever can write into the PostgreSQL extension folder will be able to inject anything anyway --strk;
Re: [PATCH] Support % wildcard in extension upgrade filenames
On Sat, May 28, 2022 at 04:50:20PM +0200, Laurenz Albe wrote: > On Fri, 2022-05-27 at 17:37 -0400, Regina Obe wrote: > > > > https://lists.osgeo.org/pipermail/postgis-devel/2022-February/029500.html > > > > Does anyone think this is such a horrible idea that we should abandon all > > hope? > > I don't think this idea is fundamentally wrong, but I have two worries: > > 1. It would be a good idea good to make sure that there is not both >"extension--%--2.0.sql" and "extension--1.0--2.0.sql" present. >Otherwise the behavior might be indeterministic. I'd make sure to use extension--1.0--2.0.sql in that case (more specific first). > 2. What if you have a "postgis--%--3.3.sql", and somebody tries to upgrade >their PostGIS 1.1 installation with it? Would that work? For PostGIS in particular it will NOT work as the PostGIS upgrade script checks for the older version and decides if the upgrade is valid or not. This is the same upgrade code used for non-extension installs. >Having a lower bound for a matching version might be a good idea, >although I have no idea how to do that. I was thinking of a broader pattern matching support, like: postgis--3.%--3.3.sql But it would be better to start simple and eventually if needed increase the complexity ? Another option could be specifying something in the control file, which would also probably be a good idea to still allow some extensions to use '%' in the version string (for example). --strk; Libre GIS consultant/developer https://strk.kbt.io/services.html
[PATCH] Support % wildcard in extension upgrade filenames
At PostGIS we've been thinking of ways to have better support, from
PostgreSQL proper, to our upgrade strategy, which has always consisted
in a SINGLE upgrade file good for upgrading from any older version.
The current lack of such support for EXTENSIONs forced us to install
a lot of files on disk and we'd like to stop doing that at some point
in the future.
The proposal is to support wildcards for versions encoded in the
filename so that (for our case) a single wildcard could match "any
version". I've been thinking about the '%' character for that, to
resemble the wildcard used for LIKE.
Here's the proposal:
https://lists.osgeo.org/pipermail/postgis-devel/2022-February/029500.html
A very very short (and untested) patch which might (or
might not) support our case is attached.
The only problem with my proposal/patch would be the presence, on the
wild, of PostgreSQL EXTENSION actually using the '%' character in
their version strings, which is currently considered legit by
PostgreSQL.
How do you feel about the proposal (which is wider than the patch) ?
--strk;
Libre GIS consultant/developer
https://strk.kbt.io/services.html
>From e9d060b19c655924c4edb6679169261d605f735d Mon Sep 17 00:00:00 2001
From: Sandro Santilli
Date: Fri, 11 Feb 2022 18:49:45 +0100
Subject: [PATCH] Support % wildcard in extension upgrade scripts
---
src/backend/commands/extension.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index a2e77c418a..aafd9c17ee 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -1072,6 +1072,8 @@ get_ext_ver_info(const char *versionname, List **evi_list)
foreach(lc, *evi_list)
{
evi = (ExtensionVersionInfo *) lfirst(lc);
+ if (strcmp(evi->name, "%") == 0)
+ return evi;
if (strcmp(evi->name, versionname) == 0)
return evi;
}
--
2.30.2
Re: [postgis-devel] About EXTENSION from UNPACKAGED on PostgreSQL 13
On Thu, Feb 27, 2020 at 09:32:24AM +0100, Sandro Santilli wrote: > On Wed, Feb 26, 2020 at 11:18:43AM -0500, Chapman Flack wrote: > > On 2/26/20 10:52 AM, Sandro Santilli wrote: > > > > > This part is not clear to me. You're _assuming_ that the unpackaged--xxx > > > will not make checks, so you _drop_ support for it ? Can't the normal > > > extension script also be unsafe for some reason ? Or can't the > > > unpackaged-xxx script be made safe by the publishers ? Or, as a last > > > resort.. can't you just mark postgis as UNSAFE and still require > > > superuser, which would give us the same experience as before ? > > > > I am wondering: does anything in the PG 13 change preclude writing > > a postgis_raster--unpackaged.sql script that could be applied with > > CREATE EXTENSION postgis_raster VERSION unpackaged; > > and would do perhaps nothing at all, or merely confirm that the > > right unpackaged things are present and are the right things... > > > > ... from which an ALTER EXTENSION postgis_raster UPDATE TO 3.0; > > would naturally run the existing postgis_raster--unpackaged--3.0.sql > > and execute all of its existing ALTER EXTENSION ... ADD operations? > > > > Has the disadvantage of being goofy, but possibly the advantage of > > being implementable in the current state of affairs. > > Thanks for this hint, yes, seems to be technically feasible, as well > as doing packaging in the extension creation scripts. Only... this > would basically work around the intentionally removed syntax, which > Steven Frost was against (still unclear to me why)... NOTE: my suggestion was to directly have CREATE EXTENSION do the packaging, which would give the same level of security as the workaround suggested here, but with less hops. --strk;
Re: [postgis-devel] About EXTENSION from UNPACKAGED on PostgreSQL 13
On Wed, Feb 26, 2020 at 11:18:43AM -0500, Chapman Flack wrote: > On 2/26/20 10:52 AM, Sandro Santilli wrote: > > > This part is not clear to me. You're _assuming_ that the unpackaged--xxx > > will not make checks, so you _drop_ support for it ? Can't the normal > > extension script also be unsafe for some reason ? Or can't the > > unpackaged-xxx script be made safe by the publishers ? Or, as a last > > resort.. can't you just mark postgis as UNSAFE and still require > > superuser, which would give us the same experience as before ? > > I am wondering: does anything in the PG 13 change preclude writing > a postgis_raster--unpackaged.sql script that could be applied with > CREATE EXTENSION postgis_raster VERSION unpackaged; > and would do perhaps nothing at all, or merely confirm that the > right unpackaged things are present and are the right things... > > ... from which an ALTER EXTENSION postgis_raster UPDATE TO 3.0; > would naturally run the existing postgis_raster--unpackaged--3.0.sql > and execute all of its existing ALTER EXTENSION ... ADD operations? > > Has the disadvantage of being goofy, but possibly the advantage of > being implementable in the current state of affairs. Thanks for this hint, yes, seems to be technically feasible, as well as doing packaging in the extension creation scripts. Only... this would basically work around the intentionally removed syntax, which Steven Frost was against (still unclear to me why)... --strk;
Re: [postgis-devel] About EXTENSION from UNPACKAGED on PostgreSQL 13
On Wed, Feb 26, 2020 at 10:37:41AM -0500, Stephen Frost wrote: > Greetings, > > * Sandro Santilli (s...@kbt.io) wrote: > > On pgsql-hackers we only want to find a future-proof way to "package > > existing objects into an extension". If the syntax > > `CREATE EXTENSION FROM UNPACKAGED` > > has gone, would it be ok for just: > > `CREATE EXTENSION ` > > to intercept unpackaged objects and package them ? > > No. The reason it was removed is because it's not going to be safe to > do when we have trusted extensions. This part is not clear to me. You're _assuming_ that the unpackaged--xxx will not make checks, so you _drop_ support for it ? Can't the normal extension script also be unsafe for some reason ? Or can't the unpackaged-xxx script be made safe by the publishers ? Or, as a last resort.. can't you just mark postgis as UNSAFE and still require superuser, which would give us the same experience as before ? > Perhaps it would be possible to > figure out a way to make it safe, but the reason FROM UNPACKAGED was > created and existed doesn't apply any more. Wasn't the reason of existance the ability for people to switch from non-extension to extension based installs ? > That PostGIS has been using > it for something else entirely is unfortunate, but the way to address > what PostGIS needs is to talk about that, not talk about how this ugly > hack used to work and doesn't any more. Seriously, what was FROM UNPACKAGED meant to be used for ? --strk;
Re: [postgis-devel] About EXTENSION from UNPACKAGED on PostgreSQL 13
On Wed, Feb 26, 2020 at 03:35:46PM +0100, Daniel Gustafsson wrote: > > On 26 Feb 2020, at 15:13, Sandro Santilli wrote: > > > On pgsql-hackers we only want to find a future-proof way to "package > > existing objects into an extension". > > What is the longterm goal of PostGIS, to use this as a stepping stone to reach > a point where no unpackaged extensions exist; or find a way to continue with > the current setup except with syntax that isn't going away? No unpackaged extension seems like a good goal in the long term. > Overloading the same syntax for creating packaged as well as unpackaged > extensions sounds like the wrong path to go down. So what other options would we have to let people upgrade a running postgis or postgis_raster outside of the EXTENSION mechanism ? --strk;
Re: [postgis-devel] About EXTENSION from UNPACKAGED on PostgreSQL 13
On Wed, Feb 26, 2020 at 08:55:03AM -0500, Stephen Frost wrote: > Greetings, > > * Darafei "Komяpa" Praliaskouski (m...@komzpa.net) wrote: > > PostGIS 2.5 had raster and vector blended together in single extension. > > In PostGIS 3, they were split out into postgis and postgis_raster > > extensions. > > For my 2c, at least, I still don't really get why that split was done. It's pretty easy to understand: to let user decide what he needs and what not. > > Removal of FROM UNPACKAGED breaks PostGIS 2.5 -> 3.0 upgrade path, and > > we haven't yet found a proper replacement since such removal wasn't > > something we were expecting. > > I agree that there probably isn't a very good path to allow an extension > to be split up like that without having to drop some things. An > alternative would have been to *not* split up postgis, but rather to > have a postgis_raster and a postgis_vector. Adding in support for other > ways to migrate a function from one extension to another would make > sense too. I think support for migrating an object between extensions DOES exist, it's just that you cannot use it from extension upgrade scripts. Anyway pgsql-hackers is not the right place for discussion. On pgsql-hackers we only want to find a future-proof way to "package existing objects into an extension". If the syntax `CREATE EXTENSION FROM UNPACKAGED` has gone, would it be ok for just: `CREATE EXTENSION ` to intercept unpackaged objects and package them ? --strk;
Re: Marking some contrib modules as trusted extensions
On Wed, Feb 26, 2020 at 12:17:37AM -0800, Andres Freund wrote: > Hi, > > On 2020-02-26 09:11:21 +0100, Sandro Santilli wrote: > > PostGIS uses unpackaged-to-XXX pretty heavily, and has it under > > automated testing (which broke since "FROM unpackaged" support was > > removed, see 14514.1581638...@sss.pgh.pa.us) > > > > We'd be ok with requiring SUPERUSER for doing that, since that's > > what is currently required so nothing would change for us. > > > > Instead, dropping UPGRADE..FROM completely puts us in trouble of > > having to find another way to "package" postgis objects. > > Coul you explain what postgis is trying to achieve with FROM unpackaged? We're turning a non-extension based install into an extension-based one. Common need for those who came to PostGIS way before EXTENSION was even invented and for those who remained there for the bigger flexibility (for example to avoid the raster component, which was unavoidable with EXTENSION mechanism until PostGIS 3.0). For the upgrades to 3.0.0 when coming from a previous version we're using that `FROM unpackaged` SYNTAX for re-packaging the raster component for those who still want it (raster objects are unpackaged from 'postgis' extension on EXTENSION UPDATE because there was no other way to move them from an extension to another). I guess it would be ok for us to do the packaging directly from the scripts that would run on `CREATE EXTENSION postgis`, but would that mean we'd take the security risk you're trying to avoid by dropping the `FROM unpackaged` syntax ? --strk;
Re: Marking some contrib modules as trusted extensions
On Thu, Feb 13, 2020 at 07:09:18PM -0500, Tom Lane wrote: > I wrote: > > Andres Freund writes: > >> It seems to me that FROM UNPACKAGED shouldn't support trusted. > > > Hmm, seems like a reasonable idea, but I'm not quite sure how to mechanize > > it given that "unpackaged" isn't magic in any way so far as extension.c > > is concerned. Maybe we could decide that the time for supporting easy > > updates from pre-9.1 is past, and just remove all the unpackaged-to-XXX > > scripts? Maybe even remove the "FROM version" option altogether. > > [ thinks some more... ] A less invasive idea would be to insist that > you be superuser to use the FROM option. But I'm thinking that the > unpackaged-to-XXX scripts are pretty much dead letters anymore. Has > anyone even tested them in years? How much longer do we want to be > on the hook to fix them? PostGIS uses unpackaged-to-XXX pretty heavily, and has it under automated testing (which broke since "FROM unpackaged" support was removed, see 14514.1581638...@sss.pgh.pa.us) We'd be ok with requiring SUPERUSER for doing that, since that's what is currently required so nothing would change for us. Instead, dropping UPGRADE..FROM completely puts us in trouble of having to find another way to "package" postgis objects. --strk;
