Revert changes about warnings/errors for placeholders.

tglsfdc · tglsfdc · commit cab5b9ab2c06 · 2021-12-27T16:01:10.000-05:00
Revert commits 5609cc0, 2ed8a8c, and 75d2206 until we have a less broken idea of how this should work in parallel workers. Per buildfarm. Discussion: https://postgr.es/m/1640909.1640638123@sss.pgh.pa.us
diff --git a/contrib/auth_delay/auth_delay.c b/contrib/auth_delay/auth_delay.c
@@ -68,7 +68,7 @@ _PG_init(void)
 							NULL,
 							NULL);
 
-	MarkGUCPrefixReserved("auth_delay");
+	EmitWarningsOnPlaceholders("auth_delay");
 
 	/* Install Hooks */
 	original_client_auth_hook = ClientAuthentication_hook;
diff --git a/contrib/auto_explain/auto_explain.c b/contrib/auto_explain/auto_explain.c
@@ -231,7 +231,7 @@ _PG_init(void)
 							 NULL,
 							 NULL);
 
-	MarkGUCPrefixReserved("auto_explain");
+	EmitWarningsOnPlaceholders("auto_explain");
 
 	/* Install hooks. */
 	prev_ExecutorStart = ExecutorStart_hook;
diff --git a/contrib/pg_prewarm/autoprewarm.c b/contrib/pg_prewarm/autoprewarm.c
@@ -136,7 +136,7 @@ _PG_init(void)
 							 NULL,
 							 NULL);
 
-	MarkGUCPrefixReserved("pg_prewarm");
+	EmitWarningsOnPlaceholders("pg_prewarm");
 
 	RequestAddinShmemSpace(MAXALIGN(sizeof(AutoPrewarmSharedState)));
 
diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -437,7 +437,7 @@ _PG_init(void)
 							 NULL,
 							 NULL);
 
-	MarkGUCPrefixReserved("pg_stat_statements");
+	EmitWarningsOnPlaceholders("pg_stat_statements");
 
 	/*
 	 * Request additional shared resources.  (These are no-ops if we're not in
diff --git a/contrib/pg_trgm/trgm_op.c b/contrib/pg_trgm/trgm_op.c
@@ -101,7 +101,7 @@ _PG_init(void)
 							 NULL,
 							 NULL);
 
-	MarkGUCPrefixReserved("pg_trgm");
+	EmitWarningsOnPlaceholders("pg_trgm");
 }
 
 /*
diff --git a/contrib/postgres_fdw/option.c b/contrib/postgres_fdw/option.c
@@ -532,5 +532,5 @@ _PG_init(void)
 							   NULL,
 							   NULL);
 
-	MarkGUCPrefixReserved("postgres_fdw");
+	EmitWarningsOnPlaceholders("postgres_fdw");
 }
diff --git a/contrib/sepgsql/hooks.c b/contrib/sepgsql/hooks.c
@@ -455,7 +455,7 @@ _PG_init(void)
 							 NULL,
 							 NULL);
 
-	MarkGUCPrefixReserved("sepgsql");
+	EmitWarningsOnPlaceholders("sepgsql");
 
 	/* Initialize userspace access vector cache */
 	sepgsql_avc_init();
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
@@ -148,8 +148,6 @@ extern bool optimize_bounded_sort;
 
 static int	GUC_check_errcode_value;
 
-static List *reserved_class_prefix = NIL;
-
 /* global variables for check hook support */
 char	   *GUC_check_errmsg_string;
 char	   *GUC_check_errdetail_string;
@@ -5569,44 +5567,18 @@ find_option(const char *name, bool create_placeholders, bool skip_errors,
 		 * doesn't contain a separator, don't assume that it was meant to be a
 		 * placeholder.
 		 */
-		const char *sep = strchr(name, GUC_QUALIFIER_SEPARATOR);
-
-		if (sep != NULL)
+		if (strchr(name, GUC_QUALIFIER_SEPARATOR) != NULL)
 		{
-			size_t		classLen = sep - name;
-			ListCell   *lc;
-
-			/* The name must be syntactically acceptable ... */
-			if (!valid_custom_variable_name(name))
-			{
-				if (!skip_errors)
-					ereport(elevel,
-							(errcode(ERRCODE_INVALID_NAME),
-							 errmsg("invalid configuration parameter name \"%s\"",
-									name),
-							 errdetail("Custom parameter names must be two or more simple identifiers separated by dots.")));
-				return NULL;
-			}
-			/* ... and it must not match any previously-reserved prefix */
-			foreach(lc, reserved_class_prefix)
-			{
-				const char *rcprefix = lfirst(lc);
-
-				if (strlen(rcprefix) == classLen &&
-					strncmp(name, rcprefix, classLen) == 0)
-				{
-					if (!skip_errors)
-						ereport(elevel,
-								(errcode(ERRCODE_INVALID_NAME),
-								 errmsg("invalid configuration parameter name \"%s\"",
-										name),
-								 errdetail("\"%s\" is a reserved prefix.",
-										   rcprefix)));
-					return NULL;
-				}
-			}
-			/* OK, create it */
-			return add_placeholder_variable(name, elevel);
+			if (valid_custom_variable_name(name))
+				return add_placeholder_variable(name, elevel);
+			/* A special error message seems desirable here */
+			if (!skip_errors)
+				ereport(elevel,
+						(errcode(ERRCODE_INVALID_NAME),
+						 errmsg("invalid configuration parameter name \"%s\"",
+								name),
+						 errdetail("Custom parameter names must be two or more simple identifiers separated by dots.")));
+			return NULL;
 		}
 	}
 
@@ -9360,21 +9332,15 @@ DefineCustomEnumVariable(const char *name,
 }
 
 /*
- * Mark the given GUC prefix as "reserved".
- *
- * This prints warnings if there are any existing placeholders matching
- * the prefix, and then prevents new ones from being created.
  * Extensions should call this after they've defined all of their custom
  * GUCs, to help catch misspelled config-file entries.
  */
 void
-MarkGUCPrefixReserved(const char *className)
+EmitWarningsOnPlaceholders(const char *className)
 {
 	int			classLen = strlen(className);
 	int			i;
-	MemoryContext oldcontext;
 
-	/* Check for existing placeholders. */
 	for (i = 0; i < num_guc_variables; i++)
 	{
 		struct config_generic *var = guc_variables[i];
@@ -9389,11 +9355,6 @@ MarkGUCPrefixReserved(const char *className)
 							var->name)));
 		}
 	}
-
-	/* And remember the name so we can prevent future mistakes. */
-	oldcontext = MemoryContextSwitchTo(TopMemoryContext);
-	reserved_class_prefix = lappend(reserved_class_prefix, pstrdup(className));
-	MemoryContextSwitchTo(oldcontext);
 }
 
 
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
@@ -354,10 +354,7 @@ extern void DefineCustomEnumVariable(const char *name,
 									 GucEnumAssignHook assign_hook,
 									 GucShowHook show_hook);
 
-extern void MarkGUCPrefixReserved(const char *className);
-
-/* old name for MarkGUCPrefixReserved, for backwards compatibility: */
-#define EmitWarningsOnPlaceholders(className) MarkGUCPrefixReserved(className)
+extern void EmitWarningsOnPlaceholders(const char *className);
 
 extern const char *GetConfigOption(const char *name, bool missing_ok,
 								   bool restrict_privileged);
diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c
@@ -453,7 +453,7 @@ _PG_init(void)
 							   PGC_SUSET, 0,
 							   NULL, NULL, NULL);
 
-	MarkGUCPrefixReserved("plperl");
+	EmitWarningsOnPlaceholders("plperl");
 
 	/*
 	 * Create hash tables.
diff --git a/src/pl/plpgsql/src/pl_handler.c b/src/pl/plpgsql/src/pl_handler.c
@@ -197,7 +197,7 @@ _PG_init(void)
 							   plpgsql_extra_errors_assign_hook,
 							   NULL);
 
-	MarkGUCPrefixReserved("plpgsql");
+	EmitWarningsOnPlaceholders("plpgsql");
 
 	plpgsql_HashTableInit();
 	RegisterXactCallback(plpgsql_xact_cb, NULL);
diff --git a/src/pl/tcl/pltcl.c b/src/pl/tcl/pltcl.c
@@ -474,8 +474,8 @@ _PG_init(void)
 							   PGC_SUSET, 0,
 							   NULL, NULL, NULL);
 
-	MarkGUCPrefixReserved("pltcl");
-	MarkGUCPrefixReserved("pltclu");
+	EmitWarningsOnPlaceholders("pltcl");
+	EmitWarningsOnPlaceholders("pltclu");
 
 	pltcl_pm_init_done = true;
 }
diff --git a/src/test/modules/delay_execution/delay_execution.c b/src/test/modules/delay_execution/delay_execution.c
@@ -91,7 +91,7 @@ _PG_init(void)
 							NULL,
 							NULL);
 
-	MarkGUCPrefixReserved("delay_execution");
+	EmitWarningsOnPlaceholders("delay_execution");
 
 	/* Install our hook */
 	prev_planner_hook = planner_hook;
diff --git a/src/test/modules/ssl_passphrase_callback/ssl_passphrase_func.c b/src/test/modules/ssl_passphrase_callback/ssl_passphrase_func.c
@@ -49,7 +49,7 @@ _PG_init(void)
 							   NULL,
 							   NULL);
 
-	MarkGUCPrefixReserved("ssl_passphrase");
+	EmitWarningsOnPlaceholders("ssl_passphrase");
 
 	if (ssl_passphrase)
 		openssl_tls_init_hook = set_rot13;
diff --git a/src/test/modules/worker_spi/worker_spi.c b/src/test/modules/worker_spi/worker_spi.c
@@ -322,7 +322,7 @@ _PG_init(void)
 							   0,
 							   NULL, NULL, NULL);
 
-	MarkGUCPrefixReserved("worker_spi");
+	EmitWarningsOnPlaceholders("worker_spi");
 
 	/* set up common data for all our workers */
 	memset(&worker, 0, sizeof(worker));
diff --git a/src/test/regress/expected/guc.out b/src/test/regress/expected/guc.out
@@ -548,23 +548,6 @@ ERROR:  invalid configuration parameter name "special.weird name"
 DETAIL:  Custom parameter names must be two or more simple identifiers separated by dots.
 SHOW special."weird name";
 ERROR:  unrecognized configuration parameter "special.weird name"
--- Check what happens when you try to set a "custom" GUC within the
--- namespace of an extension.
-SET plpgsql.bogus_setting = 42;  -- allowed if plpgsql is not loaded yet
-LOAD 'plpgsql';  -- this will now warn about it
-WARNING:  unrecognized configuration parameter "plpgsql.bogus_setting"
-SET plpgsql.extra_foo_warnings = false;  -- but now, it's an error
-ERROR:  invalid configuration parameter name "plpgsql.extra_foo_warnings"
-DETAIL:  "plpgsql" is a reserved prefix.
-SHOW plpgsql.extra_foo_warnings;
-ERROR:  unrecognized configuration parameter "plpgsql.extra_foo_warnings"
-SET plpgsql.bogus_setting = 43;  -- you can still use the pre-existing variable
-SHOW plpgsql.bogus_setting;
- plpgsql.bogus_setting 
------------------------
- 43
-(1 row)
-
 --
 -- Test DISCARD TEMP
 --
diff --git a/src/test/regress/sql/guc.sql b/src/test/regress/sql/guc.sql
@@ -163,15 +163,6 @@ SHOW custom."bad-guc";
 SET special."weird name" = 'foo';  -- could be allowed, but we choose not to
 SHOW special."weird name";
 
--- Check what happens when you try to set a "custom" GUC within the
--- namespace of an extension.
-SET plpgsql.bogus_setting = 42;  -- allowed if plpgsql is not loaded yet
-LOAD 'plpgsql';  -- this will now warn about it
-SET plpgsql.extra_foo_warnings = false;  -- but now, it's an error
-SHOW plpgsql.extra_foo_warnings;
-SET plpgsql.bogus_setting = 43;  -- you can still use the pre-existing variable
-SHOW plpgsql.bogus_setting;
-
 --
 -- Test DISCARD TEMP
 --

Original file line number	Diff line number	Diff line change
`@@ -101,7 +101,7 @@ _PG_init(void)`
`101`	`101`	`NULL,`
`102`	`102`	`NULL);`
`103`	`103`
`104`		`- MarkGUCPrefixReserved("pg_trgm");`
	`104`	`+ EmitWarningsOnPlaceholders("pg_trgm");`
`105`	`105`	`}`
`106`	`106`
`107`	`107`	`/*`
Original file line number	Diff line number	Diff line change
`@@ -532,5 +532,5 @@ _PG_init(void)`
`532`	`532`	`NULL,`
`533`	`533`	`NULL);`
`534`	`534`
`535`		`- MarkGUCPrefixReserved("postgres_fdw");`
	`535`	`+ EmitWarningsOnPlaceholders("postgres_fdw");`
`536`	`536`	`}`