[PATCH v3 1/1] Allow resetting unknown custom GUCs with reserved prefixes.

pg.ddx.io  pgsql-bugs@postgresql.org mailing list archive  
help / color / mirror / Atom feed

From: Nathan Bossart <nathan@postgresql.org>
Subject: [PATCH v3 1/1] Allow resetting unknown custom GUCs with reserved prefixes.
Date: Wed, 25 Jun 2025 10:54:33 -0500

---
 contrib/auto_explain/Makefile                 |  2 ++
 contrib/auto_explain/expected/alter_reset.out | 19 +++++++++++++
 contrib/auto_explain/meson.build              |  5 ++++
 contrib/auto_explain/sql/alter_reset.sql      | 22 +++++++++++++++
 src/backend/utils/misc/guc.c                  | 28 +++++++++++++++----
 5 files changed, 71 insertions(+), 5 deletions(-)
 create mode 100644 contrib/auto_explain/expected/alter_reset.out
 create mode 100644 contrib/auto_explain/sql/alter_reset.sql

diff --git a/contrib/auto_explain/Makefile b/contrib/auto_explain/Makefile
index efd127d3cae..94ab28e7c06 100644
--- a/contrib/auto_explain/Makefile
+++ b/contrib/auto_explain/Makefile
@@ -6,6 +6,8 @@ OBJS = \
 	auto_explain.o
 PGFILEDESC = "auto_explain - logging facility for execution plans"
 
+REGRESS = alter_reset
+
 TAP_TESTS = 1
 
 ifdef USE_PGXS
diff --git a/contrib/auto_explain/expected/alter_reset.out b/contrib/auto_explain/expected/alter_reset.out
new file mode 100644
index 00000000000..6b1db8703e5
--- /dev/null
+++ b/contrib/auto_explain/expected/alter_reset.out
@@ -0,0 +1,19 @@
+--
+-- This test verifies that superusers can reset unknown custom GUCs with
+-- reserved prefixes.  There's nothing specific to auto_explain; this is just a
+-- convenient place to put this test.
+--
+SELECT current_database() AS datname \gset
+CREATE ROLE regress_ae_role;
+ALTER DATABASE :datname SET auto_explain.bogus = 1;
+ALTER ROLE regress_ae_role SET auto_explain.bogus = 1;
+ALTER ROLE regress_ae_role IN DATABASE :datname SET auto_explain.bogus = 1;
+ALTER SYSTEM SET auto_explain.bogus = 1;
+LOAD 'auto_explain';
+WARNING:  invalid configuration parameter name "auto_explain.bogus", removing it
+DETAIL:  "auto_explain" is now a reserved prefix.
+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;
+ALTER SYSTEM RESET auto_explain.bogus;
+DROP ROLE regress_ae_role;
diff --git a/contrib/auto_explain/meson.build b/contrib/auto_explain/meson.build
index 92dc9df6f7c..a9b45cc235f 100644
--- a/contrib/auto_explain/meson.build
+++ b/contrib/auto_explain/meson.build
@@ -20,6 +20,11 @@ tests += {
   'name': 'auto_explain',
   'sd': meson.current_source_dir(),
   'bd': meson.current_build_dir(),
+  'regress': {
+    'sql': [
+      'alter_reset',
+    ],
+  },
   'tap': {
     'tests': [
       't/001_auto_explain.pl',
diff --git a/contrib/auto_explain/sql/alter_reset.sql b/contrib/auto_explain/sql/alter_reset.sql
new file mode 100644
index 00000000000..06577481228
--- /dev/null
+++ b/contrib/auto_explain/sql/alter_reset.sql
@@ -0,0 +1,22 @@
+--
+-- This test verifies that superusers can reset unknown custom GUCs with
+-- reserved prefixes.  There's nothing specific to auto_explain; this is just a
+-- convenient place to put this test.
+--
+
+SELECT current_database() AS datname \gset
+CREATE ROLE regress_ae_role;
+
+ALTER DATABASE :datname SET auto_explain.bogus = 1;
+ALTER ROLE regress_ae_role SET auto_explain.bogus = 1;
+ALTER ROLE regress_ae_role IN DATABASE :datname SET auto_explain.bogus = 1;
+ALTER SYSTEM SET auto_explain.bogus = 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;
+ALTER SYSTEM RESET auto_explain.bogus;
+
+DROP ROLE regress_ae_role;
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 667df448732..7f2463f7d9f 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -4722,8 +4722,13 @@ AlterSystemSetConfigFile(AlterSystemStmt *altersysstmt)
 			 * the config file cannot cause postmaster start to fail, so we
 			 * don't have to be too tense about possibly installing a bad
 			 * value.)
+			 *
+			 * As an exception, we skip this check if this is a RESET command
+			 * for an unknown custom GUC, else there'd be no way for users to
+			 * remove such settings with reserved prefixes.
 			 */
-			(void) assignable_custom_variable_name(name, false, ERROR);
+			if (value || !valid_custom_variable_name(name))
+				(void) assignable_custom_variable_name(name, false, ERROR);
 		}
 
 		/*
@@ -6711,6 +6716,7 @@ validate_option_array_item(const char *name, const char *value,
 
 {
 	struct config_generic *gconf;
+	bool		reset_custom;
 
 	/*
 	 * There are three cases to consider:
@@ -6729,16 +6735,28 @@ validate_option_array_item(const char *name, const char *value,
 	 * it's assumed to be fully validated.)
 	 *
 	 * name is not known and can't be created as a placeholder.  Throw error,
-	 * unless skipIfNoPermissions is true, in which case return false.
+	 * unless skipIfNoPermissions or reset_custom is true.  If find_option()
+	 * returns false and reset_custom is true, this is a RESET or RESET ALL
+	 * operation for an unknown custom GUC with a reserved prefix, in which
+	 * case we want to fall through to the placeholder check described in the
+	 * preceding paragraph (else there'd be no way for users to remove them).
+	 * Otherwise, if find_option() returns false, return false here, too.
 	 */
-	gconf = find_option(name, true, skipIfNoPermissions, ERROR);
-	if (!gconf)
+	reset_custom = (!value && valid_custom_variable_name(name));
+	gconf = find_option(name, true, skipIfNoPermissions || reset_custom, ERROR);
+	if (!gconf && !reset_custom)
 	{
 		/* not known, failed to make a placeholder */
 		return false;
 	}
 
-	if (gconf->flags & GUC_CUSTOM_PLACEHOLDER)
+	/*
+	 * If we reach this point and gconf is NULL, we know we're dealing with a
+	 * RESET or RESET ALL command for an unknown custom GUC with a reserved
+	 * prefix.  We treat that the same as if a placeholder was created, else
+	 * there'd be no way for users to remove them.
+	 */
+	if (!gconf || gconf->flags & GUC_CUSTOM_PLACEHOLDER)
 	{
 		/*
 		 * We cannot do any meaningful check on the value, so only permissions
-- 
2.39.5 (Apple Git-154)


--WnNNth7ekP1e9dZR--

Message-ID: <no-message-id-25348@localhost>
Permalink:  ../no-message-id-25348@localhost/
Also on:    postgresql.org/message-id/no-message-id-25348@localhost

 ·

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: nathan@postgresql.org
  Subject: Re: [PATCH v3 1/1] Allow resetting unknown custom GUCs with reserved prefixes.
  In-Reply-To: <no-message-id-25348@localhost>

* 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