[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

[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.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) ?

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;

And now with the actual patch attached ... (sorry)

--strk;

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;

And now a version of the patch including documentation and regression tests.
Anything you see I should improve ?

--strk;

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 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

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 and

Fwiw I believe wildcard_upgrades isn't necessary in the .control file.
If there are no % files, none would be used anyway, and if there are, it's

clear

it's meant as wildcard since % won't appear in any remotely sane version
number.

I also like the idea of skipping the wildcard_upgrades syntax.
Then there is no need to have a conditional control file for PG 16 vs. older
versions.

Here we go. Attached a version of the patch with no
"wildcard_upgrades" controlling it.

--strk;

On Sun, Nov 13, 2022 at 11:46:50PM -0500, Regina Obe wrote:

Re: Sandro Santilli

I'm attaching an updated version of the patch. This time the patch
is tested. Nothing changes unless the .control file for the
subject extension doesn't have a "wildcard_upgrades = true"

statement.

When wildcard upgrades are enabled, a file with a "%" symbol as
the "source" part of the upgrade path will match any version and

Fwiw I believe wildcard_upgrades isn't necessary in the .control file.
If there are no % files, none would be used anyway, and if there
are, it's

clear

it's meant as wildcard since % won't appear in any remotely sane
version number.

I also like the idea of skipping the wildcard_upgrades syntax.
Then there is no need to have a conditional control file for PG 16 vs.
older versions.

Here we go. Attached a version of the patch with no "wildcard_upgrades"
controlling it.

--strk;

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