public inbox for [email protected]
help / color / mirror / Atom feedRE: [PATCH] Support % wildcard in extension upgrade filenames
21+ messages / 7 participants
[nested] [flat]
* RE: [PATCH] Support % wildcard in extension upgrade filenames
@ 2023-04-11 03:09 Regina Obe <[email protected]>
2023-04-11 18:48 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
0 siblings, 1 reply; 21+ messages in thread
From: Regina Obe @ 2023-04-11 03:09 UTC (permalink / raw)
To: [email protected]; 'Yurii Rashkovskii' <[email protected]>; +Cc: 'Tom Lane' <[email protected]>; 'Regina Obe' <[email protected]>; [email protected]
> 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 follow the pattern
major.minor.micro
In almost all cases the minor upgrade script works for all micros of that minor and in postgis yes we have a single script that works for all cases.
Thanks,
Regina
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: [PATCH] Support % wildcard in extension upgrade filenames
2023-04-11 03:09 RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
@ 2023-04-11 18:48 ` Sandro Santilli <[email protected]>
2023-04-11 20:36 ` RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
0 siblings, 1 reply; 21+ messages in thread
From: Sandro Santilli @ 2023-04-11 18:48 UTC (permalink / raw)
To: Regina Obe <[email protected]>; +Cc: 'Yurii Rashkovskii' <[email protected]>; 'Tom Lane' <[email protected]>; 'Regina Obe' <[email protected]>; [email protected]
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;
^ permalink raw reply [nested|flat] 21+ messages in thread
* RE: [PATCH] Support % wildcard in extension upgrade filenames
2023-04-11 03:09 RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-11 18:48 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
@ 2023-04-11 20:36 ` Regina Obe <[email protected]>
2023-04-11 21:27 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
0 siblings, 1 reply; 21+ messages in thread
From: Regina Obe @ 2023-04-11 20:36 UTC (permalink / raw)
To: [email protected]; +Cc: 'Yurii Rashkovskii' <[email protected]>; 'Tom Lane' <[email protected]>; 'Regina Obe' <[email protected]>; [email protected]
> 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.
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: [PATCH] Support % wildcard in extension upgrade filenames
2023-04-11 03:09 RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-11 18:48 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-04-11 20:36 ` RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
@ 2023-04-11 21:27 ` Sandro Santilli <[email protected]>
2023-04-11 22:19 ` RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-13 22:32 ` RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
0 siblings, 2 replies; 21+ messages in thread
From: Sandro Santilli @ 2023-04-11 21:27 UTC (permalink / raw)
To: Regina Obe <[email protected]>; +Cc: 'Yurii Rashkovskii' <[email protected]>; 'Tom Lane' <[email protected]>; 'Regina Obe' <[email protected]>; [email protected]
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;
^ permalink raw reply [nested|flat] 21+ messages in thread
* RE: [PATCH] Support % wildcard in extension upgrade filenames
2023-04-11 03:09 RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-11 18:48 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-04-11 20:36 ` RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-11 21:27 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
@ 2023-04-11 22:19 ` Regina Obe <[email protected]>
1 sibling, 0 replies; 21+ messages in thread
From: Regina Obe @ 2023-04-11 22:19 UTC (permalink / raw)
To: [email protected]; +Cc: 'Yurii Rashkovskii' <[email protected]>; 'Tom Lane' <[email protected]>; 'Regina Obe' <[email protected]>; [email protected]
> 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.
^ permalink raw reply [nested|flat] 21+ messages in thread
* RE: [PATCH] Support % wildcard in extension upgrade filenames
2023-04-11 03:09 RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-11 18:48 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-04-11 20:36 ` RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-11 21:27 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
@ 2023-04-13 22:32 ` Regina Obe <[email protected]>
2023-04-24 17:06 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Mat Arye <[email protected]>
1 sibling, 1 reply; 21+ messages in thread
From: Regina Obe @ 2023-04-13 22:32 UTC (permalink / raw)
To: [email protected]; +Cc: 'Yurii Rashkovskii' <[email protected]>; 'Tom Lane' <[email protected]>; 'Regina Obe' <[email protected]>; [email protected]
> 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
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: [PATCH] Support % wildcard in extension upgrade filenames
2023-04-11 03:09 RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-11 18:48 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-04-11 20:36 ` RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-11 21:27 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-04-13 22:32 ` RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
@ 2023-04-24 17:06 ` Mat Arye <[email protected]>
2023-04-24 18:00 ` RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-27 10:49 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
0 siblings, 2 replies; 21+ messages in thread
From: Mat Arye @ 2023-04-24 17:06 UTC (permalink / raw)
To: Regina Obe <[email protected]>; +Cc: [email protected]; Yurii Rashkovskii <[email protected]>; Tom Lane <[email protected]>; Regina Obe <[email protected]>; [email protected]
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
^ permalink raw reply [nested|flat] 21+ messages in thread
* RE: [PATCH] Support % wildcard in extension upgrade filenames
2023-04-11 03:09 RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-11 18:48 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-04-11 20:36 ` RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-11 21:27 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-04-13 22:32 ` RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-24 17:06 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Mat Arye <[email protected]>
@ 2023-04-24 18:00 ` Regina Obe <[email protected]>
1 sibling, 0 replies; 21+ messages in thread
From: Regina Obe @ 2023-04-24 18:00 UTC (permalink / raw)
To: 'Mat Arye' <[email protected]>; +Cc: [email protected]; 'Yurii Rashkovskii' <[email protected]>; 'Tom Lane' <[email protected]>; 'Regina Obe' <[email protected]>; [email protected]
> This change also makes it easier for extensions that use versioned .so files (by that I mean uses extension-<version>.so rather than extension.so).
> Because such extensions can't really use the chaining property of the existing upgrade system and so need to write a direct X--Y.sql migration file for
> every prior version X. I know the system wasn't designed for this, but in reality a lot of extensions do this. Especially the more complex ones.
> Hopefully this is helpful,
> Mat
Thanks for the feedback. Yes this idea of versioned .so is also one of the reasons why we always have to run a replace on all functions.
Like for example on PostGIS side, we have option of full minor (so the lib could be postgis-3.3.so or postgis-3.so)
For development I always include the minor version and the windows interim builds I build against our master branch has the minor.
But the idea is someone can upgrade to stable version, so the script needs to always have the .so version in it to account for this shifting of options.
Thanks,
Regina
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: [PATCH] Support % wildcard in extension upgrade filenames
2023-04-11 03:09 RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-11 18:48 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-04-11 20:36 ` RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-11 21:27 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-04-13 22:32 ` RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-24 17:06 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Mat Arye <[email protected]>
@ 2023-04-27 10:49 ` Sandro Santilli <[email protected]>
2023-05-31 19:07 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
1 sibling, 1 reply; 21+ messages in thread
From: Sandro Santilli @ 2023-04-27 10:49 UTC (permalink / raw)
To: Mat Arye <[email protected]>; +Cc: Regina Obe <[email protected]>; Yurii Rashkovskii <[email protected]>; Tom Lane <[email protected]>; Regina Obe <[email protected]>; [email protected]
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;
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: [PATCH] Support % wildcard in extension upgrade filenames
2023-04-11 03:09 RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-11 18:48 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-04-11 20:36 ` RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-11 21:27 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-04-13 22:32 ` RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-24 17:06 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Mat Arye <[email protected]>
2023-04-27 10:49 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
@ 2023-05-31 19:07 ` Sandro Santilli <[email protected]>
2023-06-28 08:29 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Daniel Gustafsson <[email protected]>
0 siblings, 1 reply; 21+ messages in thread
From: Sandro Santilli @ 2023-05-31 19:07 UTC (permalink / raw)
To: [email protected]; +Cc: Mat Arye <[email protected]>; Regina Obe <[email protected]>; Yurii Rashkovskii <[email protected]>; Tom Lane <[email protected]>; Regina Obe <[email protected]>
On Thu, Apr 27, 2023 at 12:49:57PM +0200, Sandro Santilli wrote:
> On Mon, Apr 24, 2023 at 01:06:24PM -0400, Mat Arye wrote:
> > Hi All,
> >
> > I've done upgrade maintenance for multiple extensions now (TimescaleDB
> > core, Promscale) and I think the original suggestion (wildcard filenames
> > with control-file switch to enable) here is a good one.
>
> Thanks for your comment, Mat.
>
> I'm happy to bring back the control-file switch if there's an
> agreement about that.
I'm attaching an up-to-date version of the patch with the control-file
switch back in, so there's an explicit choice by extension developers.
--strk;
Attachments:
[text/x-diff] v3-0001-Allow-wildcard-in-extension-upgrade-paths.patch (10.3K, ../../20230531190743.nzktxombkupb25ud@c19/2-v3-0001-Allow-wildcard-in-extension-upgrade-paths.patch)
download | inline diff:
From 466338401ce8305b7ac9aa59386816c3a6884f02 Mon Sep 17 00:00:00 2001
From: Sandro Santilli <[email protected]>
Date: Wed, 14 Sep 2022 11:10:10 +0200
Subject: [PATCH v3] Allow wildcard (%) in extension upgrade paths
A wildcard character "%" will be accepted in the
"source" side of the upgrade script and be considered
usable to upgrade any version to the "target" side.
Using wildcards needs to be explicitly requested by
extensions via a "wildcard_upgrades" setting in their
control file.
Includes regression test and documentation.
---
doc/src/sgml/extend.sgml | 14 +++++
src/backend/commands/extension.c | 58 +++++++++++++++++--
src/test/modules/test_extensions/Makefile | 7 ++-
.../expected/test_extensions.out | 15 +++++
.../test_extensions/sql/test_extensions.sql | 9 +++
.../test_ext_wildcard1--%--2.0.sql | 6 ++
.../test_ext_wildcard1--1.0.sql | 6 ++
.../test_ext_wildcard1.control | 4 ++
8 files changed, 110 insertions(+), 9 deletions(-)
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1.control
diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml
index 218940ee5c..3d4003eaef 100644
--- a/doc/src/sgml/extend.sgml
+++ b/doc/src/sgml/extend.sgml
@@ -822,6 +822,20 @@ RETURNS anycompatible AS ...
</para>
</listitem>
</varlistentry>
+
+ <varlistentry id="extend-extensions-wildcard-upgrade">
+ <term><varname>wildcard_upgrades</varname> (<type>boolean</type>)</term>
+ <listitem>
+ <para>
+ This parameter, if set to <literal>true</literal> (which is not the
+ default), allows <command>ALTER EXTENSION</command> to consider
+ a wildcard character <literal>%</literal> as matching any version of
+ the extension. Such wildcard match will only be used when no
+ perfect match is found for a version.
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
<para>
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 0eabe18335..207b4649f2 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -88,6 +88,7 @@ typedef struct ExtensionControlFile
bool relocatable; /* is ALTER EXTENSION SET SCHEMA supported? */
bool superuser; /* must be superuser to install? */
bool trusted; /* allow becoming superuser on the fly? */
+ bool wildcard_upgrades; /* allow using wildcards in upgrade scripts */
int encoding; /* encoding of the script file, or -1 */
List *requires; /* names of prerequisite extensions */
List *no_relocate; /* names of prerequisite extensions that
@@ -132,6 +133,7 @@ static void ApplyExtensionUpdates(Oid extensionOid,
bool cascade,
bool is_create);
static char *read_whole_file(const char *filename, int *length);
+static bool file_exists(const char *name);
/*
@@ -584,6 +586,14 @@ parse_extension_control_file(ExtensionControlFile *control,
errmsg("parameter \"%s\" requires a Boolean value",
item->name)));
}
+ else if (strcmp(item->name, "wildcard_upgrades") == 0)
+ {
+ if (!parse_bool(item->value, &control->wildcard_upgrades))
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("parameter \"%s\" requires a Boolean value",
+ item->name)));
+ }
else if (strcmp(item->name, "encoding") == 0)
{
control->encoding = pg_valid_server_encoding(item->value);
@@ -656,6 +666,7 @@ read_extension_control_file(const char *extname)
control->relocatable = false;
control->superuser = true;
control->trusted = false;
+ control->wildcard_upgrades = false;
control->encoding = -1;
/*
@@ -913,7 +924,15 @@ execute_extension_script(Oid extensionOid, ExtensionControlFile *control,
if (from_version == NULL)
elog(DEBUG1, "executing extension script for \"%s\" version '%s'", control->name, version);
else
+ {
+ if ( control->wildcard_upgrades && ! file_exists(filename) )
+ {
+ elog(DEBUG1, "extension upgrade script \"%s\" does not exist, will try wildcard", filename);
+ /* if filename does not exist, try wildcard */
+ filename = get_extension_script_filename(control, "%", version);
+ }
elog(DEBUG1, "executing extension script for \"%s\" update from version '%s' to '%s'", control->name, from_version, version);
+ }
/*
* If installing a trusted extension on behalf of a non-superuser, become
@@ -1259,13 +1278,23 @@ identify_update_path(ExtensionControlFile *control,
/* Find shortest path */
result = find_update_path(evi_list, evi_start, evi_target, false, false);
- if (result == NIL)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("extension \"%s\" has no update path from version \"%s\" to version \"%s\"",
- control->name, oldVersion, newVersion)));
+ if (result != NIL)
+ return result;
- return result;
+ /* Find wildcard path, if allowed by control file */
+ if ( control->wildcard_upgrades )
+ {
+ evi_start = get_ext_ver_info("%", &evi_list);
+ result = find_update_path(evi_list, evi_start, evi_target, false, false);
+
+ if (result != NIL)
+ return result;
+ }
+
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("extension \"%s\" has no update path from version \"%s\" to version \"%s\"",
+ control->name, oldVersion, newVersion)));
}
/*
@@ -3470,3 +3499,20 @@ read_whole_file(const char *filename, int *length)
buf[*length] = '\0';
return buf;
}
+
+static bool
+file_exists(const char *name)
+{
+ struct stat st;
+
+ Assert(name != NULL);
+
+ if (stat(name, &st) == 0)
+ return !S_ISDIR(st.st_mode);
+ else if (!(errno == ENOENT || errno == ENOTDIR || errno == EACCES))
+ ereport(ERROR,
+ (errcode_for_file_access(),
+ errmsg("could not access file \"%s\": %m", name)));
+
+ return false;
+}
diff --git a/src/test/modules/test_extensions/Makefile b/src/test/modules/test_extensions/Makefile
index 70fc0c8e66..5a8205fd5d 100644
--- a/src/test/modules/test_extensions/Makefile
+++ b/src/test/modules/test_extensions/Makefile
@@ -7,8 +7,8 @@ EXTENSION = test_ext1 test_ext2 test_ext3 test_ext4 test_ext5 test_ext6 \
test_ext7 test_ext8 test_ext_cine test_ext_cor \
test_ext_cyclic1 test_ext_cyclic2 \
test_ext_evttrig \
- test_ext_req_schema1 test_ext_req_schema2 test_ext_req_schema3
-
+ test_ext_req_schema1 test_ext_req_schema2 test_ext_req_schema3 \
+ test_ext_wildcard1
DATA = test_ext1--1.0.sql test_ext2--1.0.sql test_ext3--1.0.sql \
test_ext4--1.0.sql test_ext5--1.0.sql test_ext6--1.0.sql \
test_ext7--1.0.sql test_ext7--1.0--2.0.sql test_ext8--1.0.sql \
@@ -18,7 +18,8 @@ DATA = test_ext1--1.0.sql test_ext2--1.0.sql test_ext3--1.0.sql \
test_ext_evttrig--1.0.sql test_ext_evttrig--1.0--2.0.sql \
test_ext_req_schema1--1.0.sql \
test_ext_req_schema2--1.0.sql \
- test_ext_req_schema3--1.0.sql
+ test_ext_req_schema3--1.0.sql \
+ test_ext_wildcard1--1.0.sql test_ext_wildcard1--%--2.0.sql
REGRESS = test_extensions test_extdepend
diff --git a/src/test/modules/test_extensions/expected/test_extensions.out b/src/test/modules/test_extensions/expected/test_extensions.out
index a31775a260..0d4d8b4b70 100644
--- a/src/test/modules/test_extensions/expected/test_extensions.out
+++ b/src/test/modules/test_extensions/expected/test_extensions.out
@@ -389,3 +389,18 @@ SELECT test_s_dep.dep_req2();
DROP EXTENSION test_ext_req_schema1 CASCADE;
NOTICE: drop cascades to extension test_ext_req_schema2
+CREATE EXTENSION test_ext_wildcard1;
+SELECT ext_wildcard1_version();
+ ext_wildcard1_version
+-----------------------
+ 1.0
+(1 row)
+
+ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0';
+SELECT ext_wildcard1_version();
+ ext_wildcard1_version
+-----------------------
+ 2.0
+(1 row)
+
+DROP EXTENSION test_ext_wildcard1;
diff --git a/src/test/modules/test_extensions/sql/test_extensions.sql b/src/test/modules/test_extensions/sql/test_extensions.sql
index f4947e7da6..3c40710fc1 100644
--- a/src/test/modules/test_extensions/sql/test_extensions.sql
+++ b/src/test/modules/test_extensions/sql/test_extensions.sql
@@ -232,3 +232,12 @@ ALTER EXTENSION test_ext_req_schema1 SET SCHEMA test_s_dep2; -- now ok
SELECT test_s_dep2.dep_req1();
SELECT test_s_dep.dep_req2();
DROP EXTENSION test_ext_req_schema1 CASCADE;
+
+--
+-- Test wildcard based upgrade paths
+--
+CREATE EXTENSION test_ext_wildcard1;
+SELECT ext_wildcard1_version();
+ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0';
+SELECT ext_wildcard1_version();
+DROP EXTENSION test_ext_wildcard1;
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql b/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
new file mode 100644
index 0000000000..75154e5c55
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
@@ -0,0 +1,6 @@
+/* src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql */
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0'" to load this file. \quit
+
+CREATE OR REPLACE FUNCTION ext_wildcard1_version() returns TEXT
+AS 'SELECT 2.0' LANGUAGE 'sql';
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql b/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
new file mode 100644
index 0000000000..a69e791fda
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
@@ -0,0 +1,6 @@
+/* src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql */
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "CREATE EXTENSION test_ext_wildcard1" to load this file. \quit
+
+CREATE FUNCTION ext_wildcard1_version() returns TEXT
+AS 'SELECT 1.0' LANGUAGE 'sql';
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1.control b/src/test/modules/test_extensions/test_ext_wildcard1.control
new file mode 100644
index 0000000000..865e37fa88
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1.control
@@ -0,0 +1,4 @@
+comment = 'Test extension wildcard 1'
+default_version = '1.0'
+relocatable = true
+wildcard_upgrades = true
--
2.34.1
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: [PATCH] Support % wildcard in extension upgrade filenames
2023-04-11 03:09 RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-11 18:48 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-04-11 20:36 ` RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-11 21:27 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-04-13 22:32 ` RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-24 17:06 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Mat Arye <[email protected]>
2023-04-27 10:49 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-05-31 19:07 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
@ 2023-06-28 08:29 ` Daniel Gustafsson <[email protected]>
2023-08-01 18:24 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Daniel Gustafsson <[email protected]>
0 siblings, 1 reply; 21+ messages in thread
From: Daniel Gustafsson @ 2023-06-28 08:29 UTC (permalink / raw)
To: Sandro Santilli <[email protected]>; +Cc: Postgres hackers <[email protected]>; Mat Arye <[email protected]>; Regina Obe <[email protected]>; Yurii Rashkovskii <[email protected]>; Tom Lane <[email protected]>; Regina Obe <[email protected]>
> On 31 May 2023, at 21:07, Sandro Santilli <[email protected]> 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
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: [PATCH] Support % wildcard in extension upgrade filenames
2023-04-11 03:09 RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-11 18:48 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-04-11 20:36 ` RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-11 21:27 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-04-13 22:32 ` RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-24 17:06 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Mat Arye <[email protected]>
2023-04-27 10:49 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-05-31 19:07 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-06-28 08:29 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Daniel Gustafsson <[email protected]>
@ 2023-08-01 18:24 ` Daniel Gustafsson <[email protected]>
2023-08-01 18:45 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Robert Haas <[email protected]>
2023-08-07 13:55 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
0 siblings, 2 replies; 21+ messages in thread
From: Daniel Gustafsson @ 2023-08-01 18:24 UTC (permalink / raw)
To: Sandro Santilli <[email protected]>; +Cc: Postgres hackers <[email protected]>; Mat Arye <[email protected]>; Regina Obe <[email protected]>; Yurii Rashkovskii <[email protected]>; Tom Lane <[email protected]>; Regina Obe <[email protected]>
> On 28 Jun 2023, at 10:29, Daniel Gustafsson <[email protected]> wrote:
>
>> On 31 May 2023, at 21:07, Sandro Santilli <[email protected]> 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
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: [PATCH] Support % wildcard in extension upgrade filenames
2023-04-11 03:09 RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-11 18:48 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-04-11 20:36 ` RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-11 21:27 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-04-13 22:32 ` RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-24 17:06 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Mat Arye <[email protected]>
2023-04-27 10:49 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-05-31 19:07 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-06-28 08:29 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Daniel Gustafsson <[email protected]>
2023-08-01 18:24 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Daniel Gustafsson <[email protected]>
@ 2023-08-01 18:45 ` Robert Haas <[email protected]>
2023-08-01 19:23 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Daniel Gustafsson <[email protected]>
1 sibling, 1 reply; 21+ messages in thread
From: Robert Haas @ 2023-08-01 18:45 UTC (permalink / raw)
To: Daniel Gustafsson <[email protected]>; +Cc: Sandro Santilli <[email protected]>; Postgres hackers <[email protected]>; Mat Arye <[email protected]>; Regina Obe <[email protected]>; Yurii Rashkovskii <[email protected]>; Tom Lane <[email protected]>; Regina Obe <[email protected]>
On Tue, Aug 1, 2023 at 2:24 PM Daniel Gustafsson <[email protected]> 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
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: [PATCH] Support % wildcard in extension upgrade filenames
2023-04-11 03:09 RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-11 18:48 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-04-11 20:36 ` RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-11 21:27 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-04-13 22:32 ` RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-24 17:06 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Mat Arye <[email protected]>
2023-04-27 10:49 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-05-31 19:07 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-06-28 08:29 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Daniel Gustafsson <[email protected]>
2023-08-01 18:24 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Daniel Gustafsson <[email protected]>
2023-08-01 18:45 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Robert Haas <[email protected]>
@ 2023-08-01 19:23 ` Daniel Gustafsson <[email protected]>
2023-08-01 19:59 ` RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-08-01 19:59 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Robert Haas <[email protected]>
0 siblings, 2 replies; 21+ messages in thread
From: Daniel Gustafsson @ 2023-08-01 19:23 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: Sandro Santilli <[email protected]>; Postgres hackers <[email protected]>; Mat Arye <[email protected]>; Regina Obe <[email protected]>; Yurii Rashkovskii <[email protected]>; Tom Lane <[email protected]>; Regina Obe <[email protected]>
> On 1 Aug 2023, at 20:45, Robert Haas <[email protected]> wrote:
>
> On Tue, Aug 1, 2023 at 2:24 PM Daniel Gustafsson <[email protected]> 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
^ permalink raw reply [nested|flat] 21+ messages in thread
* RE: [PATCH] Support % wildcard in extension upgrade filenames
2023-04-11 03:09 RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-11 18:48 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-04-11 20:36 ` RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-11 21:27 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-04-13 22:32 ` RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-24 17:06 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Mat Arye <[email protected]>
2023-04-27 10:49 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-05-31 19:07 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-06-28 08:29 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Daniel Gustafsson <[email protected]>
2023-08-01 18:24 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Daniel Gustafsson <[email protected]>
2023-08-01 18:45 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Robert Haas <[email protected]>
2023-08-01 19:23 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Daniel Gustafsson <[email protected]>
@ 2023-08-01 19:59 ` Regina Obe <[email protected]>
1 sibling, 0 replies; 21+ messages in thread
From: Regina Obe @ 2023-08-01 19:59 UTC (permalink / raw)
To: 'Daniel Gustafsson' <[email protected]>; 'Robert Haas' <[email protected]>; +Cc: 'Sandro Santilli' <[email protected]>; 'Postgres hackers' <[email protected]>; 'Mat Arye' <[email protected]>; 'Yurii Rashkovskii' <[email protected]>; 'Tom Lane' <[email protected]>; 'Regina Obe' <[email protected]>
> > On 1 Aug 2023, at 20:45, Robert Haas <[email protected]> wrote:
> >
> > On Tue, Aug 1, 2023 at 2:24 PM Daniel Gustafsson <[email protected]>
> 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
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: [PATCH] Support % wildcard in extension upgrade filenames
2023-04-11 03:09 RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-11 18:48 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-04-11 20:36 ` RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-11 21:27 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-04-13 22:32 ` RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-24 17:06 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Mat Arye <[email protected]>
2023-04-27 10:49 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-05-31 19:07 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-06-28 08:29 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Daniel Gustafsson <[email protected]>
2023-08-01 18:24 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Daniel Gustafsson <[email protected]>
2023-08-01 18:45 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Robert Haas <[email protected]>
2023-08-01 19:23 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Daniel Gustafsson <[email protected]>
@ 2023-08-01 19:59 ` Robert Haas <[email protected]>
1 sibling, 0 replies; 21+ messages in thread
From: Robert Haas @ 2023-08-01 19:59 UTC (permalink / raw)
To: Daniel Gustafsson <[email protected]>; +Cc: Sandro Santilli <[email protected]>; Postgres hackers <[email protected]>; Mat Arye <[email protected]>; Regina Obe <[email protected]>; Yurii Rashkovskii <[email protected]>; Tom Lane <[email protected]>; Regina Obe <[email protected]>
On Tue, Aug 1, 2023 at 3:23 PM Daniel Gustafsson <[email protected]> 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
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: [PATCH] Support % wildcard in extension upgrade filenames
2023-04-11 03:09 RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-11 18:48 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-04-11 20:36 ` RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-11 21:27 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-04-13 22:32 ` RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-24 17:06 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Mat Arye <[email protected]>
2023-04-27 10:49 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-05-31 19:07 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-06-28 08:29 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Daniel Gustafsson <[email protected]>
2023-08-01 18:24 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Daniel Gustafsson <[email protected]>
@ 2023-08-07 13:55 ` Sandro Santilli <[email protected]>
2024-01-07 07:06 ` Re: [PATCH] Support % wildcard in extension upgrade filenames vignesh C <[email protected]>
1 sibling, 1 reply; 21+ messages in thread
From: Sandro Santilli @ 2023-08-07 13:55 UTC (permalink / raw)
To: Daniel Gustafsson <[email protected]>; +Cc: Postgres hackers <[email protected]>; Mat Arye <[email protected]>; Regina Obe <[email protected]>; Yurii Rashkovskii <[email protected]>; Tom Lane <[email protected]>; Regina Obe <[email protected]>
On Tue, Aug 01, 2023 at 08:24:15PM +0200, Daniel Gustafsson wrote:
> > On 28 Jun 2023, at 10:29, Daniel Gustafsson <[email protected]> wrote:
> >
> >> On 31 May 2023, at 21:07, Sandro Santilli <[email protected]> wrote:
> >> On Thu, Apr 27, 2023 at 12:49:57PM +0200, Sandro Santilli wrote:
> >
> >>> I'm happy to bring back the control-file switch if there's an
> >>> agreement about that.
> >>
> >> I'm attaching an up-to-date version of the patch with the control-file
> >> switch back in, so there's an explicit choice by extension developers.
> >
> > This version fails the extension test since the comment from the .sql file is
> > missing from test_extensions.out. Can you fix that in a rebase such that the
> > CFBot can have a green build of this patch?
>
> With no update to the thread and the patch still not applying I'm marking this
> returned with feedback. Please feel free to resubmit to a future CF when there
> is a new version of the patch.
Updated version of the patch is attached, regresses cleanly.
Will try to submit this to future CF from the website, let me know
if I'm doing it wrong.
--strk;
Attachments:
[text/x-diff] v4-0001-Allow-wildcard-in-extension-upgrade-paths.patch (10.3K, ../../20230807135532.r73nul3ksqr55rbk@c19/2-v4-0001-Allow-wildcard-in-extension-upgrade-paths.patch)
download | inline diff:
From 9b8d1f45e5cd4b1e286d1c82d8ebca4fcc25eb92 Mon Sep 17 00:00:00 2001
From: Sandro Santilli <[email protected]>
Date: Wed, 14 Sep 2022 11:10:10 +0200
Subject: [PATCH v4] Allow wildcard (%) in extension upgrade paths
A wildcard character "%" will be accepted in the
"source" side of the upgrade script and be considered
usable to upgrade any version to the "target" side.
Using wildcards needs to be explicitly requested by
extensions via a "wildcard_upgrades" setting in their
control file.
Includes regression test and documentation.
---
doc/src/sgml/extend.sgml | 14 +++++
src/backend/commands/extension.c | 58 +++++++++++++++++--
src/test/modules/test_extensions/Makefile | 7 ++-
.../expected/test_extensions.out | 18 ++++++
.../test_extensions/sql/test_extensions.sql | 9 +++
.../test_ext_wildcard1--%--2.0.sql | 6 ++
.../test_ext_wildcard1--1.0.sql | 6 ++
.../test_ext_wildcard1.control | 4 ++
8 files changed, 113 insertions(+), 9 deletions(-)
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
create mode 100644 src/test/modules/test_extensions/test_ext_wildcard1.control
diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml
index 218940ee5c..3d4003eaef 100644
--- a/doc/src/sgml/extend.sgml
+++ b/doc/src/sgml/extend.sgml
@@ -822,6 +822,20 @@ RETURNS anycompatible AS ...
</para>
</listitem>
</varlistentry>
+
+ <varlistentry id="extend-extensions-wildcard-upgrade">
+ <term><varname>wildcard_upgrades</varname> (<type>boolean</type>)</term>
+ <listitem>
+ <para>
+ This parameter, if set to <literal>true</literal> (which is not the
+ default), allows <command>ALTER EXTENSION</command> to consider
+ a wildcard character <literal>%</literal> as matching any version of
+ the extension. Such wildcard match will only be used when no
+ perfect match is found for a version.
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
<para>
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 535072d181..c05055f5a0 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -88,6 +88,7 @@ typedef struct ExtensionControlFile
bool relocatable; /* is ALTER EXTENSION SET SCHEMA supported? */
bool superuser; /* must be superuser to install? */
bool trusted; /* allow becoming superuser on the fly? */
+ bool wildcard_upgrades; /* allow using wildcards in upgrade scripts */
int encoding; /* encoding of the script file, or -1 */
List *requires; /* names of prerequisite extensions */
List *no_relocate; /* names of prerequisite extensions that
@@ -132,6 +133,7 @@ static void ApplyExtensionUpdates(Oid extensionOid,
bool cascade,
bool is_create);
static char *read_whole_file(const char *filename, int *length);
+static bool file_exists(const char *name);
/*
@@ -584,6 +586,14 @@ parse_extension_control_file(ExtensionControlFile *control,
errmsg("parameter \"%s\" requires a Boolean value",
item->name)));
}
+ else if (strcmp(item->name, "wildcard_upgrades") == 0)
+ {
+ if (!parse_bool(item->value, &control->wildcard_upgrades))
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("parameter \"%s\" requires a Boolean value",
+ item->name)));
+ }
else if (strcmp(item->name, "encoding") == 0)
{
control->encoding = pg_valid_server_encoding(item->value);
@@ -656,6 +666,7 @@ read_extension_control_file(const char *extname)
control->relocatable = false;
control->superuser = true;
control->trusted = false;
+ control->wildcard_upgrades = false;
control->encoding = -1;
/*
@@ -913,7 +924,15 @@ execute_extension_script(Oid extensionOid, ExtensionControlFile *control,
if (from_version == NULL)
elog(DEBUG1, "executing extension script for \"%s\" version '%s'", control->name, version);
else
+ {
+ if ( control->wildcard_upgrades && ! file_exists(filename) )
+ {
+ elog(DEBUG1, "extension upgrade script \"%s\" does not exist, will try wildcard", filename);
+ /* if filename does not exist, try wildcard */
+ filename = get_extension_script_filename(control, "%", version);
+ }
elog(DEBUG1, "executing extension script for \"%s\" update from version '%s' to '%s'", control->name, from_version, version);
+ }
/*
* If installing a trusted extension on behalf of a non-superuser, become
@@ -1281,13 +1300,23 @@ identify_update_path(ExtensionControlFile *control,
/* Find shortest path */
result = find_update_path(evi_list, evi_start, evi_target, false, false);
- if (result == NIL)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("extension \"%s\" has no update path from version \"%s\" to version \"%s\"",
- control->name, oldVersion, newVersion)));
+ if (result != NIL)
+ return result;
- return result;
+ /* Find wildcard path, if allowed by control file */
+ if ( control->wildcard_upgrades )
+ {
+ evi_start = get_ext_ver_info("%", &evi_list);
+ result = find_update_path(evi_list, evi_start, evi_target, false, false);
+
+ if (result != NIL)
+ return result;
+ }
+
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("extension \"%s\" has no update path from version \"%s\" to version \"%s\"",
+ control->name, oldVersion, newVersion)));
}
/*
@@ -3491,3 +3520,20 @@ read_whole_file(const char *filename, int *length)
buf[*length] = '\0';
return buf;
}
+
+static bool
+file_exists(const char *name)
+{
+ struct stat st;
+
+ Assert(name != NULL);
+
+ if (stat(name, &st) == 0)
+ return !S_ISDIR(st.st_mode);
+ else if (!(errno == ENOENT || errno == ENOTDIR || errno == EACCES))
+ ereport(ERROR,
+ (errcode_for_file_access(),
+ errmsg("could not access file \"%s\": %m", name)));
+
+ return false;
+}
diff --git a/src/test/modules/test_extensions/Makefile b/src/test/modules/test_extensions/Makefile
index 1388c0fb0b..105138d08a 100644
--- a/src/test/modules/test_extensions/Makefile
+++ b/src/test/modules/test_extensions/Makefile
@@ -8,8 +8,8 @@ EXTENSION = test_ext1 test_ext2 test_ext3 test_ext4 test_ext5 test_ext6 \
test_ext_cyclic1 test_ext_cyclic2 \
test_ext_extschema \
test_ext_evttrig \
- test_ext_req_schema1 test_ext_req_schema2 test_ext_req_schema3
-
+ test_ext_req_schema1 test_ext_req_schema2 test_ext_req_schema3 \
+ test_ext_wildcard1
DATA = test_ext1--1.0.sql test_ext2--1.0.sql test_ext3--1.0.sql \
test_ext4--1.0.sql test_ext5--1.0.sql test_ext6--1.0.sql \
test_ext7--1.0.sql test_ext7--1.0--2.0.sql test_ext8--1.0.sql \
@@ -20,7 +20,8 @@ DATA = test_ext1--1.0.sql test_ext2--1.0.sql test_ext3--1.0.sql \
test_ext_evttrig--1.0.sql test_ext_evttrig--1.0--2.0.sql \
test_ext_req_schema1--1.0.sql \
test_ext_req_schema2--1.0.sql \
- test_ext_req_schema3--1.0.sql
+ test_ext_req_schema3--1.0.sql \
+ test_ext_wildcard1--1.0.sql test_ext_wildcard1--%--2.0.sql
REGRESS = test_extensions test_extdepend
diff --git a/src/test/modules/test_extensions/expected/test_extensions.out b/src/test/modules/test_extensions/expected/test_extensions.out
index 472627a232..270840183d 100644
--- a/src/test/modules/test_extensions/expected/test_extensions.out
+++ b/src/test/modules/test_extensions/expected/test_extensions.out
@@ -445,3 +445,21 @@ SELECT test_s_dep.dep_req2();
DROP EXTENSION test_ext_req_schema1 CASCADE;
NOTICE: drop cascades to extension test_ext_req_schema2
+--
+-- Test wildcard based upgrade paths
+--
+CREATE EXTENSION test_ext_wildcard1;
+SELECT ext_wildcard1_version();
+ ext_wildcard1_version
+-----------------------
+ 1.0
+(1 row)
+
+ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0';
+SELECT ext_wildcard1_version();
+ ext_wildcard1_version
+-----------------------
+ 2.0
+(1 row)
+
+DROP EXTENSION test_ext_wildcard1;
diff --git a/src/test/modules/test_extensions/sql/test_extensions.sql b/src/test/modules/test_extensions/sql/test_extensions.sql
index 51327cc321..bb567e0f19 100644
--- a/src/test/modules/test_extensions/sql/test_extensions.sql
+++ b/src/test/modules/test_extensions/sql/test_extensions.sql
@@ -276,3 +276,12 @@ ALTER EXTENSION test_ext_req_schema1 SET SCHEMA test_s_dep2; -- now ok
SELECT test_s_dep2.dep_req1();
SELECT test_s_dep.dep_req2();
DROP EXTENSION test_ext_req_schema1 CASCADE;
+
+--
+-- Test wildcard based upgrade paths
+--
+CREATE EXTENSION test_ext_wildcard1;
+SELECT ext_wildcard1_version();
+ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0';
+SELECT ext_wildcard1_version();
+DROP EXTENSION test_ext_wildcard1;
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql b/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
new file mode 100644
index 0000000000..75154e5c55
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql
@@ -0,0 +1,6 @@
+/* src/test/modules/test_extensions/test_ext_wildcard1--%--2.0.sql */
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "ALTER EXTENSION test_ext_wildcard1 UPDATE TO '2.0'" to load this file. \quit
+
+CREATE OR REPLACE FUNCTION ext_wildcard1_version() returns TEXT
+AS 'SELECT 2.0' LANGUAGE 'sql';
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql b/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
new file mode 100644
index 0000000000..a69e791fda
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql
@@ -0,0 +1,6 @@
+/* src/test/modules/test_extensions/test_ext_wildcard1--1.0.sql */
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "CREATE EXTENSION test_ext_wildcard1" to load this file. \quit
+
+CREATE FUNCTION ext_wildcard1_version() returns TEXT
+AS 'SELECT 1.0' LANGUAGE 'sql';
diff --git a/src/test/modules/test_extensions/test_ext_wildcard1.control b/src/test/modules/test_extensions/test_ext_wildcard1.control
new file mode 100644
index 0000000000..865e37fa88
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_wildcard1.control
@@ -0,0 +1,4 @@
+comment = 'Test extension wildcard 1'
+default_version = '1.0'
+relocatable = true
+wildcard_upgrades = true
--
2.34.1
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: [PATCH] Support % wildcard in extension upgrade filenames
2023-04-11 03:09 RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-11 18:48 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-04-11 20:36 ` RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-11 21:27 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-04-13 22:32 ` RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-24 17:06 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Mat Arye <[email protected]>
2023-04-27 10:49 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-05-31 19:07 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-06-28 08:29 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Daniel Gustafsson <[email protected]>
2023-08-01 18:24 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Daniel Gustafsson <[email protected]>
2023-08-07 13:55 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
@ 2024-01-07 07:06 ` vignesh C <[email protected]>
2024-02-01 11:01 ` Re: [PATCH] Support % wildcard in extension upgrade filenames vignesh C <[email protected]>
0 siblings, 1 reply; 21+ messages in thread
From: vignesh C @ 2024-01-07 07:06 UTC (permalink / raw)
To: Sandro Santilli <[email protected]>; +Cc: Daniel Gustafsson <[email protected]>; Postgres hackers <[email protected]>; Mat Arye <[email protected]>; Regina Obe <[email protected]>; Yurii Rashkovskii <[email protected]>; Tom Lane <[email protected]>; Regina Obe <[email protected]>
On Mon, 7 Aug 2023 at 19:25, Sandro Santilli <[email protected]> wrote:
>
> On Tue, Aug 01, 2023 at 08:24:15PM +0200, Daniel Gustafsson wrote:
> > > On 28 Jun 2023, at 10:29, Daniel Gustafsson <[email protected]> wrote:
> > >
> > >> On 31 May 2023, at 21:07, Sandro Santilli <[email protected]> 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/re...
Regards,
Vignesh
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: [PATCH] Support % wildcard in extension upgrade filenames
2023-04-11 03:09 RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-11 18:48 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-04-11 20:36 ` RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-11 21:27 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-04-13 22:32 ` RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-24 17:06 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Mat Arye <[email protected]>
2023-04-27 10:49 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-05-31 19:07 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-06-28 08:29 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Daniel Gustafsson <[email protected]>
2023-08-01 18:24 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Daniel Gustafsson <[email protected]>
2023-08-07 13:55 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2024-01-07 07:06 ` Re: [PATCH] Support % wildcard in extension upgrade filenames vignesh C <[email protected]>
@ 2024-02-01 11:01 ` vignesh C <[email protected]>
2024-02-01 17:58 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
0 siblings, 1 reply; 21+ messages in thread
From: vignesh C @ 2024-02-01 11:01 UTC (permalink / raw)
To: Sandro Santilli <[email protected]>; +Cc: Daniel Gustafsson <[email protected]>; Postgres hackers <[email protected]>; Mat Arye <[email protected]>; Regina Obe <[email protected]>; Yurii Rashkovskii <[email protected]>; Tom Lane <[email protected]>; Regina Obe <[email protected]>
On Sun, 7 Jan 2024 at 12:36, vignesh C <[email protected]> wrote:
>
> On Mon, 7 Aug 2023 at 19:25, Sandro Santilli <[email protected]> wrote:
> >
> > On Tue, Aug 01, 2023 at 08:24:15PM +0200, Daniel Gustafsson wrote:
> > > > On 28 Jun 2023, at 10:29, Daniel Gustafsson <[email protected]> wrote:
> > > >
> > > >> On 31 May 2023, at 21:07, Sandro Santilli <[email protected]> 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
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: [PATCH] Support % wildcard in extension upgrade filenames
2023-04-11 03:09 RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-11 18:48 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-04-11 20:36 ` RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-11 21:27 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-04-13 22:32 ` RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-24 17:06 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Mat Arye <[email protected]>
2023-04-27 10:49 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-05-31 19:07 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2023-06-28 08:29 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Daniel Gustafsson <[email protected]>
2023-08-01 18:24 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Daniel Gustafsson <[email protected]>
2023-08-07 13:55 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2024-01-07 07:06 ` Re: [PATCH] Support % wildcard in extension upgrade filenames vignesh C <[email protected]>
2024-02-01 11:01 ` Re: [PATCH] Support % wildcard in extension upgrade filenames vignesh C <[email protected]>
@ 2024-02-01 17:58 ` Sandro Santilli <[email protected]>
0 siblings, 0 replies; 21+ messages in thread
From: Sandro Santilli @ 2024-02-01 17:58 UTC (permalink / raw)
To: vignesh C <[email protected]>; +Cc: Daniel Gustafsson <[email protected]>; Postgres hackers <[email protected]>; Mat Arye <[email protected]>; Regina Obe <[email protected]>; Yurii Rashkovskii <[email protected]>; Tom Lane <[email protected]>; Regina Obe <[email protected]>
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;
Attachments:
[application/pgp-signature] signature.asc (659B, ../../ZbvbsUd75WsF9lPF@c19/2-signature.asc)
download
^ permalink raw reply [nested|flat] 21+ messages in thread
* [PATCH v3 10/17] Inline heap_freeze_execute_prepared()
@ 2024-01-07 22:03 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 21+ messages in thread
From: Melanie Plageman @ 2024-01-07 22:03 UTC (permalink / raw)
In order to merge freeze and prune records, the execution of tuple
freezing and the WAL logging of the changes to the page must be
separated so that the WAL logging can be combined with prune WAL
logging. This commit makes a helper for the tuple freezing and then
inlines the contents of heap_freeze_execute_prepared() where it is
called in heap_page_prune(). The original function,
heap_freeze_execute_prepared() is retained because the "no prune" case
in heap_page_prune() must still be able to emit a freeze record.
---
src/backend/access/heap/heapam.c | 61 +++++++++++++++++------------
src/backend/access/heap/pruneheap.c | 51 ++++++++++++++++++++++--
src/include/access/heapam.h | 8 ++++
3 files changed, 90 insertions(+), 30 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 16e3f2520a4..a3691584c55 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -91,9 +91,6 @@ static void compute_new_xmax_infomask(TransactionId xmax, uint16 old_infomask,
static TM_Result heap_lock_updated_tuple(Relation rel, HeapTuple tuple,
ItemPointer ctid, TransactionId xid,
LockTupleMode mode);
-static int heap_log_freeze_plan(HeapTupleFreeze *tuples, int ntuples,
- xl_heap_freeze_plan *plans_out,
- OffsetNumber *offsets_out);
static void GetMultiXactIdHintBits(MultiXactId multi, uint16 *new_infomask,
uint16 *new_infomask2);
static TransactionId MultiXactIdGetUpdateXid(TransactionId xmax,
@@ -6713,30 +6710,17 @@ heap_pre_freeze_checks(Buffer buffer,
}
/*
- * heap_freeze_execute_prepared
- *
- * Executes freezing of one or more heap tuples on a page on behalf of caller.
- * Caller passes an array of tuple plans from heap_prepare_freeze_tuple.
- * Caller must set 'offset' in each plan for us. Note that we destructively
- * sort caller's tuples array in-place, so caller had better be done with it.
- *
- * WAL-logs the changes so that VACUUM can advance the rel's relfrozenxid
- * later on without any risk of unsafe pg_xact lookups, even following a hard
- * crash (or when querying from a standby). We represent freezing by setting
- * infomask bits in tuple headers, but this shouldn't be thought of as a hint.
- * See section on buffer access rules in src/backend/storage/buffer/README.
+ * Helper which executes freezing of one or more heap tuples on a page on
+ * behalf of caller. Caller passes an array of tuple plans from
+ * heap_prepare_freeze_tuple. Caller must set 'offset' in each plan for us.
+ * Must be called in a critical section that also marks the buffer dirty and,
+ * if needed, emits WAL.
*/
void
-heap_freeze_execute_prepared(Relation rel, Buffer buffer,
- TransactionId snapshotConflictHorizon,
- HeapTupleFreeze *tuples, int ntuples)
+heap_freeze_prepared_tuples(Buffer buffer, HeapTupleFreeze *tuples, int ntuples)
{
Page page = BufferGetPage(buffer);
- Assert(ntuples > 0);
-
- START_CRIT_SECTION();
-
for (int i = 0; i < ntuples; i++)
{
HeapTupleFreeze *frz = tuples + i;
@@ -6746,6 +6730,29 @@ heap_freeze_execute_prepared(Relation rel, Buffer buffer,
htup = (HeapTupleHeader) PageGetItem(page, itemid);
heap_execute_freeze_tuple(htup, frz);
}
+}
+
+/*
+ * heap_freeze_execute_prepared
+ *
+ * Execute freezing of prepared tuples and WAL-logs the changes so that VACUUM
+ * can advance the rel's relfrozenxid later on without any risk of unsafe
+ * pg_xact lookups, even following a hard crash (or when querying from a
+ * standby). We represent freezing by setting infomask bits in tuple headers,
+ * but this shouldn't be thought of as a hint. See section on buffer access
+ * rules in src/backend/storage/buffer/README. Must be called from within a
+ * critical section.
+ */
+void
+heap_freeze_execute_prepared(Relation rel, Buffer buffer,
+ TransactionId snapshotConflictHorizon,
+ HeapTupleFreeze *tuples, int ntuples)
+{
+ Page page = BufferGetPage(buffer);
+
+ Assert(ntuples > 0);
+
+ heap_freeze_prepared_tuples(buffer, tuples, ntuples);
MarkBufferDirty(buffer);
@@ -6758,7 +6765,11 @@ heap_freeze_execute_prepared(Relation rel, Buffer buffer,
xl_heap_freeze_page xlrec;
XLogRecPtr recptr;
- /* Prepare deduplicated representation for use in WAL record */
+ /*
+ * Prepare deduplicated representation for use in WAL record
+ * Destructively sorts tuples array in-place, so caller had better be
+ * done with it.
+ */
nplans = heap_log_freeze_plan(tuples, ntuples, plans, offsets);
xlrec.snapshotConflictHorizon = snapshotConflictHorizon;
@@ -6783,8 +6794,6 @@ heap_freeze_execute_prepared(Relation rel, Buffer buffer,
PageSetLSN(page, recptr);
}
-
- END_CRIT_SECTION();
}
/*
@@ -6874,7 +6883,7 @@ heap_log_freeze_new_plan(xl_heap_freeze_plan *plan, HeapTupleFreeze *frz)
* (actually there is one array per freeze plan, but that's not of immediate
* concern to our caller).
*/
-static int
+int
heap_log_freeze_plan(HeapTupleFreeze *tuples, int ntuples,
xl_heap_freeze_plan *plans_out,
OffsetNumber *offsets_out)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index bc0a23da61b..d4356e0bce9 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -627,10 +627,53 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
if (do_freeze)
{
- /* Execute all freeze plans for page as a single atomic action */
- heap_freeze_execute_prepared(relation, buffer,
- frz_conflict_horizon,
- frozen, presult->nfrozen);
+ START_CRIT_SECTION();
+
+ Assert(presult->nfrozen > 0);
+
+ heap_freeze_prepared_tuples(buffer, frozen, presult->nfrozen);
+
+ MarkBufferDirty(buffer);
+
+ /* Now WAL-log freezing if necessary */
+ if (RelationNeedsWAL(relation))
+ {
+ xl_heap_freeze_plan plans[MaxHeapTuplesPerPage];
+ OffsetNumber offsets[MaxHeapTuplesPerPage];
+ int nplans;
+ xl_heap_freeze_page xlrec;
+ XLogRecPtr recptr;
+
+ /*
+ * Prepare deduplicated representation for use in WAL record
+ * Destructively sorts tuples array in-place.
+ */
+ nplans = heap_log_freeze_plan(frozen, presult->nfrozen, plans, offsets);
+
+ xlrec.snapshotConflictHorizon = frz_conflict_horizon;
+ xlrec.isCatalogRel = RelationIsAccessibleInLogicalDecoding(relation);
+ xlrec.nplans = nplans;
+
+ XLogBeginInsert();
+ XLogRegisterData((char *) &xlrec, SizeOfHeapFreezePage);
+
+ /*
+ * The freeze plan array and offset array are not actually in the
+ * buffer, but pretend that they are. When XLogInsert stores the
+ * whole buffer, the arrays need not be stored too.
+ */
+ XLogRegisterBuffer(0, buffer, REGBUF_STANDARD);
+ XLogRegisterBufData(0, (char *) plans,
+ nplans * sizeof(xl_heap_freeze_plan));
+ XLogRegisterBufData(0, (char *) offsets,
+ presult->nfrozen * sizeof(OffsetNumber));
+
+ recptr = XLogInsert(RM_HEAP2_ID, XLOG_HEAP2_FREEZE_PAGE);
+
+ PageSetLSN(page, recptr);
+ }
+
+ END_CRIT_SECTION();
}
else if (!pagefrz || !presult->all_frozen || presult->nfrozen > 0)
{
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index dffbbd3cd3e..8a6bc071345 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -14,6 +14,7 @@
#ifndef HEAPAM_H
#define HEAPAM_H
+#include "access/heapam_xlog.h"
#include "access/relation.h" /* for backward compatibility */
#include "access/relscan.h"
#include "access/sdir.h"
@@ -320,9 +321,16 @@ extern void heap_pre_freeze_checks(Buffer buffer,
extern void heap_freeze_execute_prepared(Relation rel, Buffer buffer,
TransactionId snapshotConflictHorizon,
HeapTupleFreeze *tuples, int ntuples);
+
+extern void heap_freeze_prepared_tuples(Buffer buffer,
+ HeapTupleFreeze *tuples, int ntuples);
extern bool heap_freeze_tuple(HeapTupleHeader tuple,
TransactionId relfrozenxid, TransactionId relminmxid,
TransactionId FreezeLimit, TransactionId MultiXactCutoff);
+
+extern int heap_log_freeze_plan(HeapTupleFreeze *tuples, int ntuples,
+ xl_heap_freeze_plan *plans_out,
+ OffsetNumber *offsets_out);
extern bool heap_tuple_should_freeze(HeapTupleHeader tuple,
const struct VacuumCutoffs *cutoffs,
TransactionId *NoFreezePageRelfrozenXid,
--
2.40.1
--racicctn4wry6xe5
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v3-0011-Exit-heap_page_prune-early-if-no-prune.patch"
^ permalink raw reply [nested|flat] 21+ messages in thread
end of thread, other threads:[~2024-02-01 17:58 UTC | newest]
Thread overview: 21+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-04-11 03:09 RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2023-04-11 18:48 ` Sandro Santilli <[email protected]>
2023-04-11 20:36 ` Regina Obe <[email protected]>
2023-04-11 21:27 ` Sandro Santilli <[email protected]>
2023-04-11 22:19 ` Regina Obe <[email protected]>
2023-04-13 22:32 ` Regina Obe <[email protected]>
2023-04-24 17:06 ` Mat Arye <[email protected]>
2023-04-24 18:00 ` Regina Obe <[email protected]>
2023-04-27 10:49 ` Sandro Santilli <[email protected]>
2023-05-31 19:07 ` Sandro Santilli <[email protected]>
2023-06-28 08:29 ` Daniel Gustafsson <[email protected]>
2023-08-01 18:24 ` Daniel Gustafsson <[email protected]>
2023-08-01 18:45 ` Robert Haas <[email protected]>
2023-08-01 19:23 ` Daniel Gustafsson <[email protected]>
2023-08-01 19:59 ` Regina Obe <[email protected]>
2023-08-01 19:59 ` Robert Haas <[email protected]>
2023-08-07 13:55 ` Sandro Santilli <[email protected]>
2024-01-07 07:06 ` vignesh C <[email protected]>
2024-02-01 11:01 ` vignesh C <[email protected]>
2024-02-01 17:58 ` Sandro Santilli <[email protected]>
2024-01-07 22:03 [PATCH v3 10/17] Inline heap_freeze_execute_prepared() Melanie Plageman <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox