Message-ID: <8902b6f8d5c19f09f4a052295b9de41d9f6970b5.camel@cybertec.at>
Subject: Re: BUG #18964: `ALTER DATABASE ... RESET ...` fails to reset
 extension parameters that no longer exist
From: Laurenz Albe <laurenz.albe@cybertec.at>
To: Nathan Bossart <nathandbossart@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, mert@futo.org,
	pgsql-bugs@lists.postgresql.org
Date: Thu, 24 Jul 2025 23:06:57 +0200
In-Reply-To: <aIJfucaqjZ3gh-xj@nathan>
References: <18964-ba09dea8c98fccd6@postgresql.org>
	 <aFW8R6Qlgtbp7TOk@nathan> <2474668.1750451081@sss.pgh.pa.us>
	 <aFXQmZ2sveFAwdjY@nathan> <aFXlRTxMxVyMjfjq@nathan>
	 <aFwebUErRdEKZy6i@nathan>
	 <803ddef427b9016be04af9320925e9570d3fcc06.camel@cybertec.at>
	 <aIJfucaqjZ3gh-xj@nathan>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.56.2 (3.56.2-1.fc42) 
MIME-Version: 1.0
Archived-At: 
 <https://www.postgresql.org/message-id/8902b6f8d5c19f09f4a052295b9de41d9f6970b5.camel%40cybertec.at>
Precedence: bulk

On Thu, 2025-07-24 at 11:30 -0500, Nathan Bossart wrote:
> On Tue, Jul 22, 2025 at 04:27:24AM +0200, Laurenz Albe wrote:
>=20
> > There is still one piece missing in my opinion:
> >=20
> >   ALTER SYSTEM RESET testext.swap_limit;
> >   ERROR:  invalid configuration parameter name "testext.swap_limit"
> >   DETAIL:  "testext" is a reserved prefix.
> >=20
> > I think that this case should work like the others.
>=20
> Good catch.  This should be fixed in v3.

Check.

> > I'd like to see regression tests for this, but I am not sure how
> > to best devise them.
> > One idea would be to stick them into the regression tests of some
> > contrib module, even though it is not the perfect place.
>=20
> Added in v3.

Great!  The regression test works fine on my Linux machine.

> --- /dev/null
> +++ b/contrib/auto_explain/sql/alter_reset.sql
> [...]
> +SELECT current_database() AS datname \gset
> +CREATE ROLE regress_ae_role;
> +
> +ALTER DATABASE :datname SET auto_explain.bogus =3D 1;
> +ALTER ROLE regress_ae_role SET auto_explain.bogus =3D 1;
> +ALTER ROLE regress_ae_role IN DATABASE :datname SET auto_explain.bogus =
=3D 1;
> +ALTER SYSTEM SET auto_explain.bogus =3D 1;
> +
> +LOAD 'auto_explain';
> +
> +ALTER DATABASE :datname RESET auto_explain.bogus;
> +ALTER ROLE regress_ae_role RESET auto_explain.bogus;
> +ALTER ROLE regress_ae_role IN DATABASE :datname RESET auto_explain.bogus=
;

That is perhaps a rediculous quibble, but shouldn't that be :"datname"?
I guess the regression test database will always have a proper name...

Anyway, I'll mark the patch as "ready for committer".

Yours,
Laurenz Albe