public inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH 02/10] Run 011_crash_recovery.pl with wal_level=minimal
24+ messages / 12 participants
[nested] [flat]

* [PATCH 02/10] Run 011_crash_recovery.pl with wal_level=minimal
@ 2021-03-08 06:32  Kyotaro Horiguchi <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: Kyotaro Horiguchi @ 2021-03-08 06:32 UTC (permalink / raw)

The test doesn't need that feature and pg_current_xact_id() is better
exercised by turning off the feature.

Copied from: https://www.postgresql.org/message-id/20210308.173242.463790587797836129.horikyota.ntt%40gmail.com
---
 src/test/recovery/t/011_crash_recovery.pl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/test/recovery/t/011_crash_recovery.pl b/src/test/recovery/t/011_crash_recovery.pl
index 10cd98f70a..690655dda2 100644
--- a/src/test/recovery/t/011_crash_recovery.pl
+++ b/src/test/recovery/t/011_crash_recovery.pl
@@ -11,7 +11,7 @@ use Config;
 plan tests => 3;
 
 my $node = get_new_node('primary');
-$node->init(allows_streaming => 1);
+$node->init();
 $node->start;
 
 my ($stdin, $stdout, $stderr) = ('', '', '');
-- 
2.17.0


--0qVF/w3MHQqLSynd
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="0003-Make-sure-published-XIDs-are-persistent.patch"



^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* [PATCH] Support % wildcard in extension upgrade filenames
@ 2022-02-11 17:59  Sandro Santilli <[email protected]>
  0 siblings, 1 reply; 24+ messages in thread

From: Sandro Santilli @ 2022-02-11 17:59 UTC (permalink / raw)
  To: pgsql-hackers

At PostGIS we've been thinking of ways to have better support, from
PostgreSQL proper, to our upgrade strategy, which has always consisted
in a SINGLE upgrade file good for upgrading from any older version.

The current lack of such support for EXTENSIONs forced us to install
a lot of files on disk and we'd like to stop doing that at some point
in the future.

The proposal is to support wildcards for versions encoded in the
filename so that (for our case) a single wildcard could match "any
version". I've been thinking about the '%' character for that, to
resemble the wildcard used for LIKE.

Here's the proposal:
https://lists.osgeo.org/pipermail/postgis-devel/2022-February/029500.html

A very very short (and untested) patch which might (or
might not) support our case is attached.

The only problem with my proposal/patch would be the presence, on the
wild, of PostgreSQL EXTENSION actually using the '%' character in
their version strings, which is currently considered legit by
PostgreSQL.

How do you feel about the proposal (which is wider than the patch) ?

--strk; 

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


Attachments:

  [text/x-diff] 0001-Support-wildcard-in-extension-upgrade-scripts.patch (769B, ../../YgakFklJyM5pNdt+@c19/2-0001-Support-wildcard-in-extension-upgrade-scripts.patch)
  download | inline diff:
From e9d060b19c655924c4edb6679169261d605f735d Mon Sep 17 00:00:00 2001
From: Sandro Santilli <[email protected]>
Date: Fri, 11 Feb 2022 18:49:45 +0100
Subject: [PATCH] Support % wildcard in extension upgrade scripts

---
 src/backend/commands/extension.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index a2e77c418a..aafd9c17ee 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -1072,6 +1072,8 @@ get_ext_ver_info(const char *versionname, List **evi_list)
 	foreach(lc, *evi_list)
 	{
 		evi = (ExtensionVersionInfo *) lfirst(lc);
+		if (strcmp(evi->name, "%") == 0)
+			return evi;
 		if (strcmp(evi->name, versionname) == 0)
 			return evi;
 	}
-- 
2.30.2



^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* RE: [PATCH] Support % wildcard in extension upgrade filenames
@ 2022-05-27 21:37  Regina Obe <[email protected]>
  parent: Sandro Santilli <[email protected]>
  0 siblings, 1 reply; 24+ messages in thread

From: Regina Obe @ 2022-05-27 21:37 UTC (permalink / raw)
  To: [email protected]; pgsql-hackers

> 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









^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* Re: [PATCH] Support % wildcard in extension upgrade filenames
@ 2022-05-28 14:50  Laurenz Albe <[email protected]>
  parent: Regina Obe <[email protected]>
  0 siblings, 3 replies; 24+ messages in thread

From: Laurenz Albe @ 2022-05-28 14:50 UTC (permalink / raw)
  To: Regina Obe <[email protected]>; [email protected]; pgsql-hackers

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





^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* Re: [PATCH] Support % wildcard in extension upgrade filenames
@ 2022-05-28 15:26  Daniel Gustafsson <[email protected]>
  parent: Laurenz Albe <[email protected]>
  2 siblings, 1 reply; 24+ messages in thread

From: Daniel Gustafsson @ 2022-05-28 15:26 UTC (permalink / raw)
  To: Laurenz Albe <[email protected]>; +Cc: Regina Obe <[email protected]>; [email protected]; pgsql-hackers

> On 28 May 2022, at 16:50, Laurenz Albe <[email protected]> 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/






^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* Re: [PATCH] Support % wildcard in extension upgrade filenames
@ 2022-05-28 15:37  Tom Lane <[email protected]>
  parent: Laurenz Albe <[email protected]>
  2 siblings, 2 replies; 24+ messages in thread

From: Tom Lane @ 2022-05-28 15:37 UTC (permalink / raw)
  To: Laurenz Albe <[email protected]>; +Cc: Regina Obe <[email protected]>; [email protected]; pgsql-hackers

Laurenz Albe <[email protected]> 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





^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* Re: [PATCH] Support % wildcard in extension upgrade filenames
@ 2022-06-04 09:20  Sandro Santilli <[email protected]>
  parent: Laurenz Albe <[email protected]>
  2 siblings, 1 reply; 24+ messages in thread

From: Sandro Santilli @ 2022-06-04 09:20 UTC (permalink / raw)
  To: Laurenz Albe <[email protected]>; +Cc: Regina Obe <[email protected]>; pgsql-hackers

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





^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* Re: [PATCH] Support % wildcard in extension upgrade filenames
@ 2022-06-04 09:21  Sandro Santilli <[email protected]>
  parent: Daniel Gustafsson <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: Sandro Santilli @ 2022-06-04 09:21 UTC (permalink / raw)
  To: Daniel Gustafsson <[email protected]>; +Cc: Laurenz Albe <[email protected]>; Regina Obe <[email protected]>; pgsql-hackers

On Sat, May 28, 2022 at 05:26:05PM +0200, Daniel Gustafsson wrote:
> > On 28 May 2022, at 16:50, Laurenz Albe <[email protected]> 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;





^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* Re: [PATCH] Support % wildcard in extension upgrade filenames
@ 2022-06-04 09:26  Sandro Santilli <[email protected]>
  parent: Tom Lane <[email protected]>
  1 sibling, 0 replies; 24+ messages in thread

From: Sandro Santilli @ 2022-06-04 09:26 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Laurenz Albe <[email protected]>; Regina Obe <[email protected]>; pgsql-hackers

On Sat, May 28, 2022 at 11:37:30AM -0400, Tom Lane wrote:
> Laurenz Albe <[email protected]> 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;





^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* Re: [PATCH] Support % wildcard in extension upgrade filenames
@ 2022-09-14 22:01  Sandro Santilli <[email protected]>
  parent: Sandro Santilli <[email protected]>
  0 siblings, 2 replies; 24+ messages in thread

From: Sandro Santilli @ 2022-09-14 22:01 UTC (permalink / raw)
  To: pgsql-hackers; +Cc: Laurenz Albe <[email protected]>; Regina Obe <[email protected]>; Tom Lane <[email protected]>

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





^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* Re: [PATCH] Support % wildcard in extension upgrade filenames
@ 2022-09-14 22:02  Sandro Santilli <[email protected]>
  parent: Sandro Santilli <[email protected]>
  1 sibling, 1 reply; 24+ messages in thread

From: Sandro Santilli @ 2022-09-14 22:02 UTC (permalink / raw)
  To: pgsql-hackers; Laurenz Albe <[email protected]>; Regina Obe <[email protected]>; Tom Lane <[email protected]>

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; 


^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* RE: [PATCH] Support % wildcard in extension upgrade filenames
@ 2022-10-31 05:55  Regina Obe <[email protected]>
  parent: Sandro Santilli <[email protected]>
  0 siblings, 1 reply; 24+ messages in thread

From: Regina Obe @ 2022-10-31 05:55 UTC (permalink / raw)
  To: [email protected]; pgsql-hackers; 'Laurenz Albe' <[email protected]>; 'Tom Lane' <[email protected]>

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










^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* Re: [PATCH] Support % wildcard in extension upgrade filenames
@ 2022-11-04 17:31  Sandro Santilli <[email protected]>
  parent: Regina Obe <[email protected]>
  0 siblings, 1 reply; 24+ messages in thread

From: Sandro Santilli @ 2022-11-04 17:31 UTC (permalink / raw)
  To: Regina Obe <[email protected]>; +Cc: pgsql-hackers; 'Laurenz Albe' <[email protected]>; 'Tom Lane' <[email protected]>

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;


^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* Re: [PATCH] Support % wildcard in extension upgrade filenames
@ 2022-11-09 17:53  Sandro Santilli <[email protected]>
  parent: Sandro Santilli <[email protected]>
  0 siblings, 1 reply; 24+ messages in thread

From: Sandro Santilli @ 2022-11-09 17:53 UTC (permalink / raw)
  To: Regina Obe <[email protected]>; pgsql-hackers; 'Laurenz Albe' <[email protected]>; 'Tom Lane' <[email protected]>

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;


^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* Re: [PATCH] Support % wildcard in extension upgrade filenames
@ 2022-11-10 01:10  LEO HSU <[email protected]>
  parent: Sandro Santilli <[email protected]>
  0 siblings, 1 reply; 24+ messages in thread

From: LEO HSU @ 2022-11-10 01:10 UTC (permalink / raw)
  To: [email protected]; +Cc: Sandro Santilli <[email protected]>

The following review has been posted through the commitfest application:
make installcheck-world:  tested, failed
Implements feature:       tested, passed
Spec compliant:           tested, passed
Documentation:            tested, passed

PURPOSE:  

This feature is intended to allow projects with many micro versions that do the same thing, be able to ship much fewer files by specifying less granular stopping.
One of the projects that will benefit from this is the PostGIS project.  Currently we ship over 300 extension scripts per version which are currently just symlinks to the same file.
Part of the reason for that is our changes are dependent on both PostgreSQL version and PostGIS version, so a simple upgrade that only considers say PostGIS 3.2.0--3.3.1 is not sufficient.
Also much of the time, our function definitions don't change in micros, but yet we need to ship them to allow users to properly upgrade.

This feature will allow us to get rid of all these symlinks or 0-byte files.

I've tested this feature against the PostgreSQL master branch on mingw64 gcc 8.1.

BASIC FEATURES

1) As stated, this feature only works if in the .control file, has a line:

wildcard_upgrades = true

2) It does nothing different if the line is missing or set to false.

3) If there is such a line and there is no other path that takes an extension from it's current version to the requested version, then it will use the wildcard script file.

TESTING

AUTOMATED TESTS
I have confirmed tests are in place.
However the tests do not run when I do 
make check (from the root source folder)

or when I do

make installcheck-world


To run these tests, I had to

cd src/test/modules/test_extensions
make check

and see the output showing:

============== running regression test queries        ==============
test test_extensions              ... ok          186 ms
test test_extdepend               ... ok           97 ms


I confirmed the tests are in test_extensions, which has always existed.
So I assume why it's not being run in installcheck-world is something off with my configuration 
or it's intentionally not run.

MANUAL TESTS
 I also ran some adhoc tests of my own to confirm the behavior. and checking with
SET client_min_messages='DEBUG1';

To confirm that the wildcard script I have only gets called when there is no other path that will take the user to the new version.
I created my own extension
wildtest


1) I confirmed  It only understands % as a complete wildcard for a version number


So this is legal
wildtest--%--2.0.sql


This does nothing
wildtest--2%--2.0.sql


2) I confirmed that if you have a path such as


wildtest--2.0--2.2.sql
wildtest--%--2.2.sql


then the exact match trumps the wildcard. In the above case if I am on 2.0
and going to 2.2, the wildtest--2.0--2.2.sql script is used instead of the
wildtest--% one.


3) It is not possible to downgrade with the wildcard.  For example I had
paths
wildtest--%--2.1.sql  


and I was unable to go from a version 2.2 down to a version 2.1.

DOCUMENTATION

I built the html docs and confirmed that in the section of the docs where it defines valid properties of extension files it now includes this line:

wildcard_upgrades (boolean)

    This parameter, if set to true (which is not the default), allows ALTER EXTENSION to consider a wildcard character % as matching any version of the extension. Such wildcard match will only be used when no perfect match is found for a version.

The new status of this patch is: Ready for Committer


^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* Re: [PATCH] Support % wildcard in extension upgrade filenames
@ 2022-11-10 15:23  Regina Obe <[email protected]>
  parent: LEO HSU <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: Regina Obe @ 2022-11-10 15:23 UTC (permalink / raw)
  To: [email protected]; +Cc: Sandro Santilli <[email protected]>

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

^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* Re: [PATCH] Support % wildcard in extension upgrade filenames
@ 2022-11-11 15:59  Christoph Berg <[email protected]>
  parent: Tom Lane <[email protected]>
  1 sibling, 0 replies; 24+ messages in thread

From: Christoph Berg @ 2022-11-11 15:59 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Laurenz Albe <[email protected]>; Regina Obe <[email protected]>; [email protected]; pgsql-hackers

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





^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* Re: [PATCH] Support % wildcard in extension upgrade filenames
@ 2022-11-11 16:07  Christoph Berg <[email protected]>
  parent: Sandro Santilli <[email protected]>
  1 sibling, 1 reply; 24+ messages in thread

From: Christoph Berg @ 2022-11-11 16:07 UTC (permalink / raw)
  To: [email protected]; pgsql-hackers; +Cc: Sandro Santilli <[email protected]>; Laurenz Albe <[email protected]>; Regina Obe <[email protected]>; Tom Lane <[email protected]>

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





^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* RE: [PATCH] Support % wildcard in extension upgrade filenames
@ 2022-11-14 04:46  Regina Obe <[email protected]>
  parent: Christoph Berg <[email protected]>
  0 siblings, 1 reply; 24+ messages in thread

From: Regina Obe @ 2022-11-14 04:46 UTC (permalink / raw)
  To: 'Christoph Berg' <[email protected]>; [email protected]; pgsql-hackers; +Cc: 'Sandro Santilli' <[email protected]>; 'Laurenz Albe' <[email protected]>; 'Tom Lane' <[email protected]>; 'Paul Ramsey' <[email protected]>

> 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








^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* Re: [PATCH] Support % wildcard in extension upgrade filenames
@ 2022-11-16 23:09  'Sandro Santilli' <[email protected]>
  parent: Regina Obe <[email protected]>
  0 siblings, 2 replies; 24+ messages in thread

From: 'Sandro Santilli' @ 2022-11-16 23:09 UTC (permalink / raw)
  To: Regina Obe <[email protected]>; +Cc: 'Christoph Berg' <[email protected]>; [email protected]; pgsql-hackers; 'Laurenz Albe' <[email protected]>; 'Tom Lane' <[email protected]>; 'Paul Ramsey' <[email protected]>

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;


^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* RE: [PATCH] Support % wildcard in extension upgrade filenames
@ 2022-11-16 23:16  Regina Obe <[email protected]>
  parent: 'Sandro Santilli' <[email protected]>
  1 sibling, 0 replies; 24+ messages in thread

From: Regina Obe @ 2022-11-16 23:16 UTC (permalink / raw)
  To: [email protected]; +Cc: 'Christoph Berg' <[email protected]>; [email protected]; pgsql-hackers; 'Laurenz Albe' <[email protected]>; 'Tom Lane' <[email protected]>; 'Paul Ramsey' <[email protected]>

> 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






^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* Re: [PATCH] Support % wildcard in extension upgrade filenames
@ 2022-11-16 23:25  Justin Pryzby <[email protected]>
  parent: 'Sandro Santilli' <[email protected]>
  1 sibling, 1 reply; 24+ messages in thread

From: Justin Pryzby @ 2022-11-16 23:25 UTC (permalink / raw)
  To: Sandro Santilli <[email protected]>; +Cc: Regina Obe <[email protected]>; Christoph Berg <[email protected]>; pgsql-hackers; Laurenz Albe <[email protected]>; Tom Lane <[email protected]>; Paul Ramsey <[email protected]>; [email protected]

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





^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* Re: [PATCH] Support % wildcard in extension upgrade filenames
@ 2022-11-17 09:57  Sandro Santilli <[email protected]>
  parent: Justin Pryzby <[email protected]>
  0 siblings, 1 reply; 24+ messages in thread

From: Sandro Santilli @ 2022-11-17 09:57 UTC (permalink / raw)
  To: Justin Pryzby <[email protected]>; +Cc: Regina Obe <[email protected]>; Christoph Berg <[email protected]>; pgsql-hackers; Laurenz Albe <[email protected]>; Tom Lane <[email protected]>; Paul Ramsey <[email protected]>; [email protected]

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;


^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* Re: [PATCH] Support % wildcard in extension upgrade filenames
@ 2022-11-22 19:03  Regina Obe <[email protected]>
  parent: Sandro Santilli <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: Regina Obe @ 2022-11-22 19:03 UTC (permalink / raw)
  To: [email protected]; +Cc: Sandro Santilli <[email protected]>

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't personally test this.

3) The documentation has been updated to no longer include wildcard_upgrades as a variable for control file.

and has this text now describing it's use:

 The literal value % can be used as the old_version component in an extension update script for it to match any version. Such wildcard update scripts will only be used when no explicit path is found from old to target version.

Given that a suitable update script is available, the command ALTER EXTENSION UPDATE will update an installed extension to the specified new version. The update script is run in the same environment that CREATE EXTENSION provides for installation scripts: in particular, search_path is set up in the same way, and any new objects created by the script are automatically added to the extension. Also, if the script chooses to drop extension member objects, they are automatically dissociated from the extension. 

I built the html docs but ran into a lot of warnings I don't recall getting last time.  I think this is just my doc local setup is a bit messed up or something else changed in the doc setup.

My main gripe with this patch is Sandro did not increment the version number of it, so it's the same name as an old patch he had submitted before.

^ permalink  raw  reply  [nested|flat] 24+ messages in thread


end of thread, other threads:[~2022-11-22 19:03 UTC | newest]

Thread overview: 24+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-03-08 06:32 [PATCH 02/10] Run 011_crash_recovery.pl with wal_level=minimal Kyotaro Horiguchi <[email protected]>
2022-02-11 17:59 [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2022-05-27 21:37 ` RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2022-05-28 14:50   ` Re: [PATCH] Support % wildcard in extension upgrade filenames Laurenz Albe <[email protected]>
2022-05-28 15:26     ` Re: [PATCH] Support % wildcard in extension upgrade filenames Daniel Gustafsson <[email protected]>
2022-06-04 09:21       ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2022-05-28 15:37     ` Re: [PATCH] Support % wildcard in extension upgrade filenames Tom Lane <[email protected]>
2022-06-04 09:26       ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2022-11-11 15:59       ` Re: [PATCH] Support % wildcard in extension upgrade filenames Christoph Berg <[email protected]>
2022-06-04 09:20     ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2022-09-14 22:01       ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2022-09-14 22:02         ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2022-10-31 05:55           ` RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2022-11-04 17:31             ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2022-11-09 17:53               ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2022-11-10 01:10                 ` Re: [PATCH] Support % wildcard in extension upgrade filenames LEO HSU <[email protected]>
2022-11-10 15:23                   ` Re: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2022-11-11 16:07         ` Re: [PATCH] Support % wildcard in extension upgrade filenames Christoph Berg <[email protected]>
2022-11-14 04:46           ` RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2022-11-16 23:09             ` Re: [PATCH] Support % wildcard in extension upgrade filenames 'Sandro Santilli' <[email protected]>
2022-11-16 23:16               ` RE: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[email protected]>
2022-11-16 23:25               ` Re: [PATCH] Support % wildcard in extension upgrade filenames Justin Pryzby <[email protected]>
2022-11-17 09:57                 ` Re: [PATCH] Support % wildcard in extension upgrade filenames Sandro Santilli <[email protected]>
2022-11-22 19:03                   ` Re: [PATCH] Support % wildcard in extension upgrade filenames Regina Obe <[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