Thread: [PATCH] Support % wildcard in extension upgrade filenames

[PATCH] Support % wildcard in extension upgrade filenames

From
Sandro Santilli
Date:
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

Attachment

RE: [PATCH] Support % wildcard in extension upgrade filenames

From
"Regina Obe"
Date:
> 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







Re: [PATCH] Support % wildcard in extension upgrade filenames

From
Laurenz Albe
Date:
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



Re: [PATCH] Support % wildcard in extension upgrade filenames

From
Daniel Gustafsson
Date:
> 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/




Re: [PATCH] Support % wildcard in extension upgrade filenames

From
Tom Lane
Date:
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



Re: [PATCH] Support % wildcard in extension upgrade filenames

From
Sandro Santilli
Date:
On Sat, May 28, 2022 at 04:50:20PM +0200, Laurenz Albe wrote:
> On Fri, 2022-05-27 at 17:37 -0400, Regina Obe wrote:
> >
> > https://lists.osgeo.org/pipermail/postgis-devel/2022-February/029500.html
> > 
> > Does anyone think this is such a horrible idea that we should abandon all
> > hope?
> 
> I don't think this idea is fundamentally wrong, but I have two worries:
> 
> 1. It would be a good idea good to make sure that there is not both
>    "extension--%--2.0.sql" and "extension--1.0--2.0.sql" present.
>    Otherwise the behavior might be indeterministic.

I'd make sure to use extension--1.0--2.0.sql in that case (more
specific first).

> 2. What if you have a "postgis--%--3.3.sql", and somebody tries to upgrade
>    their PostGIS 1.1 installation with it?  Would that work?

For PostGIS in particular it will NOT work as the PostGIS upgrade
script checks for the older version and decides if the upgrade is
valid or not. This is the same upgrade code used for non-extension
installs.

>    Having a lower bound for a matching version might be a good idea,
>    although I have no idea how to do that.

I was thinking of a broader pattern matching support, like:

  postgis--3.%--3.3.sql

But it would be better to start simple and eventually if needed
increase the complexity ?

Another option could be specifying something in the control file,
which would also probably be a good idea to still allow some
extensions to use '%' in the version string (for example).

--strk; 

  Libre GIS consultant/developer
  https://strk.kbt.io/services.html



Re: [PATCH] Support % wildcard in extension upgrade filenames

From
Sandro Santilli
Date:
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;



Re: [PATCH] Support % wildcard in extension upgrade filenames

From
Sandro Santilli
Date:
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;



Re: [PATCH] Support % wildcard in extension upgrade filenames

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

When wildcard upgrades are enabled, a file with a "%" symbol as
the "source" part of the upgrade path will match any version and
will be used if a specific version upgrade does not exist.
This means that in presence of the following files:

    postgis--3.0.0--3.2.0.sql
    postgis--%--3.2.0.sql

The first one will be used for going from 3.0.0 to 3.2.0.

This is the intention. The patch lacks automated tests and can
probably be improved.

For more context, a previous (non-working) version of this patch was
submitted to commitfest: https://commitfest.postgresql.org/38/3654/

--strk;

On Sat, Jun 04, 2022 at 11:20:55AM +0200, Sandro Santilli wrote:
> On Sat, May 28, 2022 at 04:50:20PM +0200, Laurenz Albe wrote:
> > On Fri, 2022-05-27 at 17:37 -0400, Regina Obe wrote:
> > >
> > > https://lists.osgeo.org/pipermail/postgis-devel/2022-February/029500.html
> > > 
> > > Does anyone think this is such a horrible idea that we should abandon all
> > > hope?
> > 
> > I don't think this idea is fundamentally wrong, but I have two worries:
> > 
> > 1. It would be a good idea good to make sure that there is not both
> >    "extension--%--2.0.sql" and "extension--1.0--2.0.sql" present.
> >    Otherwise the behavior might be indeterministic.
> 
> I'd make sure to use extension--1.0--2.0.sql in that case (more
> specific first).
> 
> > 2. What if you have a "postgis--%--3.3.sql", and somebody tries to upgrade
> >    their PostGIS 1.1 installation with it?  Would that work?
> 
> For PostGIS in particular it will NOT work as the PostGIS upgrade
> script checks for the older version and decides if the upgrade is
> valid or not. This is the same upgrade code used for non-extension
> installs.
> 
> >    Having a lower bound for a matching version might be a good idea,
> >    although I have no idea how to do that.
> 
> I was thinking of a broader pattern matching support, like:
> 
>   postgis--3.%--3.3.sql
> 
> But it would be better to start simple and eventually if needed
> increase the complexity ?
> 
> Another option could be specifying something in the control file,
> which would also probably be a good idea to still allow some
> extensions to use '%' in the version string (for example).
> 
> --strk; 
> 
>   Libre GIS consultant/developer
>   https://strk.kbt.io/services.html
> 
> 



Re: [PATCH] Support % wildcard in extension upgrade filenames

From
Sandro Santilli
Date:
And now with the actual patch attached ... (sorry)

--strk;

On Thu, Sep 15, 2022 at 12:01:04AM +0200, Sandro Santilli wrote:
> I'm attaching an updated version of the patch. This time the patch
> is tested. Nothing changes unless the .control file for the subject
> extension doesn't have a "wildcard_upgrades = true" statement.
> 
> When wildcard upgrades are enabled, a file with a "%" symbol as
> the "source" part of the upgrade path will match any version and
> will be used if a specific version upgrade does not exist.
> This means that in presence of the following files:
> 
>     postgis--3.0.0--3.2.0.sql
>     postgis--%--3.2.0.sql
> 
> The first one will be used for going from 3.0.0 to 3.2.0.
> 
> This is the intention. The patch lacks automated tests and can
> probably be improved.
> 
> For more context, a previous (non-working) version of this patch was
> submitted to commitfest: https://commitfest.postgresql.org/38/3654/
> 
> --strk;
> 
> On Sat, Jun 04, 2022 at 11:20:55AM +0200, Sandro Santilli wrote:
> > On Sat, May 28, 2022 at 04:50:20PM +0200, Laurenz Albe wrote:
> > > On Fri, 2022-05-27 at 17:37 -0400, Regina Obe wrote:
> > > >
> > > > https://lists.osgeo.org/pipermail/postgis-devel/2022-February/029500.html
> > > > 
> > > > Does anyone think this is such a horrible idea that we should abandon all
> > > > hope?
> > > 
> > > I don't think this idea is fundamentally wrong, but I have two worries:
> > > 
> > > 1. It would be a good idea good to make sure that there is not both
> > >    "extension--%--2.0.sql" and "extension--1.0--2.0.sql" present.
> > >    Otherwise the behavior might be indeterministic.
> > 
> > I'd make sure to use extension--1.0--2.0.sql in that case (more
> > specific first).
> > 
> > > 2. What if you have a "postgis--%--3.3.sql", and somebody tries to upgrade
> > >    their PostGIS 1.1 installation with it?  Would that work?
> > 
> > For PostGIS in particular it will NOT work as the PostGIS upgrade
> > script checks for the older version and decides if the upgrade is
> > valid or not. This is the same upgrade code used for non-extension
> > installs.
> > 
> > >    Having a lower bound for a matching version might be a good idea,
> > >    although I have no idea how to do that.
> > 
> > I was thinking of a broader pattern matching support, like:
> > 
> >   postgis--3.%--3.3.sql
> > 
> > But it would be better to start simple and eventually if needed
> > increase the complexity ?
> > 
> > Another option could be specifying something in the control file,
> > which would also probably be a good idea to still allow some
> > extensions to use '%' in the version string (for example).
> > 
> > --strk;

Attachment

RE: [PATCH] Support % wildcard in extension upgrade filenames

From
"Regina Obe"
Date:
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








Re: [PATCH] Support % wildcard in extension upgrade filenames

From
Sandro Santilli
Date:
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;

Attachment

Re: [PATCH] Support % wildcard in extension upgrade filenames

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

--strk;

On Fri, Nov 04, 2022 at 06:31:58PM +0100, Sandro Santilli wrote:
> On Mon, Oct 31, 2022 at 01:55:05AM -0400, Regina Obe wrote:
> > 
> > Sandro, can you submit an updated version of this patch.
> > I was testing it out and looked good first time.
> 
> Attached updated version of the patch.
> 
> > If everyone is okay with this patch, we'll go ahead and add tests and
> > documentation to go with it.
> 
> Yes please let us know if there's any chance of getting this
> mechanism approved or if we should keep advertising works around
> this limitation (it's not just installing 100s of files but also
> still missing needed upgrade paths when doing so).
> 
> 
> --strk;

Attachment

Re: [PATCH] Support % wildcard in extension upgrade filenames

From
LEO HSU
Date:
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
filesby specifying less granular stopping.
 
One of the projects that will benefit from this is the PostGIS project.  Currently we ship over 300 extension scripts
perversion 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
upgradethat 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
properlyupgrade.
 

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
requestedversion, 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
newversion.
 
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
filesit 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

Re: [PATCH] Support % wildcard in extension upgrade filenames

From
Regina Obe
Date:
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: [PATCH] Support % wildcard in extension upgrade filenames

From
Christoph Berg
Date:
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: [PATCH] Support % wildcard in extension upgrade filenames

From
Christoph Berg
Date:
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: [PATCH] Support % wildcard in extension upgrade filenames

From
"Regina Obe"
Date:
> 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






Re: [PATCH] Support % wildcard in extension upgrade filenames

From
'Sandro Santilli'
Date:
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;

Attachment

RE: [PATCH] Support % wildcard in extension upgrade filenames

From
"Regina Obe"
Date:
> 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




Re: [PATCH] Support % wildcard in extension upgrade filenames

From
Justin Pryzby
Date:
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



Re: [PATCH] Support % wildcard in extension upgrade filenames

From
Sandro Santilli
Date:
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;

Attachment

Re: [PATCH] Support % wildcard in extension upgrade filenames

From
Regina Obe
Date:
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 https://www.postgresql.org/message-id/20221117095734.igldlk6kngr6ogim%40c19

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'tpersonally 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
tothe specified new version. The update script is run in the same environment that CREATE EXTENSION provides for
installationscripts: in particular, search_path is set up in the same way, and any new objects created by the script
areautomatically added to the extension. Also, if the script chooses to drop extension member objects, they are
automaticallydissociated 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
localsetup 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
patchhe had submitted before. 

Re: [PATCH] Support % wildcard in extension upgrade filenames

From
Tom Lane
Date:
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



RE: [PATCH] Support % wildcard in extension upgrade filenames

From
"Regina Obe"
Date:
> 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




Re: [PATCH] Support % wildcard in extension upgrade filenames

From
Sandro Santilli
Date:
On Mon, Jan 09, 2023 at 05:51:49PM -0500, Tom Lane wrote:

> Have you considered the idea of instead inventing a "\include" facility

[...]

> cases, you still need one script file for each supported upgrade step

That's exactly the problem we're trying to solve here.
The include support is nice on itself, but won't solve our problem.

--strk;



Re: [PATCH] Support % wildcard in extension upgrade filenames

From
Tom Lane
Date:
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



RE: [PATCH] Support % wildcard in extension upgrade filenames

From
"Regina Obe"
Date:
> 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




Re: [PATCH] Support % wildcard in extension upgrade filenames

From
Sandro Santilli
Date:
On Tue, Jan 10, 2023 at 06:50:31PM -0500, Tom Lane wrote:


> With the proposed % feature, if foo--%--3.0.sql exists then the
> system will invoke it and expect the end result to be a valid
> 3.0 installation, whether or not the script actually has any
> ability to do a downgrade. 

It is sane, for the system to expect the end result
to be a valid 3.0 installation if no exception is
thrown by the script itself. 

If we ship foo--%--3.0.sql we must have been taken care of
protecting from unsupported downgrades/upgrades (and we did,
having the script throw an exception if anything is unexpected).

> (I suppose you could add code to look at pg_extension.extversion,

We actually added code looking at our own version-extracting
function (which existed since before PostgreSQL added support
for extensions). Is the function not there ? Raise an exception.
Is the function not owned by the extension ? Raise an exception.
In other cases -> trust the output of that function to tell what
version we're coming from, throw an exception if upgrade to the
target version is unsupported.

> to add such code is that really better than having a number of
> one-liner scripts implementing the same check declaratively?)

Yes, it is, because no matter how many one-liner scripts we install
(we currently install 87 * 6 such scripts, we always end up missing
some of them upon releasing a new bug-fix release in stable branches.

> almost certainly you are
> going to end with an extension containing some leftover 4.0
> objects, some 3.0 objects, and maybe some objects with properties
> that don't exactly match either 3.0 or 4.0. 

This is already possible, depending on WHO writes those upgrade
scripts. This proposal just gives more expressiveness power to
those script authors.

> So I really think this is a case of "be careful what you ask
> for, you might get it".  Even if PostGIS is willing to put in
> the amount of infrastructure legwork needed to make such a
> design bulletproof, I'm quite sure nobody else will manage
> to use such a thing successfully.

This is why I initially made this something to be explicitly enabled
by the .control file, which I can do again if it feels safer for you.

> I'd rather spend our
> development effort on a feature that has more than one use-case.

Use case is any extension willing to support more than a single stable
branch while still allowing upgrading from newer-stable-bugfix-release
to older-feature-release (ie: 3.2.10 -> 3.4.0 ). Does not seem so
uncommon to me, for a big project. Maybe there aren't enough big
extension-based projects out there ?

--strk; 

  Libre GIS consultant/developer
  https://strk.kbt.io/services.html



Re: [PATCH] Support % wildcard in extension upgrade filenames

From
'Sandro Santilli'
Date:
On Tue, Jan 10, 2023 at 11:09:23PM -0500, Regina Obe wrote:

> The only way we can fix that in the current setup, is to move to a minor
> version mode which means we can 
> never do micro updates.

Or just not with standard PostgreSQL syntax, because we can of course
do upgrades using the `SELECT postgis_extensions_upgrade()` call at
the moment, which, if you ask me, sounds MORE dangerous than the
wildcard upgrade approach because the _implementation_ of that
function will always be the OLD implementation, never the NEW one,
so if bogus cannot be fixed by a new release w/out a way to upgrade
there...

--strk;



Re: [PATCH] Support % wildcard in extension upgrade filenames

From
"Gregory Stark (as CFM)"
Date:
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



RE: [PATCH] Support % wildcard in extension upgrade filenames

From
"Regina Obe"
Date:
> 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
togo and we'll rebase our other patch after that. 

Thanks,
Regina




Re: [PATCH] Support % wildcard in extension upgrade filenames

From
Sandro Santilli
Date:
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;

Attachment

Re: [PATCH] Support % wildcard in extension upgrade filenames

From
Robert Haas
Date:
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



Re: [PATCH] Support % wildcard in extension upgrade filenames

From
Tom Lane
Date:
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



RE: [PATCH] Support % wildcard in extension upgrade filenames

From
"Regina Obe"
Date:
> 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).





RE: [PATCH] Support % wildcard in extension upgrade filenames

From
"Regina Obe"
Date:
> 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.

https://www.postgresql.org/message-id/000201d92572%247dcd8260%2479688720%24%40pcorp.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
livewithout that if we can stop shipping 300 empty files. 

Thanks,
Regina




Re: [PATCH] Support % wildcard in extension upgrade filenames

From
Sandro Santilli
Date:
On Tue, Mar 07, 2023 at 12:39:34PM -0500, Robert Haas wrote:

> The thing that confuses me here is why the PostGIS folks are ending up
> with so many files.

We want to allow PostGIS users to upgrade from ANY previous version,
because different distribution or user habit may result in people
upgrading from ANY older release to the newest.

Now, PostGIS has released a total of 164 versions:

    curl -s https://download.osgeo.org/postgis/source/ |
      grep -c '.tar.gz<'

In *addition* to these, most developers end up installing a "dev"
suffixed version to distinguish it from the official releases,
which means there's a total of 164*2 = 328 version *per extension*.

Now, PostGIS doesn't install a single extension but multiple ones:

  - postgis
  - postgis_raster
  - postgis_topology
  - postgis_sfcgal
  - postgis_tiger_geocoder
  - address_standardizer

So we'd be up to 328 * 6 = 1968 files needed to upgrade from ANY
postgis extension version to CURRENT version.

The numbers are slightly less because not all versions of PostGIS
did support EXTENSION (wasn't even available in early PostGIS versions)

> We certainly don't have that problem with the
> extension that are being maintained in contrib, and I guess there is
> some difference in versioning practice that is making it an issue for
> them but not for us. I wish I understood what was going on there.

Our extension versioning has 3 numbers, but PostgreSQL doesn't really
care about this right ? It's just that we've had 328 different
versions released and more are to come, and we want users to be able
to upgrade from each one of them.

I guess you may suggest that we do NOT increas the *EXTENSION* version
number UNLESS something really changes at SQL level, but honestly I
doubt we EVER released a version with no changes at the SQL level
(maybe in some occasion for really bad bugs which were only fixed at
the C level).

--strk;



Re: [PATCH] Support % wildcard in extension upgrade filenames

From
Sandro Santilli
Date:
On Tue, Mar 07, 2023 at 02:13:07PM -0500, Tom Lane wrote:

> What I am maintaining is that no extension author is actually going
> to write such a script, indeed they probably won't trouble to write
> any downgrade-like actions at all.  Which makes the proposed design
> mostly a foot-gun.

What I'm maintaining is that such authors should be warned about
the risk, and discouraged from installing any wildcard-containing
script UNLESS they deal with downgrade protection.

PostGIS does deal with that kind of protection (yes, could be helped
somehow in doing that by PostgreSQL).

> I'm not unsympathetic to the idea of trying to support multiple upgrade
> paths in one script.  I just don't like this particular design for that,
> because it requires the extension author to make promises that nobody
> is actually going to deliver on.

Would you be ok with a stricter pattern matching ? Something like:

  postgis--3.3.%--3.3.ANY.sql
  postgis--3.3.ANY--3.4.0.sql

Would that be easier to promise something about ?

--strk;



Re: [PATCH] Support % wildcard in extension upgrade filenames

From
Robert Haas
Date:
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



RE: [PATCH] Support % wildcard in extension upgrade filenames

From
"Regina Obe"
Date:
> 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
PostgreSQL16+.  

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
pathsor should we just assume if there is a map file, then that takes precedence over any paths inferred by files in
thesystem. 

Thanks,
Regina




Re: [PATCH] Support % wildcard in extension upgrade filenames

From
Sandro Santilli
Date:
On Wed, Mar 08, 2023 at 08:27:29AM -0500, Robert Haas wrote:

> I wonder if a solution to this problem might be to provide some kind
> of a version map file. Let's suppose that the map file is a bunch of
> lines of the form X Y Z, where X, Y, and Z are version numbers. The
> semantics could be: we (the extension authors) promise that if you
> want to upgrade from X to Z, it suffices to run the script that knows
> how to upgrade from Y to Z.
> This would address Tom's concern, because
> if you write a master upgrade script, you have to explicitly declare
> the versions to which it applies.

This would just move the problem from having 1968 files to having
to write 1968 lines in control files, and does not solve the problem
of allowing people with an hot-patched old version not being able to
upgrade to a newer (bug-free) version released before the hot-patch.

We maintain multiple stable branches (5, at the moment: 2.5, 3.0, 3.1,
3.2, 3.3) and would like to allow anyone running an old patched
version to still upgrade to a newer version released earlier.

--strk;

  Libre GIS consultant/developer
  https://strk.kbt.io/services.html



Re: [PATCH] Support % wildcard in extension upgrade filenames

From
'Sandro Santilli'
Date:
On Wed, Mar 08, 2023 at 03:18:06PM -0500, Regina Obe wrote:
> 
> Then question arises if you have such a map file and you have files as well (the old way).

One idea I had in the past about the .control file was to
advertise an executable that would take current version and next
version and return a list of paths to SQL files to use to upgrade.

Then our script could decide what to do, w/out Tom looking :P

--strk;



RE: [PATCH] Support % wildcard in extension upgrade filenames

From
"Regina Obe"
Date:
> > 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







Re: [PATCH] Support % wildcard in extension upgrade filenames

From
Sandro Santilli
Date:
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;

Attachment

Re: [PATCH] Support % wildcard in extension upgrade filenames

From
Yurii Rashkovskii
Date:
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.

Re: [PATCH] Support % wildcard in extension upgrade filenames

From
Sandro Santilli
Date:
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;

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



RE: [PATCH] Support % wildcard in extension upgrade filenames

From
"Regina Obe"
Date:
> 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;
>

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
followthe 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
scriptthat works for all cases. 

Thanks,
Regina











Re: [PATCH] Support % wildcard in extension upgrade filenames

From
Sandro Santilli
Date:
On Mon, Apr 10, 2023 at 11:09:40PM -0400, Regina Obe wrote:
> > On Mon, Apr 03, 2023 at 09:26:25AM +0700, Yurii Rashkovskii wrote:
> > > I want to chime in on the issue of lower-number releases that are
> > > released after higher-number releases. The way I see this particular
> > > problem is that we always put upgrade SQL files in release "packages,"
> > > and they obviously become static resources.
> > >
> > > While I [intentionally] overlook some details here, what if (as a
> > > convention, for projects where it matters) we shipped extensions with
> > > non-upgrade SQL files only, and upgrades were available as separate
> > > downloads? This way, we're not tying releases themselves to upgrade paths.
> > > This also requires no changes to Postgres.
> > 
> > This is actually something that's on the plate, and we recently added a --
> > disable-extension-upgrades-install configure switch and a `install-extension-
> > upgrades-from-known-versions` make target in PostGIS to help going in that
> > direction. I guess the ball would now be in the hands of packagers.
> > 
> > > I know this may be a big delivery layout departure for
> > > well-established projects; I also understand that this won't solve the
> > > problem of having to have these files in the first place (though in
> > > many cases, they can be automatically generated once, I suppose, if they are
> > trivial).
> > 
> > We will now also be providing a `postgis` script for administration that among
> > other things will support a `install-extension-upgrades` command to install
> > upgrade paths from specific old versions, but will not hard-code any list of
> > "known" old versions as such a list will easily become outdated.
> > 
> > Note that all upgrade scripts installed by the Makefile target or by the
> > `postgis` scripts will only be empty upgrade paths from the source version to
> > the fake "ANY" version, as the ANY--<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;



RE: [PATCH] Support % wildcard in extension upgrade filenames

From
"Regina Obe"
Date:
> 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
> > 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;

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.






Re: [PATCH] Support % wildcard in extension upgrade filenames

From
Sandro Santilli
Date:
On Tue, Apr 11, 2023 at 04:36:04PM -0400, Regina Obe wrote:
> 
> > Hey, best would be having support for wildcard wouldn't it ?
> 
> I'm a woman of compromise. Sure 1 file would be ideal, but 
> I'd rather live with a big file listing all version upgrades
> than 1000 files with the same information.

Personally I don't see the benefit of 1 big file vs. many 0-length
files to justify the cost (time and complexity) of a PostgreSQL change,
with the corresponding cost of making use of this new functionality
based on PostgreSQL version.

We'd still have the problem of missing upgrade paths unless we release
a new version of PostGIS even if it's ONLY for the sake of updating
that 1 big file (or adding a new file, in the current situation).

--strk;



RE: [PATCH] Support % wildcard in extension upgrade filenames

From
"Regina Obe"
Date:
> 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.






RE: [PATCH] Support % wildcard in extension upgrade filenames

From
"Regina Obe"
Date:
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?









RE: [PATCH] Support % wildcard in extension upgrade filenames

From
"Regina Obe"
Date:
> 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?
> 

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




Re: [PATCH] Support % wildcard in extension upgrade filenames

From
Mat Arye
Date:
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

RE: [PATCH] Support % wildcard in extension upgrade filenames

From
"Regina Obe"
Date:
> This change also makes it easier for extensions that use versioned .so files (by that I mean uses
extension-<version>.sorather than extension.so).  
> Because such extensions can't really use the chaining property of the existing upgrade system and so need to write a
directX--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.
Especiallythe 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
onall 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
theminor. 
But the idea is someone can upgrade to stable version, so the script needs to always have the .so version in it to
accountfor this shifting of options. 

Thanks,
Regina




Re: [PATCH] Support % wildcard in extension upgrade filenames

From
Sandro Santilli
Date:
On Mon, Apr 24, 2023 at 01:06:24PM -0400, Mat Arye wrote:
> Hi All,
> 
> I've done upgrade maintenance for multiple extensions now (TimescaleDB
> core, Promscale) and I think the original suggestion (wildcard filenames
> with control-file switch to enable) here is a good one.

Thanks for your comment, Mat.

I'm happy to bring back the control-file switch if there's an
agreement about that.

The rationale for me to add that was solely to be 100% sure about not
breaking any extension using "%" character as an actual version.

I never considered it a security measure because the author of the control
file is the same as the author of the ugprade scripts so shipping a
wildcard upgrade script could be seen as a switch itself.

[...]

> As for Tom's concern about downgrades, I think it's valid but it's a case
> that is easy to test for in Plpgsql and either handle or error. For
> example, we use semver so testing for a downgrade at the top of the upgrade
> script is trivial.

I'd say it could be made even easier if PostgreSQL itself would provide
a variable (or other way to fetch it) for the "source" version of the extension
during exection of the "source"--"target" upgrade.

I'm saying this because PostGIS fetches this version by calling a
PostGIS function, but some extensions might have such "version"
function pointing to a C file that doesn't exist enymore at the time
of upgrade and thus would be left with the impossibility to rely on
calling it.

--strk;



RE: [PATCH] Support % wildcard in extension upgrade filenames

From
"Regina Obe"
Date:
> 
> > 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




Re: [PATCH] Support % wildcard in extension upgrade filenames

From
Eric Ridge
Date:
(I'm the developer of ZomboDB and pgrx, which while not an extension per se, allows others to make extensions that then
needupgrade 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
fromX to Y is great.  In theory that's not much different than having a bunch of individual files, but in practice an
extensionlikely could pare their actual file set down to just a few, and dealing with less files is a big win.  And
thenthe 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.
Rustand 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'mnot 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
tothe Rust rewrite, these were all hand-generated, and sometimes were just zero bytes.  I've since developed tooling to
auto-generateupgrade scripts by diffing the full schemas for OLDVER and NEWVER and emitting whatever DDL can move
OLDVERto 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.sqlhas a few, one of which is to optionally enable PostGIS support! haha  ZDB is fairly complex too.
Hundredsof functions, dozens of types, an IAM implementation, a dozen views, a few tables, some operators.  I also
don'tsee 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
EXTENSIONor 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
seenin this thread I really like the mapping file idea and I don't have any original thoughts on the subject. 

eric




Re: [PATCH] Support % wildcard in extension upgrade filenames

From
Robert Haas
Date:
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



Re: [PATCH] Support % wildcard in extension upgrade filenames

From
Eric Ridge
Date:
> 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
facteach release needs its own upgrade.sql script. I'm working on a point release right this moment and it won't have
anyschema 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
Postgresinternals 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
usto 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
cando what we want without regard to the project's release schedule.  In general, y'all nailed the extension system
backin 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
definitelywant to support anything that would make their lives easier.  But I also don't want to see something that
pgrxusers 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
theway of patches to offer.  I also have some radical ideas about annotating the sources themselves, but I don't feel
likelobbing a grenade today. 

eric


Re: [PATCH] Support % wildcard in extension upgrade filenames

From
Tom Lane
Date:
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
facteach release needs its own upgrade.sql script. I'm working on a point release right this moment and it won't have
anyschema 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



Re: [PATCH] Support % wildcard in extension upgrade filenames

From
Eric Ridge
Date:
> 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
facteach release needs its own upgrade.sql script. I'm working on a point release right this moment and it won't have
anyschema 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
versionkeeps 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
REPLACEfor every function on every extension version bump.  That forces an upgrade script even if the schema didn't
technicallychange 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


RE: [PATCH] Support % wildcard in extension upgrade filenames

From
"Regina Obe"
Date:
> 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.





Re: [PATCH] Support % wildcard in extension upgrade filenames

From
Yurii Rashkovskii
Date:


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]

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. 

--

Re: [PATCH] Support % wildcard in extension upgrade filenames

From
Przemysław Sztoch
Date:
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

Re: [PATCH] Support % wildcard in extension upgrade filenames

From
Sandro Santilli
Date:
On Thu, May 18, 2023 at 11:14:52PM +0200, Przemysław Sztoch wrote:
> For me, it would be a big help if you could specify a simple from/to range
> as the source version:
> myext--1.0.0-1.9.9--2.0.0.sql
> myext--2.0.0-2.9.9--3.0.0.sql
> myext--3.0.0-3.2.3--3.2.4.sql
> 
> The idea of % wildcard in my opinion is fully contained in the from/to
> range.

Yes, it will be once partial matching is implemented (in its current
state the patch only allows the wildcard to cover the whole version,
not just a portion, but the idea was to move to partial matches too).

> The name "ANY" should also be allowed as the source version.

It is. The only character with a special meaning, in my patch, would
be the "%" character, and would ONLY be special if the extension has
set the wildcard_upgrades directive to "true" in the .control file.

> Some extensions are a collection of CREATE OR REPLACE FUNCTION. All upgrades
> are one and the same SQL file.

Yes, this is the case for PostGIS, and that's why we're looking
forward to addint support for this wildcard.

--strk;



Re: [PATCH] Support % wildcard in extension upgrade filenames

From
Sandro Santilli
Date:
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;

Attachment

Re: [PATCH] Support % wildcard in extension upgrade filenames

From
Daniel Gustafsson
Date:
> 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




Re: [PATCH] Support % wildcard in extension upgrade filenames

From
Daniel Gustafsson
Date:
> 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




Re: [PATCH] Support % wildcard in extension upgrade filenames

From
Robert Haas
Date:
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



Re: [PATCH] Support % wildcard in extension upgrade filenames

From
Daniel Gustafsson
Date:
> 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




RE: [PATCH] Support % wildcard in extension upgrade filenames

From
"Regina Obe"
Date:
> > 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




Re: [PATCH] Support % wildcard in extension upgrade filenames

From
Robert Haas
Date:
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



Re: [PATCH] Support % wildcard in extension upgrade filenames

From
Sandro Santilli
Date:
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;

Attachment

Re: [PATCH] Support % wildcard in extension upgrade filenames

From
vignesh C
Date:
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();

More details available at [2].

[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



Re: [PATCH] Support % wildcard in extension upgrade filenames

From
vignesh C
Date:
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



Re: [PATCH] Support % wildcard in extension upgrade filenames

From
Sandro Santilli
Date:
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;

Attachment