Date: Fri, 20 Jun 2025 17:48:37 -0500
From: Nathan Bossart <nathandbossart@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: mert@futo.org, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #18964: `ALTER DATABASE ... RESET ...` fails to reset
 extension parameters that no longer exist
Message-ID: <aFXlRTxMxVyMjfjq@nathan>
References: <18964-ba09dea8c98fccd6@postgresql.org>
 <aFW8R6Qlgtbp7TOk@nathan>
 <2474668.1750451081@sss.pgh.pa.us>
 <aFXQmZ2sveFAwdjY@nathan>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="PRs+Mbx6GUCOxb5m"
Content-Disposition: inline
In-Reply-To: <aFXQmZ2sveFAwdjY@nathan>
Archived-At: <https://www.postgresql.org/message-id/aFXlRTxMxVyMjfjq%40nathan>
Precedence: bulk


--PRs+Mbx6GUCOxb5m
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

Here is a fragile proof-of-concept patch that demonstrates roughly what I
had in mind.

-- 
nathan

--PRs+Mbx6GUCOxb5m
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename=fix_alter_role_reset_v1.patch

diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 667df448732..ba1bf49e83c 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -250,7 +250,7 @@ static void reapply_stacked_values(struct config_generic *variable,
 								   GucContext curscontext, GucSource cursource,
 								   Oid cursrole);
 static bool validate_option_array_item(const char *name, const char *value,
-									   bool skipIfNoPermissions);
+									   bool skipIfNoPermissions, bool isReset);
 static void write_auto_conf_file(int fd, const char *filename, ConfigVariable *head);
 static void replace_auto_config_value(ConfigVariable **head_p, ConfigVariable **tail_p,
 									  const char *name, const char *value);
@@ -6497,7 +6497,7 @@ GUCArrayAdd(ArrayType *array, const char *name, const char *value)
 	Assert(value);
 
 	/* test if the option is valid and we're allowed to set it */
-	(void) validate_option_array_item(name, value, false);
+	(void) validate_option_array_item(name, value, false, false);
 
 	/* normalize name (converts obsolete GUC names to modern spellings) */
 	record = find_option(name, false, true, WARNING);
@@ -6574,7 +6574,7 @@ GUCArrayDelete(ArrayType *array, const char *name)
 	Assert(name);
 
 	/* test if the option is valid and we're allowed to set it */
-	(void) validate_option_array_item(name, NULL, false);
+	(void) validate_option_array_item(name, NULL, false, true);
 
 	/* normalize name (converts obsolete GUC names to modern spellings) */
 	record = find_option(name, false, true, WARNING);
@@ -6672,7 +6672,7 @@ GUCArrayReset(ArrayType *array)
 		*eqsgn = '\0';
 
 		/* skip if we have permission to delete it */
-		if (validate_option_array_item(val, NULL, true))
+		if (validate_option_array_item(val, NULL, true, true))
 			continue;
 
 		/* else add it to the output array */
@@ -6707,10 +6707,18 @@ GUCArrayReset(ArrayType *array)
  */
 static bool
 validate_option_array_item(const char *name, const char *value,
-						   bool skipIfNoPermissions)
+						   bool skipIfNoPermissions, bool isReset)
 
 {
 	struct config_generic *gconf;
+	bool		skip_errors;
+
+	/*
+	 * For RESET and RESET ALL, we treat unknown custom GUCs with reserved
+	 * prefixes the same as if a placeholder is created.  That requires we
+	 * skip errors in find_option().
+	 */
+	skip_errors = (isReset && valid_custom_variable_name(name));
 
 	/*
 	 * There are three cases to consider:
@@ -6731,14 +6739,19 @@ validate_option_array_item(const char *name, const char *value,
 	 * name is not known and can't be created as a placeholder.  Throw error,
 	 * unless skipIfNoPermissions is true, in which case return false.
 	 */
-	gconf = find_option(name, true, skipIfNoPermissions, ERROR);
-	if (!gconf)
+	gconf = find_option(name, true, skipIfNoPermissions || skip_errors, ERROR);
+	if (!gconf && !skip_errors)
 	{
 		/* 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.
+	 */
+	if (!gconf || gconf->flags & GUC_CUSTOM_PLACEHOLDER)
 	{
 		/*
 		 * We cannot do any meaningful check on the value, so only permissions

--PRs+Mbx6GUCOxb5m--