I wrote:
> As a stopgap to turn the farm green again, I am going to revert
> 75d22069e as well as my followup patches. If we don't want to
> give up on that idea altogether, we have to find some way to
> suppress the chatter from parallel workers. I wonder whether
> it would be appropriate to go further than we have, and actively
> delete placeholders that turn out to be within an extension's
> reserved namespace. The core issue here is that workers don't
> necessarily set GUCs and load extensions in the same order that
> their parent did, so if we leave any invalid placeholders behind
> after reserving an extension's prefix, we're risking issues
> during worker start.
Here's a delta patch (meant to be applied after reverting cab5b9ab2)
that does things like that. It fixes the parallel-mode problem ...
so do we want to tighten things up this much?
regards, tom lane
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index d239004347..fec535283c 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -9362,8 +9362,8 @@ 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.
+ * This deletes 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.
*/
@@ -9374,7 +9374,12 @@ MarkGUCPrefixReserved(const char *className)
int i;
MemoryContext oldcontext;
- /* Check for existing placeholders. */
+ /*
+ * Check for existing placeholders. We must actually remove invalid
+ * placeholders, else future parallel worker startups will fail. (We
+ * don't bother trying to free associated memory, since this shouldn't
+ * happen often.)
+ */
for (i = 0; i < num_guc_variables; i++)
{
struct config_generic *var = guc_variables[i];
@@ -9384,9 +9389,14 @@ MarkGUCPrefixReserved(const char *className)
var->name[classLen] == GUC_QUALIFIER_SEPARATOR)
{
ereport(WARNING,
- (errcode(ERRCODE_UNDEFINED_OBJECT),
- errmsg("unrecognized configuration parameter \"%s\"",
- var->name)));
+ (errcode(ERRCODE_INVALID_NAME),
+ errmsg("invalid configuration parameter name \"%s\", removing it",
+ var->name),
+ errdetail("\"%s\" is now a reserved prefix.",
+ className)));
+ num_guc_variables--;
+ memmove(&guc_variables[i], &guc_variables[i + 1],
+ (num_guc_variables - i) * sizeof(struct config_generic *));
}
}
diff --git a/src/test/regress/expected/guc.out b/src/test/regress/expected/guc.out
index 5ad7477f61..60998ee644 100644
--- a/src/test/regress/expected/guc.out
+++ b/src/test/regress/expected/guc.out
@@ -550,21 +550,15 @@ 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
+SET plpgsql.extra_foo_warnings = true; -- allowed if plpgsql is not loaded yet
+LOAD 'plpgsql'; -- this will throw a warning and delete the variable
+WARNING: invalid configuration parameter name "plpgsql.extra_foo_warnings", removing it
+DETAIL: "plpgsql" is now a reserved prefix.
+SET plpgsql.extra_foo_warnings = true; -- 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
index f97f4e4488..63ab2d9be6 100644
--- a/src/test/regress/sql/guc.sql
+++ b/src/test/regress/sql/guc.sql
@@ -165,12 +165,10 @@ 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
+SET plpgsql.extra_foo_warnings = true; -- allowed if plpgsql is not loaded yet
+LOAD 'plpgsql'; -- this will throw a warning and delete the variable
+SET plpgsql.extra_foo_warnings = true; -- 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
