diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index bff949a40bc..f345eceb5b7 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -148,6 +148,8 @@ 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; @@ -235,8 +237,6 @@ 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, @@ -5569,18 +5569,44 @@ 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. */ - if (strchr(name, GUC_QUALIFIER_SEPARATOR) != NULL) + const char *sep = strchr(name, GUC_QUALIFIER_SEPARATOR); + + if (sep != NULL) { - 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; + 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); } } @@ -8764,7 +8790,6 @@ 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: @@ -8850,8 +8875,6 @@ 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(); @@ -9345,8 +9368,9 @@ EmitWarningsOnPlaceholders(const char *className) { int classLen = strlen(className); int i; - MemoryContext oldcontext; + MemoryContext oldcontext; + /* Check for existing placeholders. */ for (i = 0; i < num_guc_variables; i++) { struct config_generic *var = guc_variables[i]; @@ -9362,48 +9386,12 @@ EmitWarningsOnPlaceholders(const char *className) } } + /* And remember the name so we can prevent future mistakes. */ 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 2433038c3ec..5ad7477f618 100644 --- a/src/test/regress/expected/guc.out +++ b/src/test/regress/expected/guc.out @@ -548,6 +548,23 @@ 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 -- @@ -813,22 +830,3 @@ 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 b57758ed273..f97f4e44884 100644 --- a/src/test/regress/sql/guc.sql +++ b/src/test/regress/sql/guc.sql @@ -163,6 +163,15 @@ 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 -- @@ -311,14 +320,3 @@ 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;