From 75d22069e00d638d08c04e3aba71688f3fb002ed Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Wed, 1 Dec 2021 15:07:29 +0100 Subject: [PATCH] Warning on SET of nonexisting setting with a prefix reserved by an extension An extension can already de facto reserve a GUC prefix using EmitWarningsOnPlaceholders(). But this was only checked against settings that exist at the time the extension is loaded (or the extension chooses to call this). No diagnostic is given when a SET command later uses a nonexisting setting with a custom prefix. With this change, EmitWarningsOnPlaceholders() saves the prefixes it reserves in a list, and SET checks when it finds a "placeholder" setting whether it belongs to a reserved prefix and issues a warning in that case. Add a regression test that checks the patch using the "plpgsql" registered prefix. Author: Florin Irion Discussion: https://www.postgresql.org/message-id/flat/CA+HEvJDhWuuTpGTJT9Tgbdzm4QS4EzPAwDBScWK18H2Q=FVJFw@mail.gmail.com --- src/backend/utils/misc/guc.c | 47 +++++++++++++++++++++++++++++++ src/test/regress/expected/guc.out | 19 +++++++++++++ src/test/regress/sql/guc.sql | 11 ++++++++ 3 files changed, 77 insertions(+) diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index 25ac4ca85b..a292498e2f 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -235,6 +235,8 @@ static bool check_recovery_target_lsn(char **newval, void **extra, GucSource sou static void assign_recovery_target_lsn(const char *newval, void *extra); static bool check_primary_slot_name(char **newval, void **extra, GucSource source); static bool check_default_with_oids(bool *newval, void **extra, GucSource source); +static void check_reserved_prefixes(const char *varName); +static List *reserved_class_prefix = NIL; /* Private functions in guc-file.l that need to be called from guc.c */ static ConfigVariable *ProcessConfigFileInternal(GucContext context, @@ -8755,6 +8757,7 @@ ExecSetVariableStmt(VariableSetStmt *stmt, bool isTopLevel) (superuser() ? PGC_SUSET : PGC_USERSET), PGC_S_SESSION, action, true, 0, false); + check_reserved_prefixes(stmt->name); break; case VAR_SET_MULTI: @@ -8840,6 +8843,8 @@ ExecSetVariableStmt(VariableSetStmt *stmt, bool isTopLevel) (superuser() ? PGC_SUSET : PGC_USERSET), PGC_S_SESSION, action, true, 0, false); + + check_reserved_prefixes(stmt->name); break; case VAR_RESET_ALL: ResetAllOptions(); @@ -9326,6 +9331,7 @@ EmitWarningsOnPlaceholders(const char *className) { int classLen = strlen(className); int i; + MemoryContext oldcontext; for (i = 0; i < num_guc_variables; i++) { @@ -9341,8 +9347,49 @@ EmitWarningsOnPlaceholders(const char *className) var->name))); } } + + oldcontext = MemoryContextSwitchTo(TopMemoryContext); + reserved_class_prefix = lappend(reserved_class_prefix, pstrdup(className)); + MemoryContextSwitchTo(oldcontext); } +/* + * Check a setting name against prefixes previously reserved by + * EmitWarningsOnPlaceholders() and throw a warning if matching. + */ +static void +check_reserved_prefixes(const char *varName) +{ + char *sep = strchr(varName, GUC_QUALIFIER_SEPARATOR); + + if (sep) + { + size_t classLen = sep - varName; + ListCell *lc; + + foreach(lc, reserved_class_prefix) + { + char *rcprefix = lfirst(lc); + + if (strncmp(varName, rcprefix, classLen) == 0) + { + for (int i = 0; i < num_guc_variables; i++) + { + struct config_generic *var = guc_variables[i]; + + if ((var->flags & GUC_CUSTOM_PLACEHOLDER) != 0 && + strcmp(varName, var->name) == 0) + { + ereport(WARNING, + (errcode(ERRCODE_UNDEFINED_OBJECT), + errmsg("unrecognized configuration parameter \"%s\"", var->name), + errdetail("\"%.*s\" is a reserved prefix.", (int) classLen, var->name))); + } + } + } + } + } +} /* * SHOW command diff --git a/src/test/regress/expected/guc.out b/src/test/regress/expected/guc.out index 59da91ff04..2433038c3e 100644 --- a/src/test/regress/expected/guc.out +++ b/src/test/regress/expected/guc.out @@ -813,3 +813,22 @@ set default_with_oids to f; -- Should not allow to set it to true. set default_with_oids to t; ERROR: tables declared WITH OIDS are not supported +-- test SET unrecognized parameter +SET foo = false; -- no such setting +ERROR: unrecognized configuration parameter "foo" +-- test setting a parameter with a registered prefix (plpgsql) +SET plpgsql.extra_foo_warnings = false; -- no such setting +WARNING: unrecognized configuration parameter "plpgsql.extra_foo_warnings" +DETAIL: "plpgsql" is a reserved prefix. +SHOW plpgsql.extra_foo_warnings; -- but the parameter is set + plpgsql.extra_foo_warnings +---------------------------- + false +(1 row) + +-- cleanup +RESET foo; +ERROR: unrecognized configuration parameter "foo" +RESET plpgsql.extra_foo_warnings; +WARNING: unrecognized configuration parameter "plpgsql.extra_foo_warnings" +DETAIL: "plpgsql" is a reserved prefix. diff --git a/src/test/regress/sql/guc.sql b/src/test/regress/sql/guc.sql index c39c11388d..b57758ed27 100644 --- a/src/test/regress/sql/guc.sql +++ b/src/test/regress/sql/guc.sql @@ -311,3 +311,14 @@ reset check_function_bodies; set default_with_oids to f; -- Should not allow to set it to true. set default_with_oids to t; + +-- test SET unrecognized parameter +SET foo = false; -- no such setting + +-- test setting a parameter with a registered prefix (plpgsql) +SET plpgsql.extra_foo_warnings = false; -- no such setting +SHOW plpgsql.extra_foo_warnings; -- but the parameter is set + +-- cleanup +RESET foo; +RESET plpgsql.extra_foo_warnings;