[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
Attachments:
0001-Support-wildcard-in-extension-upgrade-scripts.patchtext/x-diff; charset=us-asciiDownload
From e9d060b19c655924c4edb6679169261d605f735d Mon Sep 17 00:00:00 2001
From: Sandro Santilli <strk@kbt.io>
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
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.htmlA 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
[Regina Obe]
Just a heads up about the above, Sandro has added it as a commitfest item
which hopefully we can polish in time for PG16.
https://commitfest.postgresql.org/38/3654/
Does anyone think this is such a horrible idea that we should abandon all
hope?
The main impetus is that many extensions (postgis, pgRouting, and I'm sure
others) have over 300 extensions script files that are exactly the same.
We'd like to reduce this footprint significantly.
strk said the patch is crappy so don't look at it just yet. We'll work on
polishing it. I'll review and provide docs for it.
Thanks,
Regina
On Fri, 2022-05-27 at 17:37 -0400, Regina Obe wrote:
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.htmlA 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) ?
Just a heads up about the above, Sandro has added it as a commitfest item
which hopefully we can polish in time for PG16.
https://commitfest.postgresql.org/38/3654/Does anyone think this is such a horrible idea that we should abandon all
hope?The main impetus is that many extensions (postgis, pgRouting, and I'm sure
others) have over 300 extensions script files that are exactly the same.
We'd like to reduce this footprint significantly.strk said the patch is crappy so don't look at it just yet. We'll work on
polishing it. I'll review and provide docs for it.
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.
Yours,
Laurenz Albe
On 28 May 2022, at 16:50, Laurenz Albe <laurenz.albe@cybertec.at> 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?
--
Daniel Gustafsson https://vmware.com/
Laurenz Albe <laurenz.albe@cybertec.at> writes:
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.
The existing logic to find multi-step upgrade paths is going to make
this a very pressing problem. For example, if you provide both
extension--%--2.0.sql and extension--%--2.1.sql, it's not at all
clear whether the code would try to use both of those or just one
to get from 1.x to 2.1.
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? (Seems unlikely, as it couldn't
know what 4.x objects it ought to alter or remove.) But it's going to be
very hard to provide bounds, because we intentionally designed the
extension system to not have an explicit concept of some versions being
less than others.
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.
regards, tom lane
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
On Sat, May 28, 2022 at 05:26:05PM +0200, Daniel Gustafsson wrote:
On 28 May 2022, at 16:50, Laurenz Albe <laurenz.albe@cybertec.at> 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;
On Sat, May 28, 2022 at 11:37:30AM -0400, Tom Lane wrote:
Laurenz Albe <laurenz.albe@cybertec.at> 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;
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;
Show quoted text
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
And now with the actual patch attached ... (sorry)
--strk;
Show quoted text
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.sqlThe 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;
Attachments:
v2-0001-Allow-wildcard-in-extension-upgrade-paths.patchtext/x-diff; charset=us-asciiDownload
From 5d57d7b755c3a23ca94bf922581a417844249044 Mon Sep 17 00:00:00 2001
From: Sandro Santilli <strk@kbt.io>
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->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 char *filename, int *length)
buf[*length] = '\0';
return buf;
}
+
+static bool
+file_exists(const char *name)
+{
+ struct stat st;
+
+ AssertArg(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;
+}
--
2.34.1
Just to reiterate the main impetus for this patch is to save PostGIS from
shipping 100s of duplicate extension files for each release.
And now with the actual patch attached ... (sorry)
--strk;
Sandro, can you submit an updated version of this patch.
I was testing it out and looked good first time.
But I retried just now testing against master, and it fails with
commands/extension.o: In function `file_exists':
postgresql-git\src\backend\commands/extension.c:3430: undefined reference to
`AssertArg'
It seems 2 days ago AssertArg and AssertState were removed.
https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=b1099eca8f38f
f5cfaf0901bb91cb6a22f909bc6
So your use of AssertArg needs to be replaced with Assert I guess.
I did that and was able to test again with a sample extension I made
Called:
wildtest
1) The wildcard patch in its current state only does anything if
wildcard_upgrades = true
is in the control file. If it's false or missing, then the behavior of
extension upgrades doesn't change.
2) It only understands % as a complete wildcard for a version number
So this is legal
wildtest--%--2.0.sql
This does nothing
wildtest--2%--2.0.sql
3) I confirmed that if you have a path such as
wildtest--2.0--2.2.sql
wildtest--%--2.2.sql
then the exact match trumps the wildcard. In the above case if I am on 2.0
and going to 2.2, the wildtest--2.0--2.2.sql script is used instead of the
wildtest--% one.
4) It is not possible to downgrade with the wildcard. For example I had
paths
wildtest--%--2.1.sql
and I was unable to go from a version 2.2 down to a version 2.1. I didn't
check why that was so, but probably a good thing.
If everyone is okay with this patch, we'll go ahead and add tests and
documentation to go with it.
Thanks,
Regina
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;
Attachments:
0001-Allow-wildcard-in-extension-upgrade-paths.patchtext/x-diff; charset=us-asciiDownload
From 8725c616c31a9bc25478c7128ea81ce753e51089 Mon Sep 17 00:00:00 2001
From: Sandro Santilli <strk@kbt.io>
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 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;
+}
--
2.34.1
And now a version of the patch including documentation and regression tests.
Anything you see I should improve ?
--strk;
Show quoted text
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;
Attachments:
Allow-wildcard-in-extension-upgrade-paths.patchtext/x-diff; charset=us-asciiDownload
From 4b10297315d2db4365e6f47e375588394b260d0d Mon Sep 17 00:00:00 2001
From: Sandro Santilli <strk@kbt.io>
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 ...
</para>
</listitem>
</varlistentry>
+
+ <varlistentry>
+ <term><varname>wildcard_upgrades</varname> (<type>boolean</type>)</term>
+ <listitem>
+ <para>
+ This parameter, if set to <literal>true</literal> (which is not the
+ default), allows <command>ALTER EXTENSION</command> to consider
+ a wildcard character <literal>%</literal> as matching any version of
+ the extension. Such wildcard match will only be used when no
+ perfect match is found for a version.
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
<para>
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 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 c3139ab0fc..4fe2d82b6e 100644
--- a/src/test/modules/test_extensions/Makefile
+++ b/src/test/modules/test_extensions/Makefile
@@ -6,14 +6,16 @@ PGFILEDESC = "test_extensions - regression testing for EXTENSION support"
EXTENSION = test_ext1 test_ext2 test_ext3 test_ext4 test_ext5 test_ext6 \
test_ext7 test_ext8 test_ext_cine test_ext_cor \
test_ext_cyclic1 test_ext_cyclic2 \
- test_ext_evttrig
+ test_ext_evttrig test_ext_wildcard1
DATA = test_ext1--1.0.sql test_ext2--1.0.sql test_ext3--1.0.sql \
test_ext4--1.0.sql test_ext5--1.0.sql test_ext6--1.0.sql \
test_ext7--1.0.sql test_ext7--1.0--2.0.sql test_ext8--1.0.sql \
test_ext_cine--1.0.sql test_ext_cine--1.0--1.1.sql \
test_ext_cor--1.0.sql \
test_ext_cyclic1--1.0.sql test_ext_cyclic2--1.0.sql \
- test_ext_evttrig--1.0.sql test_ext_evttrig--1.0--2.0.sql
+ test_ext_evttrig--1.0.sql test_ext_evttrig--1.0--2.0.sql \
+ test_ext_wildcard1--1.0.sql test_ext_wildcard1--%--2.0.sql \
+
REGRESS = test_extensions test_extdepend
diff --git a/src/test/modules/test_extensions/expected/test_extensions.out b/src/test/modules/test_extensions/expected/test_extensions.out
index 821fed38d1..1c4dc5be42 100644
--- a/src/test/modules/test_extensions/expected/test_extensions.out
+++ b/src/test/modules/test_extensions/expected/test_extensions.out
@@ -312,3 +312,18 @@ Objects in extension "test_ext_cine"
table ext_cine_tab3
(9 rows)
+CREATE EXTENSION test_ext_wildcard1;
+SELECT ext_wildcard1_version();
+ ext_wildcard1_version
+-----------------------
+ 1.0
+(1 row)
+
+ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0';
+SELECT ext_wildcard1_version();
+ ext_wildcard1_version
+-----------------------
+ 2.0
+(1 row)
+
+DROP EXTENSION test_ext_wildcard1;
diff --git a/src/test/modules/test_extensions/sql/test_extensions.sql b/src/test/modules/test_extensions/sql/test_extensions.sql
index 41b6cddf0b..071845e8df 100644
--- a/src/test/modules/test_extensions/sql/test_extensions.sql
+++ b/src/test/modules/test_extensions/sql/test_extensions.sql
@@ -209,3 +209,10 @@ CREATE EXTENSION test_ext_cine;
ALTER EXTENSION test_ext_cine UPDATE TO '1.1';
\dx+ test_ext_cine
+
+
+CREATE EXTENSION test_ext_wildcard1;
+SELECT ext_wildcard1_version();
+ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0';
+SELECT ext_wildcard1_version();
+DROP EXTENSION test_ext_wildcard1;
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql b/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
new file mode 100644
index 0000000000..75154e5c55
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
@@ -0,0 +1,6 @@
+/* src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql */
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0'" to load this file. \quit
+
+CREATE OR REPLACE FUNCTION ext_wildcard1_version() returns TEXT
+AS 'SELECT 2.0' LANGUAGE 'sql';
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql b/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
new file mode 100644
index 0000000000..a69e791fda
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
@@ -0,0 +1,6 @@
+/* src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql */
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "CREATE EXTENSION test_ext_wildcard1" to load this file. \quit
+
+CREATE FUNCTION ext_wildcard1_version() returns TEXT
+AS 'SELECT 1.0' LANGUAGE 'sql';
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1.control b/src/test/modules/test_extensions/test_ext_wildcard1.control
new file mode 100644
index 0000000000..865e37fa88
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1.control
@@ -0,0 +1,4 @@
+comment = 'Test extension wildcard 1'
+default_version = '1.0'
+relocatable = true
+wildcard_upgrades = true
--
2.34.1
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
PURPOSE:
This feature is intended to allow projects with many micro versions that do the same thing, be able to ship much fewer files by specifying less granular stopping.
One of the projects that will benefit from this is the PostGIS project. Currently we ship over 300 extension scripts per version which are currently just symlinks to the same file.
Part of the reason for that is our changes are dependent on both PostgreSQL version and PostGIS version, so a simple upgrade that only considers say PostGIS 3.2.0--3.3.1 is not sufficient.
Also much of the time, our function definitions don't change in micros, but yet we need to ship them to allow users to properly upgrade.
This feature will allow us to get rid of all these symlinks or 0-byte files.
I've tested this feature against the PostgreSQL master branch on mingw64 gcc 8.1.
BASIC FEATURES
1) As stated, this feature only works if in the .control file, has a line:
wildcard_upgrades = true
2) It does nothing different if the line is missing or set to false.
3) If there is such a line and there is no other path that takes an extension from it's current version to the requested version, then it will use the wildcard script file.
TESTING
AUTOMATED TESTS
I have confirmed tests are in place.
However the tests do not run when I do
make check (from the root source folder)
or when I do
make installcheck-world
To run these tests, I had to
cd src/test/modules/test_extensions
make check
and see the output showing:
============== running regression test queries ==============
test test_extensions ... ok 186 ms
test test_extdepend ... ok 97 ms
I confirmed the tests are in test_extensions, which has always existed.
So I assume why it's not being run in installcheck-world is something off with my configuration
or it's intentionally not run.
MANUAL TESTS
I also ran some adhoc tests of my own to confirm the behavior. and checking with
SET client_min_messages='DEBUG1';
To confirm that the wildcard script I have only gets called when there is no other path that will take the user to the new version.
I created my own extension
wildtest
1) I confirmed It only understands % as a complete wildcard for a version number
So this is legal
wildtest--%--2.0.sql
This does nothing
wildtest--2%--2.0.sql
2) I confirmed that if you have a path such as
wildtest--2.0--2.2.sql
wildtest--%--2.2.sql
then the exact match trumps the wildcard. In the above case if I am on 2.0
and going to 2.2, the wildtest--2.0--2.2.sql script is used instead of the
wildtest--% one.
3) It is not possible to downgrade with the wildcard. For example I had
paths
wildtest--%--2.1.sql
and I was unable to go from a version 2.2 down to a version 2.1.
DOCUMENTATION
I built the html docs and confirmed that in the section of the docs where it defines valid properties of extension files it now includes this line:
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.
The new status of this patch is: Ready for Committer
Apologies. I made a mistake on my downgrade test.
So 3 should be
3) It is possible to downgrade with the wildcard. For example I had
paths
wildtest--%--2.1.sql
and confirmed it used the downgrade path when going from 2.2 to 2.1
Re: Tom Lane
The existing logic to find multi-step upgrade paths is going to make
this a very pressing problem. For example, if you provide both
extension--%--2.0.sql and extension--%--2.1.sql, it's not at all
clear whether the code would try to use both of those or just one
to get from 1.x to 2.1.
find_update_path already includes Dijkstra to avoid that problem.
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.
I was just talking to strk about that on #postgis and where's what I
took away from the discussion:
The main objection seems to be that 500 identical (linked) files
aren't how the extension system is supposed to be used, and that this
proposal merely changes that to a single foo--%--1.2.3.sql file which
is again not how the system is supposed to be used. Instead, we should
try to find a small set of files that just change what needs to be
changed between the versions.
First, it's 580 files because the same problem exists for 7 extensions
shipped in postgis, so we are effectively talking about 80 files per
extension.
So the question is, can we reduce that 80 to some small number. It
turns out the answer is "no" for several reasons:
* parallel branches: postgis maintains several major branches, and
people can upgrade at any time. If we currently have 3.3.4 and
3.4.0, we will likely have a 3.3.4--3.4.0.sql file. Now when 3.3.5
and 3.4.1 are released, we add 3.3.4--3.3.5 in the 3.3 branch, and a
3.4.0--3.4.1 in the 3.4 branch.
But we will also have to provide a new 3.3.5--3.4.0 (or
3.3.5--3.4.1) file *in the 3.4 branch*. That means even if we decide
that upgrades always need to go through a 3.4.0 pivot version, we
have to update the 3.4 branch each time 3.3 is updated, because
history isn't single-branch linear. Lots of extra files, which need
to be stored in a non-natural location.
If we wanted to use the "proper" update files here, we'd be left
with something like (number of major versions)*(number of all minor
versions in total) number of files, spread over several major
versions, all interconnected.
* Even if we disregard topology, we still have to provide *some*
start-updates-here file for each minor version released ever before.
Hence we are at 80. (The number might be lower for smaller
extensions like address_standardizer, but postgis itself would
change in each version, plus many of the other extensions in there.)
Christoph
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.
Christoph
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 andFwiw 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.Christoph
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.
Thanks,
Regina
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 andFwiw 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'sclear
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;
Attachments:
0001-Allow-wildcard-in-extension-upgrade-paths.patchtext/x-diff; charset=us-asciiDownload
From 9b138eae95e0d389bee3776247ba9d7d5144bcc5 Mon Sep 17 00:00:00 2001
From: Sandro Santilli <strk@kbt.io>
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
<literal>1.1</literal>).
</para>
+ <para>
+ The literal value <literal>%</literal> can be used as the
+ <replaceable>old_version</replaceable> 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.
+ </para>
+
<para>
Given that a suitable update script is available, the command
<command>ALTER EXTENSION UPDATE</command> 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 c3139ab0fc..4fe2d82b6e 100644
--- a/src/test/modules/test_extensions/Makefile
+++ b/src/test/modules/test_extensions/Makefile
@@ -6,14 +6,16 @@ PGFILEDESC = "test_extensions - regression testing for EXTENSION support"
EXTENSION = test_ext1 test_ext2 test_ext3 test_ext4 test_ext5 test_ext6 \
test_ext7 test_ext8 test_ext_cine test_ext_cor \
test_ext_cyclic1 test_ext_cyclic2 \
- test_ext_evttrig
+ test_ext_evttrig test_ext_wildcard1
DATA = test_ext1--1.0.sql test_ext2--1.0.sql test_ext3--1.0.sql \
test_ext4--1.0.sql test_ext5--1.0.sql test_ext6--1.0.sql \
test_ext7--1.0.sql test_ext7--1.0--2.0.sql test_ext8--1.0.sql \
test_ext_cine--1.0.sql test_ext_cine--1.0--1.1.sql \
test_ext_cor--1.0.sql \
test_ext_cyclic1--1.0.sql test_ext_cyclic2--1.0.sql \
- test_ext_evttrig--1.0.sql test_ext_evttrig--1.0--2.0.sql
+ test_ext_evttrig--1.0.sql test_ext_evttrig--1.0--2.0.sql \
+ test_ext_wildcard1--1.0.sql test_ext_wildcard1--%--2.0.sql \
+
REGRESS = test_extensions test_extdepend
diff --git a/src/test/modules/test_extensions/expected/test_extensions.out b/src/test/modules/test_extensions/expected/test_extensions.out
index 821fed38d1..1c4dc5be42 100644
--- a/src/test/modules/test_extensions/expected/test_extensions.out
+++ b/src/test/modules/test_extensions/expected/test_extensions.out
@@ -312,3 +312,18 @@ Objects in extension "test_ext_cine"
table ext_cine_tab3
(9 rows)
+CREATE EXTENSION test_ext_wildcard1;
+SELECT ext_wildcard1_version();
+ ext_wildcard1_version
+-----------------------
+ 1.0
+(1 row)
+
+ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0';
+SELECT ext_wildcard1_version();
+ ext_wildcard1_version
+-----------------------
+ 2.0
+(1 row)
+
+DROP EXTENSION test_ext_wildcard1;
diff --git a/src/test/modules/test_extensions/sql/test_extensions.sql b/src/test/modules/test_extensions/sql/test_extensions.sql
index 41b6cddf0b..071845e8df 100644
--- a/src/test/modules/test_extensions/sql/test_extensions.sql
+++ b/src/test/modules/test_extensions/sql/test_extensions.sql
@@ -209,3 +209,10 @@ CREATE EXTENSION test_ext_cine;
ALTER EXTENSION test_ext_cine UPDATE TO '1.1';
\dx+ test_ext_cine
+
+
+CREATE EXTENSION test_ext_wildcard1;
+SELECT ext_wildcard1_version();
+ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0';
+SELECT ext_wildcard1_version();
+DROP EXTENSION test_ext_wildcard1;
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql b/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
new file mode 100644
index 0000000000..75154e5c55
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
@@ -0,0 +1,6 @@
+/* src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql */
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0'" to load this file. \quit
+
+CREATE OR REPLACE FUNCTION ext_wildcard1_version() returns TEXT
+AS 'SELECT 2.0' LANGUAGE 'sql';
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql b/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
new file mode 100644
index 0000000000..a69e791fda
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
@@ -0,0 +1,6 @@
+/* src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql */
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "CREATE EXTENSION test_ext_wildcard1" to load this file. \quit
+
+CREATE FUNCTION ext_wildcard1_version() returns TEXT
+AS 'SELECT 1.0' LANGUAGE 'sql';
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1.control b/src/test/modules/test_extensions/test_ext_wildcard1.control
new file mode 100644
index 0000000000..0c2fc6fca6
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1.control
@@ -0,0 +1,3 @@
+comment = 'Test extension wildcard 1'
+default_version = '1.0'
+relocatable = true
--
2.34.1
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 andFwiw 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'sclear
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;
I think you should increment the version number on the file name of this
patch.
You had one earlier called 0001-...
The one before that was missing a version number entirely.
Maybe call this 0003-...
Thanks,
Regina
Note that the patch is passing tests when using autoconf build but
failing for meson builds.
http://cfbot.cputube.org/sandro-santilli.html
I imagine you need to make the corresponding change to ./meson.build
that you made to ./Makefile.
https://wiki.postgresql.org/wiki/Meson_for_patch_authors
https://wiki.postgresql.org/wiki/Meson
You can test under cirrusci from your github account, with instructions
at: ./src/tools/ci/README
Cheers,
--
Justin
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;
Attachments:
0001-Allow-wildcard-in-extension-upgrade-paths.patchtext/x-diff; charset=us-asciiDownload
From 344f2b96c172ed8c75990ecb86e78494c82df54d Mon Sep 17 00:00:00 2001
From: Sandro Santilli <strk@kbt.io>
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
<literal>1.1</literal>).
</para>
+ <para>
+ The literal value <literal>%</literal> can be used as the
+ <replaceable>old_version</replaceable> 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.
+ </para>
+
<para>
Given that a suitable update script is available, the command
<command>ALTER EXTENSION UPDATE</command> 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 c3139ab0fc..4fe2d82b6e 100644
--- a/src/test/modules/test_extensions/Makefile
+++ b/src/test/modules/test_extensions/Makefile
@@ -6,14 +6,16 @@ PGFILEDESC = "test_extensions - regression testing for EXTENSION support"
EXTENSION = test_ext1 test_ext2 test_ext3 test_ext4 test_ext5 test_ext6 \
test_ext7 test_ext8 test_ext_cine test_ext_cor \
test_ext_cyclic1 test_ext_cyclic2 \
- test_ext_evttrig
+ test_ext_evttrig test_ext_wildcard1
DATA = test_ext1--1.0.sql test_ext2--1.0.sql test_ext3--1.0.sql \
test_ext4--1.0.sql test_ext5--1.0.sql test_ext6--1.0.sql \
test_ext7--1.0.sql test_ext7--1.0--2.0.sql test_ext8--1.0.sql \
test_ext_cine--1.0.sql test_ext_cine--1.0--1.1.sql \
test_ext_cor--1.0.sql \
test_ext_cyclic1--1.0.sql test_ext_cyclic2--1.0.sql \
- test_ext_evttrig--1.0.sql test_ext_evttrig--1.0--2.0.sql
+ test_ext_evttrig--1.0.sql test_ext_evttrig--1.0--2.0.sql \
+ test_ext_wildcard1--1.0.sql test_ext_wildcard1--%--2.0.sql \
+
REGRESS = test_extensions test_extdepend
diff --git a/src/test/modules/test_extensions/expected/test_extensions.out b/src/test/modules/test_extensions/expected/test_extensions.out
index 821fed38d1..1c4dc5be42 100644
--- a/src/test/modules/test_extensions/expected/test_extensions.out
+++ b/src/test/modules/test_extensions/expected/test_extensions.out
@@ -312,3 +312,18 @@ Objects in extension "test_ext_cine"
table ext_cine_tab3
(9 rows)
+CREATE EXTENSION test_ext_wildcard1;
+SELECT ext_wildcard1_version();
+ ext_wildcard1_version
+-----------------------
+ 1.0
+(1 row)
+
+ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0';
+SELECT ext_wildcard1_version();
+ ext_wildcard1_version
+-----------------------
+ 2.0
+(1 row)
+
+DROP EXTENSION test_ext_wildcard1;
diff --git a/src/test/modules/test_extensions/meson.build b/src/test/modules/test_extensions/meson.build
index e95a9f2e7e..79d90b34c1 100644
--- a/src/test/modules/test_extensions/meson.build
+++ b/src/test/modules/test_extensions/meson.build
@@ -29,6 +29,9 @@ install_data(
'test_ext_evttrig--1.0--2.0.sql',
'test_ext_evttrig--1.0.sql',
'test_ext_evttrig.control',
+ 'test_ext_wildcard1--1.0.sql',
+ 'test_ext_wildcard1--%--2.0.sql',
+ 'test_ext_wildcard1.control',
kwargs: contrib_data_args,
)
diff --git a/src/test/modules/test_extensions/sql/test_extensions.sql b/src/test/modules/test_extensions/sql/test_extensions.sql
index 41b6cddf0b..071845e8df 100644
--- a/src/test/modules/test_extensions/sql/test_extensions.sql
+++ b/src/test/modules/test_extensions/sql/test_extensions.sql
@@ -209,3 +209,10 @@ CREATE EXTENSION test_ext_cine;
ALTER EXTENSION test_ext_cine UPDATE TO '1.1';
\dx+ test_ext_cine
+
+
+CREATE EXTENSION test_ext_wildcard1;
+SELECT ext_wildcard1_version();
+ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0';
+SELECT ext_wildcard1_version();
+DROP EXTENSION test_ext_wildcard1;
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql b/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
new file mode 100644
index 0000000000..75154e5c55
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
@@ -0,0 +1,6 @@
+/* src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql */
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0'" to load this file. \quit
+
+CREATE OR REPLACE FUNCTION ext_wildcard1_version() returns TEXT
+AS 'SELECT 2.0' LANGUAGE 'sql';
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql b/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
new file mode 100644
index 0000000000..a69e791fda
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
@@ -0,0 +1,6 @@
+/* src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql */
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "CREATE EXTENSION test_ext_wildcard1" to load this file. \quit
+
+CREATE FUNCTION ext_wildcard1_version() returns TEXT
+AS 'SELECT 1.0' LANGUAGE 'sql';
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1.control b/src/test/modules/test_extensions/test_ext_wildcard1.control
new file mode 100644
index 0000000000..0c2fc6fca6
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1.control
@@ -0,0 +1,3 @@
+comment = 'Test extension wildcard 1'
+default_version = '1.0'
+relocatable = true
--
2.34.1
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: tested, passed
Documentation: tested, passed
I reviewed this patch /messages/by-id/20221117095734.igldlk6kngr6ogim@c19
Most look good to me. The only things that have changed since last I reviewed this, with the new patch is
1) wildcard_upgrades=true is no longer needed in the control file and will break if present. This is an expected change, since we are now going strictly by presence of % extension upgrade scripts per suggestions
2) The meson build issue has been fixed. Cirrus is passing on that now. I'm still fiddling with my meson setup, so didn't personally test this.
3) The documentation has been updated to no longer include wildcard_upgrades as a variable for control file.
and has this text now describing it's use:
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 to the specified new version. The update script is run in the same environment that CREATE EXTENSION provides for installation scripts: in particular, search_path is set up in the same way, and any new objects created by the script are automatically added to the extension. Also, if the script chooses to drop extension member objects, they are automatically dissociated from the extension.
I built the html docs but ran into a lot of warnings I don't recall getting last time. I think this is just my doc local setup is a bit messed up or something else changed in the doc setup.
My main gripe with this patch is Sandro did not increment the version number of it, so it's the same name as an old patch he had submitted before.
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
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
The problem with an include is that it does nothing for us. We still need to
ship a ton of script files. We've already dealt with the file size issue.
So our PostGIS 3.4.0+ (not yet released) already kind of simulates include
using blank script files that have nothing in them but forces the extension
machinery to upgrade the version to ANY to get to the required installed
version 3.4.0+
So for example postgis--3.3.0--ANY.sql
Would contain this:
-- Just tag extension postgis version as "ANY"
-- Installed by postgis 3.4.0dev
-- Built on ...
And the file has the upgrade steps:
postgis--ANY--3.4.0dev.sql
So that when you are on 3.3.0 and want to upgrade to 3.4.0dev ( it takes
3.3.0 -> ANY -> 3.4.0dev)
The other option I had proposed was getting rid of the micro version, but I
got shot down on that argument -- with PostGIS PSC complaining about people
not being able to upgrade a micro if per chance we have some security patch
released in a micro.
https://lists.osgeo.org/pipermail/postgis-devel/2022-June/029673.html
https://lists.osgeo.org/pipermail/postgis-devel/2022-July/029713.html
Thanks,
Regina
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;
Sandro Santilli <strk@kbt.io> writes:
On Mon, Jan 09, 2023 at 05:51:49PM -0500, Tom Lane wrote:
... 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.
The script-file-per-upgrade-path aspect solves a problem that you
have, whether you admit it or not; I think you simply aren't realizing
that because you have not had to deal with the consequences of
your proposed feature. Namely that you won't have any control
over what the backend will try to do in terms of upgrade paths.
As an example, suppose that a database has foo 4.0 installed, and
the DBA decides to try to downgrade to 3.0. With the system as it
stands, if you've provided foo--4.0--3.0.sql then the conversion
will go through, and presumably it will work because you tested that
that script does what it is intended to. If you haven't provided
any such downgrade script, then ALTER EXTENSION UPDATE will say
"Sorry Dave, I'm afraid I can't do that" and no harm is done.
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. Moreover, there isn't any very
good way to detect or prevent unsupported version transitions.
(I suppose you could add code to look at pg_extension.extversion,
but I'm not sure if that works: it looks to me like we update that
before we run the extension script. Besides which, if you have
to add such code is that really better than having a number of
one-liner scripts implementing the same check declaratively?)
It gets worse though, because above I'm supposing that 4.0 at
least existed when this copy of foo--%--3.0.sql was made.
Suppose that somebody fat-fingered a package upgrade, such that
the extension fileset available to a database containing foo 4.0
now corresponds to foo 3.0, and there's no knowledge of 4.0 at all
in the extension scripts. The DBA trustingly issues ALTER EXTENSION
UPDATE, which will conclude from foo.control that it should update to
3.0, and invoke foo--%--3.0.sql to do it. Maybe the odds of success
are higher than zero, but not by much; 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. Even if that state
of affairs manages not to cause immediate problems, it'll surely
be a mess whenever somebody tries to re-update to 4.0 or later.
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. I'd rather spend our
development effort on a feature that has more than one use-case.
regards, tom lane
Sandro Santilli <strk@kbt.io> writes:
On Mon, Jan 09, 2023 at 05:51:49PM -0500, Tom Lane wrote:
... 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.The script-file-per-upgrade-path aspect solves a problem that you have,
whether you admit it or not; I think you simply aren't realizing that
because
you have not had to deal with the consequences of your proposed feature.
Namely that you won't have any control over what the backend will try to
do in
terms of upgrade paths.
It would be nice if there were some way to apply at least a range match to
upgrades or explicitly state in the control file what paths should be
applied for an upgrade.
Ultimately as Sandro mentioned, it's the 1000 files issue we are trying to
address.
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.
As an example, suppose that a database has foo 4.0 installed, and the DBA
decides to try to downgrade to 3.0. With the system as it stands, if
you've
provided foo--4.0--3.0.sql then the conversion will go through, and
presumably
it will work because you tested that that script does what it is intended
to. If
you haven't provided any such downgrade script, then ALTER EXTENSION
UPDATE will say "Sorry Dave, I'm afraid I can't do that" and no harm is
done.
In our case we've already addressed that in our script, because our upgrades
don't rely on what
extension model tells us is the version, ultimately what our
postgis..version() tells us is the true determinate (one for the lib file
and one for the script).
But you are right, that's a selfish way of thinking about it, cause we have
internal plumbing to handle that issue already and other projects probably
don't.
What I was hoping for was to having a section in the control file to say
something like
Upgrade patterns: 3.0.%--4.0.0.sql, 2.0.%--4.0.0.sql
(and perhaps some logic to guarantee no way to match two patterns)
So you wouldn't be able to have a set of patterns like
Upgrade patterns: %--4.0.0.sql, 3.0.%--4.0.0.sql, 2.0.%--4.0.0.sql
That would allow your proposed include something or other to work and for us
to be able to use that, and still reduce our
file footprint.
I'd even settle for absolute paths stated in the control file if we can
dispense with the need a file path to push you forward.
In that mode, your downgrade issue would never happen even with the way
people normally create scripts now.
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. I'd rather spend our development effort on
a
feature that has more than one use-case.
regards, tom lane
I think you are underestimating the number of extensions that have this
issue and could benefit (agree not in the current incarnation of the patch).
It's not just PostGIS, it's pgRouting (has 56 files), h3 extension (has 37
files in last release, most of them a do nothing, except at the minor update
steps) that have the same issue (and both pgRouting and h3 do have little
bitty script updates that follows the best practice way of doing this).
For pgRouting alone there are 56 files for the last release (of which it can
easily be reduced to about 5 files if the paths could be explicitly stated
in the control file).
Yes all those extensions should dispense with their dreams of having micro
updates (I honestly wish they would).
Perhaps I'm a little obsessive, but it annoys me to see 1000s of files in my
extension folder, when I know even if I followed best practice I only need
like 5 files for each extension.
And as a packager to have to ship these files is even more annoying.
The reality is for micros, no one ships new functions (or at least shouldn't
be), so all functions just need to be replaced perhaps with a micro update
file you propose.
Heck if we could even have the option to itemize our own upgrade file paths
explicitly in the control file,
Like:
paths: 3.0.1--4.0.0 = 3--4.0.0.sql, 3.0.2--4.0.0 = 3--4.0.0.sql,
2--4.0.0.sql = 2.0.2--4.0.0.sql
I'd be happy, and PostgreSQL can do the math pretending there are files it
thinks it needs.
So if we could somehow rework this patch perhaps adding something in the
control to explicitly state the upgrade paths.
I think that would satisfy your concern? And I'd be eternally grateful.
Thanks,
Regina
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
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;
This patch too is conflicting on meson.build. Are these two patches
interdependent?
This one looks a bit more controversial. I'm not sure if Tom has been
convinced and I haven't seen anyone else speak up.
Perhaps this needs a bit more discussion of other options to solve
this issue. Maybe it can just be solved with multiple one-line scripts
that call to a master script?
--
Gregory Stark
As Commitfest Manager
This patch too is conflicting on meson.build. Are these two patches
interdependent?This one looks a bit more controversial. I'm not sure if Tom has been
convinced and I haven't seen anyone else speak up.Perhaps this needs a bit more discussion of other options to solve this issue.
Maybe it can just be solved with multiple one-line scripts that call to a master
script?--
Gregory Stark
As Commitfest Manager
They edit the same file yes so yes conflicts are expected.
The wildcard one, Tom was not convinced, so I assume we'll need to
go a couple more rounds on it and hopefully we can get something that will not be so controversial.
I don't think the schema qualifying required extension feature is controversial though so I think that should be able to go and we'll rebase our other patch after that.
Thanks,
Regina
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 <old_version> to a virtual "ANY" version which we then
use to have a single ANY--<new_version> upgrade path, but we
are still REQUIRED to install a file for any <old_version> we
want to allow upgrade from, which is what this patch is aimed
at fixing.
--strk;
Attachments:
v1-0001-Allow-wildcard-in-extension-upgrade-paths.patchtext/x-diff; charset=us-asciiDownload
From ffefd039f24e3def8d2216b3ac7875d0b6feb8fb Mon Sep 17 00:00:00 2001
From: Sandro Santilli <strk@kbt.io>
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
<literal>1.1</literal>).
</para>
+ <para>
+ The literal value <literal>%</literal> can be used as the
+ <replaceable>old_version</replaceable> 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.
+ </para>
+
<para>
Given that a suitable update script is available, the command
<command>ALTER EXTENSION UPDATE</command> 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;
}
+
+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 c3139ab0fc..4fe2d82b6e 100644
--- a/src/test/modules/test_extensions/Makefile
+++ b/src/test/modules/test_extensions/Makefile
@@ -6,14 +6,16 @@ PGFILEDESC = "test_extensions - regression testing for EXTENSION support"
EXTENSION = test_ext1 test_ext2 test_ext3 test_ext4 test_ext5 test_ext6 \
test_ext7 test_ext8 test_ext_cine test_ext_cor \
test_ext_cyclic1 test_ext_cyclic2 \
- test_ext_evttrig
+ test_ext_evttrig test_ext_wildcard1
DATA = test_ext1--1.0.sql test_ext2--1.0.sql test_ext3--1.0.sql \
test_ext4--1.0.sql test_ext5--1.0.sql test_ext6--1.0.sql \
test_ext7--1.0.sql test_ext7--1.0--2.0.sql test_ext8--1.0.sql \
test_ext_cine--1.0.sql test_ext_cine--1.0--1.1.sql \
test_ext_cor--1.0.sql \
test_ext_cyclic1--1.0.sql test_ext_cyclic2--1.0.sql \
- test_ext_evttrig--1.0.sql test_ext_evttrig--1.0--2.0.sql
+ test_ext_evttrig--1.0.sql test_ext_evttrig--1.0--2.0.sql \
+ test_ext_wildcard1--1.0.sql test_ext_wildcard1--%--2.0.sql \
+
REGRESS = test_extensions test_extdepend
diff --git a/src/test/modules/test_extensions/expected/test_extensions.out b/src/test/modules/test_extensions/expected/test_extensions.out
index 821fed38d1..1c4dc5be42 100644
--- a/src/test/modules/test_extensions/expected/test_extensions.out
+++ b/src/test/modules/test_extensions/expected/test_extensions.out
@@ -312,3 +312,18 @@ Objects in extension "test_ext_cine"
table ext_cine_tab3
(9 rows)
+CREATE EXTENSION test_ext_wildcard1;
+SELECT ext_wildcard1_version();
+ ext_wildcard1_version
+-----------------------
+ 1.0
+(1 row)
+
+ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0';
+SELECT ext_wildcard1_version();
+ ext_wildcard1_version
+-----------------------
+ 2.0
+(1 row)
+
+DROP EXTENSION test_ext_wildcard1;
diff --git a/src/test/modules/test_extensions/meson.build b/src/test/modules/test_extensions/meson.build
index c3af3e1721..026be6a879 100644
--- a/src/test/modules/test_extensions/meson.build
+++ b/src/test/modules/test_extensions/meson.build
@@ -30,6 +30,9 @@ test_install_data += files(
'test_ext_evttrig--1.0--2.0.sql',
'test_ext_evttrig--1.0.sql',
'test_ext_evttrig.control',
+ 'test_ext_wildcard1--1.0.sql',
+ 'test_ext_wildcard1--%--2.0.sql',
+ 'test_ext_wildcard1.control',
)
tests += {
diff --git a/src/test/modules/test_extensions/sql/test_extensions.sql b/src/test/modules/test_extensions/sql/test_extensions.sql
index 41b6cddf0b..071845e8df 100644
--- a/src/test/modules/test_extensions/sql/test_extensions.sql
+++ b/src/test/modules/test_extensions/sql/test_extensions.sql
@@ -209,3 +209,10 @@ CREATE EXTENSION test_ext_cine;
ALTER EXTENSION test_ext_cine UPDATE TO '1.1';
\dx+ test_ext_cine
+
+
+CREATE EXTENSION test_ext_wildcard1;
+SELECT ext_wildcard1_version();
+ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0';
+SELECT ext_wildcard1_version();
+DROP EXTENSION test_ext_wildcard1;
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql b/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
new file mode 100644
index 0000000000..75154e5c55
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
@@ -0,0 +1,6 @@
+/* src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql */
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0'" to load this file. \quit
+
+CREATE OR REPLACE FUNCTION ext_wildcard1_version() returns TEXT
+AS 'SELECT 2.0' LANGUAGE 'sql';
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql b/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
new file mode 100644
index 0000000000..a69e791fda
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
@@ -0,0 +1,6 @@
+/* src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql */
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "CREATE EXTENSION test_ext_wildcard1" to load this file. \quit
+
+CREATE FUNCTION ext_wildcard1_version() returns TEXT
+AS 'SELECT 1.0' LANGUAGE 'sql';
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1.control b/src/test/modules/test_extensions/test_ext_wildcard1.control
new file mode 100644
index 0000000000..0c2fc6fca6
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1.control
@@ -0,0 +1,3 @@
+comment = 'Test extension wildcard 1'
+default_version = '1.0'
+relocatable = true
--
2.34.1
On Tue, Jan 10, 2023 at 6:50 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
The script-file-per-upgrade-path aspect solves a problem that you
have, whether you admit it or not; I think you simply aren't realizing
that because you have not had to deal with the consequences of
your proposed feature. Namely that you won't have any control
over what the backend will try to do in terms of upgrade paths.As an example, suppose that a database has foo 4.0 installed, and
the DBA decides to try to downgrade to 3.0. With the system as it
stands, if you've provided foo--4.0--3.0.sql then the conversion
will go through, and presumably it will work because you tested that
that script does what it is intended to. If you haven't provided
any such downgrade script, then ALTER EXTENSION UPDATE will say
"Sorry Dave, I'm afraid I can't do that" and no harm is done.
I mean, there is nothing to prevent the extension author from writing
a script which ensures that the extension membership after the
downgrade is exactly what it should be for version 3.0, regardless of
what it was before. SQL is Turing-complete.
The thing that confuses me here is why the PostGIS folks are ending up
with so many files. 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.
But that to one side, there is clearly a problem here, and I think
PostgreSQL ought to provide some infrastructure to help solve it, even
if the ultimate cause of that problem is that the PostGIS folks
managed their extension versions in some less-than-ideal way. I can't
shake the feeling that you're just holding your breath here and hoping
the problem goes away by itself, and judging by the responses, that
doesn't seem like it's going to happen.
--
Robert Haas
EDB: http://www.enterprisedb.com
Robert Haas <robertmhaas@gmail.com> writes:
On Tue, Jan 10, 2023 at 6:50 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
As an example, suppose that a database has foo 4.0 installed, and
the DBA decides to try to downgrade to 3.0. With the system as it
stands, if you've provided foo--4.0--3.0.sql then the conversion
will go through, and presumably it will work because you tested that
that script does what it is intended to. If you haven't provided
any such downgrade script, then ALTER EXTENSION UPDATE will say
"Sorry Dave, I'm afraid I can't do that" and no harm is done.
I mean, there is nothing to prevent the extension author from writing
a script which ensures that the extension membership after the
downgrade is exactly what it should be for version 3.0, regardless of
what it was before. SQL is Turing-complete.
It may be Turing-complete, but I seriously doubt that that's sufficient
for the problem I'm thinking of, which is to downgrade from an extension
version that you never heard of at the time the script was written.
In the worst case, that version might even contain objects of types
you never heard of and don't know how to drop.
You can imagine various approaches to deal with that; for instance,
maybe we could provide some kind of command to deal with dropping an
object identified by classid and objid, which the upgrade script
could scrape out of pg_depend. After writing even more code to issue
those drops in dependency-aware order, you could get on with modifying
the objects you do know about ... but maybe those now have properties
you never heard of and don't know how to adjust.
Whether this is all theoretically possible is sort of moot. 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.
But that to one side, there is clearly a problem here, and I think
PostgreSQL ought to provide some infrastructure to help solve it, even
if the ultimate cause of that problem is that the PostGIS folks
managed their extension versions in some less-than-ideal way. I can't
shake the feeling that you're just holding your breath here and hoping
the problem goes away by itself, and judging by the responses, that
doesn't seem like it's going to happen.
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.
regards, tom lane
The thing that confuses me here is why the PostGIS folks are ending up with
so many files.
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.
The contrib files are minor versioned. PostGIS is micro versioned.
So we have for example postgis--3.3.0--3.3.1.sql
Also we have 5 extensions we ship all micro versions, so multiply that issue by 5
postgis
postgis_raster
postgis_topology
postgis_tiger_geocoder
postgis_sfcgal
The other wrinkle we have that I don't think postgresql's contrib have is that we support
Each version across multiple PostgreSQL versions.
So for example our 3.3 series supports PostgreSQL 12-15 (with plans to also support 16).
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.regards, tom lane
How about the idea I mentioned, of we revise the patch to read versioned upgrades from the control file
rather than relying on said file to exist.
/messages/by-id/000201d92572$7dcd8260$79688720$@pcorp.us
Even better, we have an additional control file, something like
postgis--paths.control
That has separate lines to itemize those paths. It would be nice if we could allow wild-cards in that, but I could live without that if we can stop shipping 300 empty files.
Thanks,
Regina
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;
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;
On Wed, Mar 8, 2023 at 7:27 AM Sandro Santilli <strk@kbt.io> wrote:
Now, PostGIS has released a total of 164 versions:
That is a LOT of versions. PostGIS must release really frequently, I guess?
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).
Well, that's certainly the design intention here. The point of the
extension version number from the point of PostgreSQL is to provide a
way to cause SQL changes to happen via extension upgrade steps, so if
there's no SQL change required then the version number change has no
purpose from PostgreSQL's point of view. But I do see a problem with
that theory, which is that you might quite reasonably want the
extension version number and your own release version number to match.
That problem doesn't arise for the extensions bundled with core
because every new core release has a new PostgreSQL version number,
which is entirely sufficient to identify the fact that there may have
been C code changes, so we have no motivation to bump the extension
version number unless we need a SQL change. This is true even across
major releases -- an extension in contrib can go for years without an
SQL version bump even though the C code may change repeatedly to
accommodate new major releases.
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. But it gives you a way to do that
which does not require creating a bazillion empty files. Instead you
can just declare that if you're running the upgrade script from
14.36.279 to 14.36.280, that also suffices for an upgrade from
14.36.278 or 14.36.277 or 14.36.276 or ....
--
Robert Haas
EDB: http://www.enterprisedb.com
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. But it gives you a way to do
that which does not require creating a bazillion empty files. Instead you can
just declare that if you're running the upgrade script from
14.36.279 to 14.36.280, that also suffices for an upgrade from
14.36.278 or 14.36.277 or 14.36.276 or ....--
Robert Haas
EDB: http://www.enterprisedb.com
That's what I was proposing as a compromise.
Just not sure what the appropriate name would be for the map file.
Originally I thought maybe we could put it in the .control, but decided
it's better to have as a separate file that way we don't need to change the control and just add this extra file for PostgreSQL 16+.
Then question arises if you have such a map file and you have files as well (the old way).
Would we meld the two, so the map file would be used to simulate the file paths and these get added on as extra target paths or should we just assume if there is a map file, then that takes precedence over any paths inferred by files in the system.
Thanks,
Regina
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
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;
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,
1968 lines in one control file, is still much nicer than 1968 files in my
book.
From a packaging standpoint also much cleaner, as that single file gets
replaced with each upgrade and no need to uninstall more junk from prior
install.
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
That said, I really would like a wildcard option for issues strk mentioned.
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
In many cases, they don't even need anything done in an upgrade step, aside
from moving the dial button on extension up a notch to coincide with the lib
version.
In our case, yes ours would be below because we already block downgrades
internally in our scripts
%--3.4.0 = ANY--3.4.0
Or we could move to a
2.%--3.4.0 = 2--3.4.0
3.%--3.4.0 = 3--3.4.0
Thanks,
Regina
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;
Attachments:
v2-0001-Allow-wildcard-in-extension-upgrade-paths.patchtext/x-diff; charset=us-asciiDownload
From a10a1a7200f76bbc6e2def8d4684b647a99316cd Mon Sep 17 00:00:00 2001
From: Sandro Santilli <strk@kbt.io>
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
<literal>1.1</literal>).
</para>
+ <para>
+ The literal value <literal>%</literal> can be used as the
+ <replaceable>old_version</replaceable> 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.
+ </para>
+
<para>
Given that a suitable update script is available, the command
<command>ALTER EXTENSION UPDATE</command> 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(),
+ 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 70fc0c8e66..beec04eea3 100644
--- a/src/test/modules/test_extensions/Makefile
+++ b/src/test/modules/test_extensions/Makefile
@@ -7,8 +7,8 @@ EXTENSION = test_ext1 test_ext2 test_ext3 test_ext4 test_ext5 test_ext6 \
test_ext7 test_ext8 test_ext_cine test_ext_cor \
test_ext_cyclic1 test_ext_cyclic2 \
test_ext_evttrig \
- test_ext_req_schema1 test_ext_req_schema2 test_ext_req_schema3
-
+ test_ext_req_schema1 test_ext_req_schema2 test_ext_req_schema3 \
+ test_ext_evttrig test_ext_wildcard1
DATA = test_ext1--1.0.sql test_ext2--1.0.sql test_ext3--1.0.sql \
test_ext4--1.0.sql test_ext5--1.0.sql test_ext6--1.0.sql \
test_ext7--1.0.sql test_ext7--1.0--2.0.sql test_ext8--1.0.sql \
@@ -18,7 +18,8 @@ DATA = test_ext1--1.0.sql test_ext2--1.0.sql test_ext3--1.0.sql \
test_ext_evttrig--1.0.sql test_ext_evttrig--1.0--2.0.sql \
test_ext_req_schema1--1.0.sql \
test_ext_req_schema2--1.0.sql \
- test_ext_req_schema3--1.0.sql
+ test_ext_req_schema3--1.0.sql \
+ test_ext_wildcard1--1.0.sql test_ext_wildcard1--%--2.0.sql \
REGRESS = test_extensions test_extdepend
diff --git a/src/test/modules/test_extensions/expected/test_extensions.out b/src/test/modules/test_extensions/expected/test_extensions.out
index a31775a260..790b9b9368 100644
--- a/src/test/modules/test_extensions/expected/test_extensions.out
+++ b/src/test/modules/test_extensions/expected/test_extensions.out
@@ -389,3 +389,19 @@ SELECT test_s_dep.dep_req2();
DROP EXTENSION test_ext_req_schema1 CASCADE;
NOTICE: drop cascades to extension test_ext_req_schema2
+
+CREATE EXTENSION test_ext_wildcard1;
+SELECT ext_wildcard1_version();
+ ext_wildcard1_version
+-----------------------
+ 1.0
+(1 row)
+
+ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0';
+SELECT ext_wildcard1_version();
+ ext_wildcard1_version
+-----------------------
+ 2.0
+(1 row)
+
+DROP EXTENSION test_ext_wildcard1;
diff --git a/src/test/modules/test_extensions/meson.build b/src/test/modules/test_extensions/meson.build
index 29e5bb2fb5..7b14d65545 100644
--- a/src/test/modules/test_extensions/meson.build
+++ b/src/test/modules/test_extensions/meson.build
@@ -36,6 +36,9 @@ test_install_data += files(
'test_ext_req_schema2.control',
'test_ext_req_schema3--1.0.sql',
'test_ext_req_schema3.control',
+ 'test_ext_wildcard1--1.0.sql',
+ 'test_ext_wildcard1--%--2.0.sql',
+ 'test_ext_wildcard1.control',
)
tests += {
diff --git a/src/test/modules/test_extensions/sql/test_extensions.sql b/src/test/modules/test_extensions/sql/test_extensions.sql
index f4947e7da6..676face363 100644
--- a/src/test/modules/test_extensions/sql/test_extensions.sql
+++ b/src/test/modules/test_extensions/sql/test_extensions.sql
@@ -232,3 +232,12 @@ ALTER EXTENSION test_ext_req_schema1 SET SCHEMA test_s_dep2; -- now ok
SELECT test_s_dep2.dep_req1();
SELECT test_s_dep.dep_req2();
DROP EXTENSION test_ext_req_schema1 CASCADE;
+
+--
+-- Test wildcard upgrade
+--
+CREATE EXTENSION test_ext_wildcard1;
+SELECT ext_wildcard1_version();
+ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0';
+SELECT ext_wildcard1_version();
+DROP EXTENSION test_ext_wildcard1;
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql b/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
new file mode 100644
index 0000000000..75154e5c55
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
@@ -0,0 +1,6 @@
+/* src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql */
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0'" to load this file. \quit
+
+CREATE OR REPLACE FUNCTION ext_wildcard1_version() returns TEXT
+AS 'SELECT 2.0' LANGUAGE 'sql';
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql b/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
new file mode 100644
index 0000000000..a69e791fda
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
@@ -0,0 +1,6 @@
+/* src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql */
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "CREATE EXTENSION test_ext_wildcard1" to load this file. \quit
+
+CREATE FUNCTION ext_wildcard1_version() returns TEXT
+AS 'SELECT 1.0' LANGUAGE 'sql';
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1.control b/src/test/modules/test_extensions/test_ext_wildcard1.control
new file mode 100644
index 0000000000..0c2fc6fca6
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1.control
@@ -0,0 +1,3 @@
+comment = 'Test extension wildcard 1'
+default_version = '1.0'
+relocatable = true
--
2.34.1
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.
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).
On Tue, Jan 10, 2023 at 5:52 AM Tom Lane <tgl@sss.pgh.pa.us> 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.
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--<current> upgrade path
will still be the "catch-all" upgrade script.
--strk;
Show quoted text
On Tue, Jan 10, 2023 at 5:52 AM Tom Lane <tgl@sss.pgh.pa.us> 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.
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 aretrivial).
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--<current> upgrade path will still be the
"catch-all" upgrade script.--strk;
Sounds like a user and packaging nightmare to me.
How is a packager to know which versions a user might have installed?
and leaving that to users to run an extra command sounds painful. They barely know how to run
ALTER EXTENSION postgis UPDATE;
Or pg_upgrade
I much preferred the idea of just listing all our upgrade targets in the control file.
The many annoyance I have left is the 1000 of files that seem to grow forever.
This isn't just postgis. It's pgRouting, it's h3_pg, it's mobilitydb.
I don't want to have to set this up for every single extension that does micro-updates.
I just want a single file that can list all the target paths worst case.
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
major.minor.micro
In almost all cases the minor upgrade script works for all micros of that minor and in postgis yes we have a single script that works for all cases.
Thanks,
Regina
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 aretrivial).
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--<current> 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;
Packager might actually know better in that they could ONLY consider the
packages ever packaged by them.
I'm a special case packager cause I'm on the PostGIS project and I only
package postgis related extensions, but even I find this painful.
But for most packagers, I think they are juggling too many packages and too
many OS versions to micro manage the business of each package.
In my case my job is simple. I deal just with Windows and that doesn't
change from Windows version to Windows version (just PG specific).
Think of upgrading from Debian 10 to Debian 12 - what would you as a PG
packager expect people to be running and upgrading from?
They could be switching from say the main distro to the pgdg distro.
Hey, best would be having support for wildcard wouldn't it ?
For PostGIS yes and any other extension that does nothing but add new
functions or replaces existing ones. For others some minor handling would
be ideal, though I guess some other projects would be happy with a wildcard
(e.g. pgRouting would prefer a wildcard) since most of the changes are just
additions of new functions or replacements of existing functions.
For something like h3-pg I think a simpler micro handling would be ideal,
though not sure.
They ship two extensions (one that is a bridge to postgis and their newest
takes advantage of postgis_raster too)
https://github.com/zachasme/h3-pg/tree/main/h3_postgis/sql/updates
https://github.com/zachasme/h3-pg/tree/main/h3/sql/updates
Many of their upgrades are No-ops cause they really are just lib upgrades.
I'm thinking maybe we should discuss these ideas with projects who would
benefit most from this:
(many of which I'm familiar with because they are postgis offsprings and I
package or plan to package them - pgRouting, h3-pg, pgPointCloud,
mobilityDb,
Not PostGIS offspring:
ZomboDB - https://github.com/zombodb/zombodb/tree/master/sql - (most of
those are just changing versioning on the function call, so yah wildcard
would be cleaner there)
TimescaleDB - https://github.com/timescale/timescaledb/tree/main/sql/updates
( I don't think a wildcard would work here especially since they have some
downgrade paths, but is a useful example of a micro-level extension upgrade
pattern we should think about if we could make easier)
https://github.com/timescale/timescaledb/tree/main/sql/updates
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 already do, remember you wrote it :)
https://git.osgeo.org/gitea/postgis/postgis/src/branch/master/extensions/upg
radeable_versions.mk
Yes it does require manual updating each release cycle (and putting in
versions from just released stable branches). I can live with continuing
with that exercise. It was a nice to have but is not nearly as annoying as
1000 scripts or as a packager trying to catalog what versions of packages
have I released that I need to worry about.
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
patternI 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;
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.
It's cleaner to read a single file than make sense of a pile of files.
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;
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.
From a packaging stand-point 1 big file is better than tons of 0-length
files.
Fewer files to uninstall and to double check when testing.
As to the added complexity agree it's more but in my mind worth it if we
could get rid of this mountain of files.
But my vote would be the wild-card solution as I think it would serve more
than just postgis need.
Any project that rarely does anything but add, remove, or modify functions
doesn't really need multiple upgrade scripts and
I think quite a few extensions fall in that boat.
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;
I think we always have more releases with newer stable versions than older
stable versions.
I can't remember a case when we had this ONLY issue.
If there is one fix in an older stable, version we usually wait for several
more before bothering with a release
and then all the stables are released around the same time. So I don't see
the ONLY being a real problem.
Here are my thoughts of how this can work to satisfy our specific needs and
that of others who have many micro versions.
1) We define an additional file. I'll call this a paths file
So for example postgis would have a
postgis.paths file
The format of the path file would be of the form
<version pattern1>,<version pattern2> => 3.3.0--3.4.0
It will also allow a wildcard option
% => ANY--3.4.0.sql
So a postgis.paths with multiple lines might look like
3.2.0,3.2.1 => 3.2.2--3.3.0
3.3.% => 3.3--3.4.0
% => ANY--3.4.0
2) The order of precedence would be:
a) physical files are always used first
b) If no physical path is present on disk, then it looks at a
<component>.paths file to formulate virtual paths
c) Longer mappings are preferred over shorter mappings
So that means the % => ANY--3.4.0 would be the path of last resort
Let's say our current installed version of postgis is postgis VERSION 3.2.0
The above path formulated would be
3.2.0 -> 3.3.0 -> 3.4.0
The simulated scripts used to get there would be
postgis--3.2.2--3.3.0.sql
postgis--3.3.0--3.4.0.sql
This however does not solve the issue of downgrades, which admittedly
postgis is not concerned about since we have accounted for that in our
internal scripts.
So we might have issue with having a bear: %. If we don't allow a bear %
Then our postgis patterns might look something like:
3.%, 2.% => ANY --3.4.0
Which would mean 3.0.1, 3.0.2, 3.2.etc would all use the same script.
Which would still be a major improvement from what we have today and
minimizes likeliness of downgrade footguns.
Thoughts anyone?
Here are my thoughts of how this can work to satisfy our specific needs
and
that of others who have many micro versions.
1) We define an additional file. I'll call this a paths file
So for example postgis would have a
postgis.paths file
The format of the path file would be of the form
<version pattern1>,<version pattern2> => 3.3.0--3.4.0
It will also allow a wildcard option
% => ANY--3.4.0.sqlSo a postgis.paths with multiple lines might look like
3.2.0,3.2.1 => 3.2.2--3.3.0
3.3.% => 3.3--3.4.0
% => ANY--3.4.02) The order of precedence would be:
a) physical files are always used first
b) If no physical path is present on disk, then it looks at a
<component>.paths
file to formulate virtual paths
c) Longer mappings are preferred over shorter mappingsSo that means the % => ANY--3.4.0 would be the path of last resort
Let's say our current installed version of postgis is postgis VERSION
3.2.0
The above path formulated would be
3.2.0 -> 3.3.0 -> 3.4.0
The simulated scripts used to get there would bepostgis--3.2.2--3.3.0.sql
postgis--3.3.0--3.4.0.sqlThis however does not solve the issue of downgrades, which admittedly
postgis is not concerned about since we have accounted for that in our
internal scripts.So we might have issue with having a bear: %. If we don't allow a bear %
Then our postgis patterns might look something like:
3.%, 2.% => ANY --3.4.0
Which would mean 3.0.1, 3.0.2, 3.2.etc would all use the same script.
Which would still be a major improvement from what we have today and
minimizes likeliness of downgrade footguns.Thoughts anyone?
Minor correction scripts to get from 3.2.0 to 3.4.0 would be:
postgis--3.2.2--3.3.0.sql
postgis--3.3--3.4.0.sql
Import Notes
Reply to msg id not found:
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.
Having one big upgrade file, at its core, enables you to do is move the
logic for "what parts of the script to run for an upgrade from version V1
to V2" from a script to generate .sql files to Plpgsql in the form of
something like:
DO$$
IF <guard> THEN
<execute upgrade>
<record metadata>
END IF;
$$;
Where the guard can check postgres catalogs, internal extension catalogs,
or anything else the extension wants. What we have done in the past is
record executions of non-idempotent migration scripts in an
extension catalog table and simply check if that row exists in the guard.
This allows for much easier management of backporting patches, for
instance. Having the full power of Plpgsql makes all of this much easier
and (more flexible) than in a build script that compiles various V1--v2.sql
upgrade files.
Currently, when we did this in Promscale, we also added V1--v2.sql upgrade
files as symlinks to "the one big file". Not the end of the world, but I
think a patch like the one suggested in this thread will make things a lot
nicer.
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 think this concern is a good reason to have this
functionality enabled in the control file. That way, this issue can be
documented and then only extensions that test for valid upgrades in the
script can enable this. Such an "opt-in" prevents people who haven't
thought of the need to validate the version changes from using this by
accident.
I'll add two more related points:
1) When the extension framework was first implemented I don't believe DO
blocks existed, so handling upgrade logic in the script itself just wasn't
possible. That's why I think rethinking some of the design now makes sense.
2) This change also makes it easier for extensions that use versioned .so
files (by that I mean uses extension-<version>.so rather than
extension.so). Because such extensions can't really use the chaining
property of the existing upgrade system and so need to write a
direct X--Y.sql migration file for every prior version X. I know the system
wasn't designed for this, but in reality a lot of extensions do this.
Especially the more complex ones.
Hopefully this is helpful,
Mat
This change also makes it easier for extensions that use versioned .so files (by that I mean uses extension-<version>.so rather than extension.so).
Because such extensions can't really use the chaining property of the existing upgrade system and so need to write a direct X--Y.sql migration file for
every prior version X. I know the system wasn't designed for this, but in reality a lot of extensions do this. Especially the more complex ones.
Hopefully this is helpful,
Mat
Thanks for the feedback. Yes this idea of versioned .so is also one of the reasons why we always have to run a replace on all functions.
Like for example on PostGIS side, we have option of full minor (so the lib could be postgis-3.3.so or postgis-3.so)
For development I always include the minor version and the windows interim builds I build against our master branch has the minor.
But the idea is someone can upgrade to stable version, so the script needs to always have the .so version in it to account for this shifting of options.
Thanks,
Regina
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;
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 was thinking about this more. The extension model as it stands doesn't
allow an extension author to define version hierarchy. We handle this
internally in our scripts to detect a downgrade attempt and stop it similar
to what Mat is saying.
Most of that is basically converting our version string to a numeric number
which we largely do with a regex pattern.
I was thinking if there were some way to codify that regex pattern in our
control file, then wild cards can only be applied if such a pattern is
defined and the
%--<target version>
The % has to be numerically before the target version.
Thanks,
Regina
(I'm the developer of ZomboDB and pgrx, which while not an extension per se, allows others to make extensions that then need upgrade scripts. So this topic is interesting to me.)
On Mar 13, 2023, at 2:48 PM, Regina Obe <lr@pcorp.us> 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,1968 lines in one control file, is still much nicer than 1968 files in my
book.
From a packaging standpoint also much cleaner, as that single file gets
replaced with each upgrade and no need to uninstall more junk from prior
install.
I tend to agree with this. I like this mapping file idea. Allowing an extension to declare what to use (Z) to get from X to Y is great. In theory that's not much different than having a bunch of individual files, but in practice an extension likely could pare their actual file set down to just a few, and dealing with less files is a big win. And then the mapping file would allow the extension author to make the tree as complex as necessary.
(I have some hand-wavy ideas for pgrx and autogenerating upgrade scripts and a file like this could help quite a bit. Rust and rust-adjacent things prefer explicitness)
If this "wildcard in a filename" idea existed back in 2015 when I started ZomboDB I'm not sure I'd have used it, and I'm not sure I'd want to switch to using it now. The ambiguities are too great when what I want is something that's obvious.
Primarily, ZDB purposely doesn't support downgrade paths so I wouldn't want to use a pattern that implies it does.
ZomboDB has 137 releases over the past 8 years. Each one of those adds an upgrade script from OLDVER--NEWVER. Prior to the Rust rewrite, these were all hand-generated, and sometimes were just zero bytes. I've since developed tooling to auto-generate upgrade scripts by diffing the full schemas for OLDVER and NEWVER and emitting whatever DDL can move OLDVER to NEWVER. In practice this usually generates an upgrade script that just replaces 1 function... the "zdb.version()" function. Of course, I've had to hand-edit these on occasion as well -- this is not a perfect system.
It might just be the nature of our extensions, but I don't recall ever needing DO statements in an upgrade script. The extension.sql has a few, one of which is to optionally enable PostGIS support! haha ZDB is fairly complex too. Hundreds of functions, dozens of types, an IAM implementation, a dozen views, a few tables, some operators. I also don't see a lot of changes to ZDB's extension schema nowadays -- new releases are usually fixing some terrible bug.
(As an aside, I wish Postgres would show the line number in whatever .sql file when an ERROR occurs during CREATE EXTENSION or ALTER EXTENSION UPDATE. That'd be a huge QoL improvement for me -- maybe it's my turn to put a patch together)
Just some musings from a guy hanging out here on the fringes of Postgres extension development. Of the things I've seen in this thread I really like the mapping file idea and I don't have any original thoughts on the subject.
eric
On Fri, Apr 28, 2023 at 10:03 AM Eric Ridge <eebbrr@gmail.com> wrote:
ZomboDB has 137 releases over the past 8 years.
Dang.
This may be one of those cases where the slow pace of change for
extensions shipped with PostgreSQL gives those of us who are
developers for PostgreSQL itself a sort of tunnel vision. The system
that we have in core works well for those extensions, which get a new
version at most once a year and typically much less often than that.
I'm not sure we'd be so sanguine about the status quo if we had 137
releases out there.
--
Robert Haas
EDB: http://www.enterprisedb.com
On May 1, 2023, at 12:12 PM, Robert Haas <robertmhaas@gmail.com> wrote:
On Fri, Apr 28, 2023 at 10:03 AM Eric Ridge <eebbrr@gmail.com> wrote:
ZomboDB has 137 releases over the past 8 years.
Dang.
This may be one of those cases where the slow pace of change for
extensions shipped with PostgreSQL gives those of us who are
developers for PostgreSQL itself a sort of tunnel vision.
Could be I'm a terrible programmer too. Could be Elasticsearch is a PITA to deal with.
FWIW, outside of major ZDB releases, most of those have little-to-zero schema changes. But that doesn't negate the fact each release needs its own upgrade.sql script. I'm working on a point release right this moment and it won't have any schema changes but it'll have an upgrade.sql script.
Maybe related, pgrx (formally pgx) has had 77 release since June 2020, but that's mostly us just slowly trying to wrap Postgres internals in Rust, bug fixing, and such. Getting support for a new major Postgres release usually isn't *that* hard -- a day or two of work, depending on how drastically y'all changed an internal API. `List` and `FunctionCallInfo(Base)Data` come to mind as minor horrors for us, hahaha. Rust at least makes it straightforward for us to have divergent implementations and have the decisions solved at compile time.
The system
that we have in core works well for those extensions, which get a new
version at most once a year and typically much less often than that.
I'm not sure we'd be so sanguine about the status quo if we had 137
releases out there.
I don't lose sleep over it but it is a lot of bookkeeping. The nice thing about Postgres' extension system is that we can do what we want without regard to the project's release schedule. In general, y'all nailed the extension system back in the day.
I feel for the PostGIS folks as they've clearly got more, hmm, complete Postgres version support than ZDB does, and I'd definitely want to support anything that would make their lives easier. But I also don't want to see something that pgrx users might want to adopt that might turn out to be bad for them.
I have a small list of things I'd love to see changed wrt extensions but I'm just the idea guy and don't have much in the way of patches to offer. I also have some radical ideas about annotating the sources themselves, but I don't feel like lobbing a grenade today.
eric
Eric Ridge <eebbrr@gmail.com> writes:
FWIW, outside of major ZDB releases, most of those have little-to-zero schema changes. But that doesn't negate the fact each release needs its own upgrade.sql script. I'm working on a point release right this moment and it won't have any schema changes but it'll have an upgrade.sql script.
Hmm ... our model for the in-core extensions has always been that you
don't need a new upgrade script unless the SQL declarations change.
Admittedly, that means that the script version number isn't a real
helpful guide to which version of the .so you're dealing with.
We expect the .so's own minor version number to suffice for that,
but I realize that that might not be the most user-friendly answer.
regards, tom lane
On May 1, 2023, at 4:24 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Eric Ridge <eebbrr@gmail.com> writes:
FWIW, outside of major ZDB releases, most of those have little-to-zero schema changes. But that doesn't negate the fact each release needs its own upgrade.sql script. I'm working on a point release right this moment and it won't have any schema changes but it'll have an upgrade.sql script.
Hmm ... our model for the in-core extensions has always been that you
don't need a new upgrade script unless the SQL declarations change.
That's a great model when the schemas only change once every few years or never.
Admittedly, that means that the script version number isn't a real
helpful guide to which version of the .so you're dealing with.
It isn't. ZDB, and I think (at least) PostGIS, have their own "version()" function. Keeping everything the same version keeps me "sane" and eliminates a class of round-trip questions with users.
So when I say "little-to-zero schema changes" I mean that at least the version() function changes -- it's a `LANGUAGE sql` function for easy human inspection.
We expect the .so's own minor version number to suffice for that,
but I realize that that might not be the most user-friendly answer.
One of my desires is that the on-disk .so's filename be associated with the pg_extension entry and not Each. Individual. Function. There's a few extensions that like to version the on-disk .so's filename which means a CREATE OR REPLACE for every function on every extension version bump. That forces an upgrade script even if the schema didn't technically change and also creates the need for bespoke tooling around extension.sql and upgrade.sql scripts.
But I don't want to derail this thread.
eric
It isn't. ZDB, and I think (at least) PostGIS, have their own "version()"
function.
Keeping everything the same version keeps me "sane" and eliminates a class
of round-trip questions with users.
Yes we have several version numbers and yes we too like to keep the
extension version the same, cause it's the only thing that is universally
consistent across all PostgreSQL extensions.
Yes we have our own internal version functions. One for PostGIS (has the
true lib version file) and a script version and we have logic to make sure
they are aligned. We even have versions for our dependencies (PROJ, GEOS,
GDAL) cause behavior of PostGIS changes based on versions of those.
This goes for each extension we package that has a lib file (postgis,
postgis_raster, postgis_topology, postgis_sfcgal)
In addition to that we also have a version for PostgreSQL (that the scripts
were installed on). To catch cases when a pg_upgrade is needed to go from
3.3.0 to 3.3.0.
Yes we need same version upgrades (particularly because of pg_upgrade).
Sandro and I were talking about this. This is something we do in our
postgis_extensions_upgrade() (basically forcing an upgrade to a version
that does nothing, so we can force an upgrade to 3.3.0 again) to make up for
this limitation in extension model.
The reason for that is features get exposed based on version of PostgreSQL
you are running.
So in 3.3.0 we leveraged the new gist fast indexing build, which is only
enabled for users running PostgreSQL 15 and above.
What usually happens is someone has PostGIS 3.3.0 say on PG 14, they
pg_upgrade to PG 15 but they are running with PG 14 scripts
So they are not taking advantage of the new PG 15 features until they do a
SELECT postgis_extensions_upgrade();
So this is why we need the DO commands for scenarios like this.
One of my desires is that the on-disk .so's filename be associated with
the
pg_extension entry and not Each. Individual. Function. There's a few
extensions that like to version the on-disk .so's filename which means a
CREATE OR REPLACE for every function on every extension version bump.
That forces an upgrade script even if the schema didn't technically change
and
also creates the need for bespoke tooling around extension.sql and
upgrade.sql scripts.But I don't want to derail this thread.
eric=
This is more or less the reason why we had to do CREATE OR REPLACE for all
our functions.
In the past we minor versioned our lib files so we had postgis-2.4,
postgis-2.5
At 3.0 after much in-fighting (battle between convenience of developers vs.
regular old users just wanting to use PostGIS and my frustration trying to
hold peoples hands thru pg_upgrade), we settled on major version for the lib
file, with option for developers to still keep the minor.
So default install will be postgis-3 for life of 3 series and become
postgis-4 when 4 comes along (hopefully not for another 10 years).
Completely stripping the version we decided not to do cause with the change
we have a whole baggage of legacy functions we needed to stub as we removed
them so pg_upgrade will work more or less seamlessly. So come postgis-4
these stubs will be removed.
Our CI bots however many of them do use the minor versionings 3.1, 3.2, 3.3
etc, cause it's easier to test upgrades and do regressions.
And many PostGIS developers do the same. So a replace of all functions is
still largely needed. This is one of the reasons the whole chained upgrade
path never worked for us and why we settled on one script to handle
everything.
On Mon, May 1, 2023 at 11:05 PM Eric Ridge <eebbrr@gmail.com> wrote:
We expect the .so's own minor version number to suffice for that,
but I realize that that might not be the most user-friendly answer.One of my desires is that the on-disk .so's filename be associated with
the pg_extension entry and not Each. Individual. Function. There's a few
extensions that like to version the on-disk .so's filename which means a
CREATE OR REPLACE for every function on every extension version bump. That
forces an upgrade script even if the schema didn't technically change and
also creates the need for bespoke tooling around extension.sql and
upgrade.sql scripts.
I understand the potential pain with this (though I suppose MODULE_PATHNAME
can somewhat alleviate it). However, I'd like to highlight the fact
that, besides the fact that control files contain a reference to a .so
file, there's very little in the way of actual dependency of the extension
mechanism on shared libraries, as that part relies on functions being able
to use C language for their implementation. Nothing I am aware of would
stop you from having multiple .so files in a given extension (for one
reason or another reason), and I think that's actually a great design,
incidentally or not. This does allow for a great deal of flexibility and an
escape hatch for when the straightforward case doesn't work [1]https://yrashk.com/blog/2023/04/10/avoiding-postgres-extensions-limitations/ -- https://omnigr.es Yurii.
If the extension subsystem wasn't replacing MODULE_PATHNAME, I don't think
there would be a reason for having `module_pathname` in the control file.
It doesn't preload the file or call anything from it. It's what `create
function` in the scripts will do. And that's actually great.
I am referencing this not because I don't think you know this but to try
and capture the higher-level picture here.
This doesn't mean we shouldn't improve the UX/DX of one of the common and
straightforward cases (having a single .so file with no other
complications) where possible.
[1]: https://yrashk.com/blog/2023/04/10/avoiding-postgres-extensions-limitations/ -- https://omnigr.es Yurii.
https://yrashk.com/blog/2023/04/10/avoiding-postgres-extensions-limitations/
--
https://omnigr.es
Yurii.
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.
The name "ANY" should also be allowed as the source version.
Some extensions are a collection of CREATE OR REPLACE FUNCTION. All
upgrades are one and the same SQL file.
For example: address_standardizer_data_us--ANY--3.2.4.sql
--
Przemysław Sztoch | Mobile +48 509 99 00 66
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.sqlThe 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;
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;
Attachments:
v3-0001-Allow-wildcard-in-extension-upgrade-paths.patchtext/x-diff; charset=us-asciiDownload
From 466338401ce8305b7ac9aa59386816c3a6884f02 Mon Sep 17 00:00:00 2001
From: Sandro Santilli <strk@kbt.io>
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 ...
</para>
</listitem>
</varlistentry>
+
+ <varlistentry id="extend-extensions-wildcard-upgrade">
+ <term><varname>wildcard_upgrades</varname> (<type>boolean</type>)</term>
+ <listitem>
+ <para>
+ This parameter, if set to <literal>true</literal> (which is not the
+ default), allows <command>ALTER EXTENSION</command> to consider
+ a wildcard character <literal>%</literal> as matching any version of
+ the extension. Such wildcard match will only be used when no
+ perfect match is found for a version.
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
<para>
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, 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
@@ -1259,13 +1278,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)));
}
/*
@@ -3470,3 +3499,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 70fc0c8e66..5a8205fd5d 100644
--- a/src/test/modules/test_extensions/Makefile
+++ b/src/test/modules/test_extensions/Makefile
@@ -7,8 +7,8 @@ EXTENSION = test_ext1 test_ext2 test_ext3 test_ext4 test_ext5 test_ext6 \
test_ext7 test_ext8 test_ext_cine test_ext_cor \
test_ext_cyclic1 test_ext_cyclic2 \
test_ext_evttrig \
- test_ext_req_schema1 test_ext_req_schema2 test_ext_req_schema3
-
+ test_ext_req_schema1 test_ext_req_schema2 test_ext_req_schema3 \
+ test_ext_wildcard1
DATA = test_ext1--1.0.sql test_ext2--1.0.sql test_ext3--1.0.sql \
test_ext4--1.0.sql test_ext5--1.0.sql test_ext6--1.0.sql \
test_ext7--1.0.sql test_ext7--1.0--2.0.sql test_ext8--1.0.sql \
@@ -18,7 +18,8 @@ DATA = test_ext1--1.0.sql test_ext2--1.0.sql test_ext3--1.0.sql \
test_ext_evttrig--1.0.sql test_ext_evttrig--1.0--2.0.sql \
test_ext_req_schema1--1.0.sql \
test_ext_req_schema2--1.0.sql \
- test_ext_req_schema3--1.0.sql
+ test_ext_req_schema3--1.0.sql \
+ test_ext_wildcard1--1.0.sql test_ext_wildcard1--%--2.0.sql
REGRESS = test_extensions test_extdepend
diff --git a/src/test/modules/test_extensions/expected/test_extensions.out b/src/test/modules/test_extensions/expected/test_extensions.out
index a31775a260..0d4d8b4b70 100644
--- a/src/test/modules/test_extensions/expected/test_extensions.out
+++ b/src/test/modules/test_extensions/expected/test_extensions.out
@@ -389,3 +389,18 @@ SELECT test_s_dep.dep_req2();
DROP EXTENSION test_ext_req_schema1 CASCADE;
NOTICE: drop cascades to extension test_ext_req_schema2
+CREATE EXTENSION test_ext_wildcard1;
+SELECT ext_wildcard1_version();
+ ext_wildcard1_version
+-----------------------
+ 1.0
+(1 row)
+
+ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0';
+SELECT ext_wildcard1_version();
+ ext_wildcard1_version
+-----------------------
+ 2.0
+(1 row)
+
+DROP EXTENSION test_ext_wildcard1;
diff --git a/src/test/modules/test_extensions/sql/test_extensions.sql b/src/test/modules/test_extensions/sql/test_extensions.sql
index f4947e7da6..3c40710fc1 100644
--- a/src/test/modules/test_extensions/sql/test_extensions.sql
+++ b/src/test/modules/test_extensions/sql/test_extensions.sql
@@ -232,3 +232,12 @@ ALTER EXTENSION test_ext_req_schema1 SET SCHEMA test_s_dep2; -- now ok
SELECT test_s_dep2.dep_req1();
SELECT test_s_dep.dep_req2();
DROP EXTENSION test_ext_req_schema1 CASCADE;
+
+--
+-- Test wildcard based upgrade paths
+--
+CREATE EXTENSION test_ext_wildcard1;
+SELECT ext_wildcard1_version();
+ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0';
+SELECT ext_wildcard1_version();
+DROP EXTENSION test_ext_wildcard1;
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql b/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
new file mode 100644
index 0000000000..75154e5c55
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
@@ -0,0 +1,6 @@
+/* src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql */
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0'" to load this file. \quit
+
+CREATE OR REPLACE FUNCTION ext_wildcard1_version() returns TEXT
+AS 'SELECT 2.0' LANGUAGE 'sql';
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql b/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
new file mode 100644
index 0000000000..a69e791fda
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
@@ -0,0 +1,6 @@
+/* src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql */
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "CREATE EXTENSION test_ext_wildcard1" to load this file. \quit
+
+CREATE FUNCTION ext_wildcard1_version() returns TEXT
+AS 'SELECT 1.0' LANGUAGE 'sql';
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1.control b/src/test/modules/test_extensions/test_ext_wildcard1.control
new file mode 100644
index 0000000000..865e37fa88
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1.control
@@ -0,0 +1,4 @@
+comment = 'Test extension wildcard 1'
+default_version = '1.0'
+relocatable = true
+wildcard_upgrades = true
--
2.34.1
On 31 May 2023, at 21:07, Sandro Santilli <strk@kbt.io> 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?
--
Daniel Gustafsson
On 28 Jun 2023, at 10:29, Daniel Gustafsson <daniel@yesql.se> wrote:
On 31 May 2023, at 21:07, Sandro Santilli <strk@kbt.io> 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.
--
Daniel Gustafsson
On Tue, Aug 1, 2023 at 2:24 PM Daniel Gustafsson <daniel@yesql.se> wrote:
returned with feedback. Please feel free to resubmit to a future CF when there
is a new version of the patch.
Isn't the real problem here that there's no consensus on what to do?
Or to put a finer point on it, that Tom seems adamantly opposed to any
design that would solve the problem the patch authors are worried
about?
--
Robert Haas
EDB: http://www.enterprisedb.com
On 1 Aug 2023, at 20:45, Robert Haas <robertmhaas@gmail.com> wrote:
On Tue, Aug 1, 2023 at 2:24 PM Daniel Gustafsson <daniel@yesql.se> wrote:
returned with feedback. Please feel free to resubmit to a future CF when there
is a new version of the patch.Isn't the real problem here that there's no consensus on what to do?
I don't disagree with that, but there is nothing preventing that discussion to
continue here or on other threads. The fact that consensus is that far away
and no patch that applies exist seems to me to indicate that a new CF entry is
a better option than pushing forward.
--
Daniel Gustafsson
On 1 Aug 2023, at 20:45, Robert Haas <robertmhaas@gmail.com> wrote:
On Tue, Aug 1, 2023 at 2:24 PM Daniel Gustafsson <daniel@yesql.se>
wrote:
returned with feedback. Please feel free to resubmit to a future CF
when there is a new version of the patch.Isn't the real problem here that there's no consensus on what to do?
I don't disagree with that, but there is nothing preventing that discussion to
continue here or on other threads. The fact that consensus is that far away
and no patch that applies exist seems to me to indicate that a new CF entry is
a better option than pushing forward.--
Daniel Gustafsson
We are still interested. We'll clean up in the next week or so. Been tied up with PostGIS release cycle.
To Robert's point, we are losing faith in coming up with a solution that everyone agrees is workable.
What I thought Sandro had so far in his last patch seemed like a good compromise to me.
If we get it back to the point of passing the CF Bot, can we continue on this thread or are we just wasting our time?
Thanks,
Regina
On Tue, Aug 1, 2023 at 3:23 PM Daniel Gustafsson <daniel@yesql.se> wrote:
I don't disagree with that, but there is nothing preventing that discussion to
continue here or on other threads. The fact that consensus is that far away
and no patch that applies exist seems to me to indicate that a new CF entry is
a better option than pushing forward.
Fair point, thanks.
--
Robert Haas
EDB: http://www.enterprisedb.com
On Tue, Aug 01, 2023 at 08:24:15PM +0200, Daniel Gustafsson wrote:
On 28 Jun 2023, at 10:29, Daniel Gustafsson <daniel@yesql.se> wrote:
On 31 May 2023, at 21:07, Sandro Santilli <strk@kbt.io> 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;
Attachments:
v4-0001-Allow-wildcard-in-extension-upgrade-paths.patchtext/x-diff; charset=us-asciiDownload
From 9b8d1f45e5cd4b1e286d1c82d8ebca4fcc25eb92 Mon Sep 17 00:00:00 2001
From: Sandro Santilli <strk@kbt.io>
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 ...
</para>
</listitem>
</varlistentry>
+
+ <varlistentry id="extend-extensions-wildcard-upgrade">
+ <term><varname>wildcard_upgrades</varname> (<type>boolean</type>)</term>
+ <listitem>
+ <para>
+ This parameter, if set to <literal>true</literal> (which is not the
+ default), allows <command>ALTER EXTENSION</command> to consider
+ a wildcard character <literal>%</literal> as matching any version of
+ the extension. Such wildcard match will only be used when no
+ perfect match is found for a version.
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
<para>
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 = 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, 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
@@ -1281,13 +1300,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)));
}
/*
@@ -3491,3 +3520,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 1388c0fb0b..105138d08a 100644
--- a/src/test/modules/test_extensions/Makefile
+++ b/src/test/modules/test_extensions/Makefile
@@ -8,8 +8,8 @@ EXTENSION = test_ext1 test_ext2 test_ext3 test_ext4 test_ext5 test_ext6 \
test_ext_cyclic1 test_ext_cyclic2 \
test_ext_extschema \
test_ext_evttrig \
- test_ext_req_schema1 test_ext_req_schema2 test_ext_req_schema3
-
+ test_ext_req_schema1 test_ext_req_schema2 test_ext_req_schema3 \
+ test_ext_wildcard1
DATA = test_ext1--1.0.sql test_ext2--1.0.sql test_ext3--1.0.sql \
test_ext4--1.0.sql test_ext5--1.0.sql test_ext6--1.0.sql \
test_ext7--1.0.sql test_ext7--1.0--2.0.sql test_ext8--1.0.sql \
@@ -20,7 +20,8 @@ DATA = test_ext1--1.0.sql test_ext2--1.0.sql test_ext3--1.0.sql \
test_ext_evttrig--1.0.sql test_ext_evttrig--1.0--2.0.sql \
test_ext_req_schema1--1.0.sql \
test_ext_req_schema2--1.0.sql \
- test_ext_req_schema3--1.0.sql
+ test_ext_req_schema3--1.0.sql \
+ test_ext_wildcard1--1.0.sql test_ext_wildcard1--%--2.0.sql
REGRESS = test_extensions test_extdepend
diff --git a/src/test/modules/test_extensions/expected/test_extensions.out b/src/test/modules/test_extensions/expected/test_extensions.out
index 472627a232..270840183d 100644
--- a/src/test/modules/test_extensions/expected/test_extensions.out
+++ b/src/test/modules/test_extensions/expected/test_extensions.out
@@ -445,3 +445,21 @@ SELECT test_s_dep.dep_req2();
DROP EXTENSION test_ext_req_schema1 CASCADE;
NOTICE: drop cascades to extension test_ext_req_schema2
+--
+-- Test wildcard based upgrade paths
+--
+CREATE EXTENSION test_ext_wildcard1;
+SELECT ext_wildcard1_version();
+ ext_wildcard1_version
+-----------------------
+ 1.0
+(1 row)
+
+ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0';
+SELECT ext_wildcard1_version();
+ ext_wildcard1_version
+-----------------------
+ 2.0
+(1 row)
+
+DROP EXTENSION test_ext_wildcard1;
diff --git a/src/test/modules/test_extensions/sql/test_extensions.sql b/src/test/modules/test_extensions/sql/test_extensions.sql
index 51327cc321..bb567e0f19 100644
--- a/src/test/modules/test_extensions/sql/test_extensions.sql
+++ b/src/test/modules/test_extensions/sql/test_extensions.sql
@@ -276,3 +276,12 @@ ALTER EXTENSION test_ext_req_schema1 SET SCHEMA test_s_dep2; -- now ok
SELECT test_s_dep2.dep_req1();
SELECT test_s_dep.dep_req2();
DROP EXTENSION test_ext_req_schema1 CASCADE;
+
+--
+-- Test wildcard based upgrade paths
+--
+CREATE EXTENSION test_ext_wildcard1;
+SELECT ext_wildcard1_version();
+ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0';
+SELECT ext_wildcard1_version();
+DROP EXTENSION test_ext_wildcard1;
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql b/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
new file mode 100644
index 0000000000..75154e5c55
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
@@ -0,0 +1,6 @@
+/* src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql */
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0'" to load this file. \quit
+
+CREATE OR REPLACE FUNCTION ext_wildcard1_version() returns TEXT
+AS 'SELECT 2.0' LANGUAGE 'sql';
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql b/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
new file mode 100644
index 0000000000..a69e791fda
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
@@ -0,0 +1,6 @@
+/* src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql */
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "CREATE EXTENSION test_ext_wildcard1" to load this file. \quit
+
+CREATE FUNCTION ext_wildcard1_version() returns TEXT
+AS 'SELECT 1.0' LANGUAGE 'sql';
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1.control b/src/test/modules/test_extensions/test_ext_wildcard1.control
new file mode 100644
index 0000000000..865e37fa88
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1.control
@@ -0,0 +1,4 @@
+comment = 'Test extension wildcard 1'
+default_version = '1.0'
+relocatable = true
+wildcard_upgrades = true
--
2.34.1
On Mon, 7 Aug 2023 at 19:25, Sandro Santilli <strk@kbt.io> wrote:
On Tue, Aug 01, 2023 at 08:24:15PM +0200, Daniel Gustafsson wrote:
On 28 Jun 2023, at 10:29, Daniel Gustafsson <daniel@yesql.se> wrote:
On 31 May 2023, at 21:07, Sandro Santilli <strk@kbt.io> 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.
One of tests has failed in CFBot at [1]https://cirrus-ci.com/task/6090742435676160 with:
+++ /tmp/cirrus-ci-build/build/testrun/test_extensions/regress/results/test_extensions.out
2023-12-21 02:24:07.738726000 +0000
@@ -449,17 +449,20 @@
-- Test wildcard based upgrade paths
--
CREATE EXTENSION test_ext_wildcard1;
+ERROR: extension "test_ext_wildcard1" is not available
+DETAIL: Could not open extension control file
"/tmp/cirrus-ci-build/build/tmp_install/usr/local/pgsql/share/extension/test_ext_wildcard1.control":
No such file or directory.
+HINT: The extension must first be installed on the system where
PostgreSQL is running.
SELECT ext_wildcard1_version();
- ext_wildcard1_version
------------------------
- 1.0
-(1 row)
-
+ERROR: function ext_wildcard1_version() does not exist
+LINE 1: SELECT ext_wildcard1_version();
More details available at [2]https://api.cirrus-ci.com/v1/artifact/task/6090742435676160/testrun/build/testrun/test_extensions/regress/regression.diffs.
[1]: https://cirrus-ci.com/task/6090742435676160
[2]: https://api.cirrus-ci.com/v1/artifact/task/6090742435676160/testrun/build/testrun/test_extensions/regress/regression.diffs
Regards,
Vignesh
On Sun, 7 Jan 2024 at 12:36, vignesh C <vignesh21@gmail.com> wrote:
On Mon, 7 Aug 2023 at 19:25, Sandro Santilli <strk@kbt.io> wrote:
On Tue, Aug 01, 2023 at 08:24:15PM +0200, Daniel Gustafsson wrote:
On 28 Jun 2023, at 10:29, Daniel Gustafsson <daniel@yesql.se> wrote:
On 31 May 2023, at 21:07, Sandro Santilli <strk@kbt.io> 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.One of tests has failed in CFBot at [1] with:
+++ /tmp/cirrus-ci-build/build/testrun/test_extensions/regress/results/test_extensions.out 2023-12-21 02:24:07.738726000 +0000 @@ -449,17 +449,20 @@ -- Test wildcard based upgrade paths -- CREATE EXTENSION test_ext_wildcard1; +ERROR: extension "test_ext_wildcard1" is not available +DETAIL: Could not open extension control file "/tmp/cirrus-ci-build/build/tmp_install/usr/local/pgsql/share/extension/test_ext_wildcard1.control": No such file or directory. +HINT: The extension must first be installed on the system where PostgreSQL is running. SELECT ext_wildcard1_version(); - ext_wildcard1_version ------------------------ - 1.0 -(1 row) - +ERROR: function ext_wildcard1_version() does not exist +LINE 1: SELECT ext_wildcard1_version();
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.
Regards,
Vignesh
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;