pg.ddx.io  pgsql-bugs@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Laurenz Albe <laurenz.albe@cybertec.at>
To: Nathan Bossart <nathandbossart@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: mert@futo.org
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #18964: `ALTER DATABASE ... RESET ...` fails to reset extension parameters that no longer exist
Date: Tue, 22 Jul 2025 04:27:24 +0200
Message-ID: <803ddef427b9016be04af9320925e9570d3fcc06.camel@cybertec.at> (raw)
In-Reply-To: <aFwebUErRdEKZy6i@nathan>
References: <18964-ba09dea8c98fccd6@postgresql.org>
	<aFW8R6Qlgtbp7TOk@nathan>
	<2474668.1750451081@sss.pgh.pa.us>
	<aFXQmZ2sveFAwdjY@nathan>
	<aFXlRTxMxVyMjfjq@nathan>
	<aFwebUErRdEKZy6i@nathan>

On Wed, 2025-06-25 at 11:06 -0500, Nathan Bossart wrote:
> Here is a tidied-up version of the patch.  It's still quite fragile, but
> AFAICT to do any better we'd need to return more context from
> find_option(), and I don't think we can change that one on the
> back-branches since it's exported.  I assume we don't want find_option() to
> create a placeholder in this case, either.  I'll continue to look around
> for a better solution...

I tested your patch, and it works as expected for

  ALTER DATABASE ... RESET
  ALTER ROLE ... RESET
  ALTER ROLE ... IN DATABASE ... RESET

There is still one piece missing in my opinion:

  ALTER SYSTEM RESET testext.swap_limit;
  ERROR:  invalid configuration parameter name "testext.swap_limit"
  DETAIL:  "testext" is a reserved prefix.

I think that this case should work like the others.

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.
Perhaps a sequence like this:

  DO $$BEGIN EXECUTE format(
    'ALTER DATABASE %I SET pg_trgm.somevar = 42',
    current_catalog); END$$;

  LOAD 'pg_trgm';
  WARNING:  invalid configuration parameter name "pg_trgm.somevar", removing it
  DETAIL:  "pg_trgm" is now a reserved prefix.

  DO $$BEGIN EXECUTE format(
    'ALTER DATABASE %I RESET pg_trgm.somevar',
    current_catalog); END$$;

  \drds

> If we wanted to open up RESET in this case to everyone with suitable
> privileges on the target DB and/or role (as proposed by Tom upthread [0]),
> I think we'd just need to replace the "return false;" under find_option()
> to "return reset_custom;".
> 
> [0] https://postgr.es/m/2474668.1750451081%40sss.pgh.pa.us

I have no strong opinion about that.
I'd say it is good enough to allow superusers to reset such parameters.

Yours,
Laurenz Albe





view thread (14+ messages)  latest in thread

Message-ID: <803ddef427b9016be04af9320925e9570d3fcc06.camel@cybertec.at>
Permalink:  ../803ddef427b9016be04af9320925e9570d3fcc06.camel@cybertec.at/
Also on:    postgresql.org/message-id/803ddef427b9016be04af9320925e9570d3fcc06.camel@cybertec.at

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-bugs@postgresql.org
  Cc: laurenz.albe@cybertec.at, nathandbossart@gmail.com, tgl@sss.pgh.pa.us, mert@futo.org, pgsql-bugs@lists.postgresql.org
  Subject: Re: BUG #18964: `ALTER DATABASE ... RESET ...` fails to reset extension parameters that no longer exist
  In-Reply-To: <803ddef427b9016be04af9320925e9570d3fcc06.camel@cybertec.at>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox