diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index d173af9b9a..3282ab4f20 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -101,7 +101,9 @@ include 'filename' value. Alternatively, you can send the signal to a single server process directly. Some parameters can only be set at server start; any changes to their entries in the configuration file will be ignored - until the server is restarted. + until the server is restarted. Invalid parameter settings in the + configuration file are likewise ignored (but logged) during + SIGHUP processing. diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 8f65ddcaa5..befb507047 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -5295,10 +5295,12 @@ readRecoveryCommandFile(void) } /* - * Since we're asking ParseConfigFp() to error out at FATAL, there's no - * need to check the return value. + * Since we're asking ParseConfigFp() to report errors as FATAL, there's + * no need to check the return value. */ - ParseConfigFp(fd, RECOVERY_COMMAND_FILE, 0, FATAL, &head, &tail); + (void) ParseConfigFp(fd, RECOVERY_COMMAND_FILE, 0, FATAL, &head, &tail); + + FreeFile(fd); for (item = head; item; item = item->next) { @@ -5504,7 +5506,6 @@ readRecoveryCommandFile(void) } FreeConfigVariables(head); - FreeFile(fd); } /* diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c index 9ffce8529b..5ebc7553bc 100644 --- a/src/backend/commands/extension.c +++ b/src/backend/commands/extension.c @@ -472,9 +472,10 @@ parse_extension_control_file(ExtensionControlFile *control, } /* - * Parse the file content, using GUC's file parsing code + * Parse the file content, using GUC's file parsing code. We need not + * check the return value since any errors will be thrown at ERROR level. */ - ParseConfigFp(file, filename, 0, ERROR, &head, &tail); + (void) ParseConfigFp(file, filename, 0, ERROR, &head, &tail); FreeFile(file); diff --git a/src/backend/utils/misc/guc-file.l b/src/backend/utils/misc/guc-file.l index 809307da8d..a7cf0378dc 100644 --- a/src/backend/utils/misc/guc-file.l +++ b/src/backend/utils/misc/guc-file.l @@ -105,6 +105,8 @@ STRING \'([^'\\\n]|\\.|\'\')*\' void ProcessConfigFile(GucContext context) { + bool error = false; + bool apply = false; int elevel; ConfigVariable *item, *head, @@ -113,24 +115,28 @@ ProcessConfigFile(GucContext context) struct config_string *cvc_struct; int i; - Assert(context == PGC_POSTMASTER || context == PGC_SIGHUP); + /* + * Config files are processed on startup (by the postmaster only) + * and on SIGHUP (by the postmaster and its children) + */ + Assert((context == PGC_POSTMASTER && !IsUnderPostmaster) || + context == PGC_SIGHUP); - if (context == PGC_SIGHUP) - { - /* - * To avoid cluttering the log, only the postmaster bleats loudly - * about problems with the config file. - */ - elevel = IsUnderPostmaster ? DEBUG2 : LOG; - } - else - elevel = ERROR; + /* + * To avoid cluttering the log, only the postmaster bleats loudly + * about problems with the config file. + */ + elevel = IsUnderPostmaster ? DEBUG2 : LOG; /* Parse the file into a list of option names and values */ head = tail = NULL; if (!ParseConfigFile(ConfigFileName, NULL, 0, elevel, &head, &tail)) + { + /* Syntax error(s) detected in the file, so bail out */ + error = true; goto cleanup_list; + } /* * We need the proposed new value of custom_variable_classes to check @@ -147,7 +153,10 @@ ProcessConfigFile(GucContext context) { cvc = guc_strdup(elevel, cvc_struct->reset_val); if (cvc == NULL) + { + error = true; goto cleanup_list; + } } else if (head != NULL && guc_name_compare(head->name, "custom_variable_classes") == 0) @@ -159,10 +168,16 @@ ProcessConfigFile(GucContext context) cvc = guc_strdup(elevel, head->value); if (cvc == NULL) + { + error = true; goto cleanup_list; + } if (!call_string_check_hook(cvc_struct, &cvc, &extra, PGC_S_FILE, elevel)) + { + error = true; goto cleanup_list; + } if (extra) free(extra); } @@ -180,60 +195,69 @@ ProcessConfigFile(GucContext context) } /* - * Check if all options are valid. As a side-effect, the GUC_IS_IN_FILE - * flag is set on each GUC variable mentioned in the list. + * Check if all the supplied option names are valid, as an additional + * quasi-syntactic check on the validity of the config file. It is + * important that the postmaster and all backends agree on the results + * of this phase, else we will have strange inconsistencies about which + * processes accept a config file update and which don't. Hence, custom + * variable names can only be checked against custom_variable_classes, + * not against any loadable modules that might (or might not) be present. + * Likewise, we don't attempt to validate the options' values here. + * + * In addition, the GUC_IS_IN_FILE flag is set on each existing GUC + * variable mentioned in the file. */ for (item = head; item; item = item->next) { - char *sep = strchr(item->name, GUC_QUALIFIER_SEPARATOR); + char *sep = strchr(item->name, GUC_QUALIFIER_SEPARATOR); + struct config_generic *record; if (sep) { - /* - * We have to consider three cases for custom variables: - * - * 1. The class name is not valid according to the (new) setting - * of custom_variable_classes. If so, reject. We don't care - * which side is at fault. - */ + /* Custom variable, so check against custom_variable_classes */ if (!is_custom_class(item->name, sep - item->name, cvc)) { ereport(elevel, (errcode(ERRCODE_UNDEFINED_OBJECT), - errmsg("unrecognized configuration parameter \"%s\"", - item->name))); - goto cleanup_list; - } - /* - * 2. There is no GUC entry. If we called set_config_option then - * it would make a placeholder, which we don't want to do yet, - * since we could still fail further down the list. Do nothing - * (assuming that making the placeholder will succeed later). - */ - if (find_option(item->name, false, elevel) == NULL) + errmsg("unrecognized configuration parameter \"%s\" in file \"%s\" line %u", + item->name, + item->filename, item->sourceline))); + error = true; continue; - /* - * 3. There is already a GUC entry (either real or placeholder) for - * the variable. In this case we should let set_config_option - * check it, since the assignment could well fail if it's a real - * entry. - */ + } } - if (!set_config_option(item->name, item->value, context, - PGC_S_FILE, GUC_ACTION_SET, false)) - goto cleanup_list; + record = find_option(item->name, false, elevel); + + if (record) + record->status |= GUC_IS_IN_FILE; + else if (!sep) + { + /* Invalid non-custom variable, so complain */ + ereport(elevel, + (errcode(ERRCODE_UNDEFINED_OBJECT), + errmsg("unrecognized configuration parameter \"%s\" in file \"%s\" line %u", + item->name, + item->filename, item->sourceline))); + error = true; + } } + /* + * If we've detected any errors so far, we don't want to risk applying + * any changes. + */ + if (error) + goto cleanup_list; + + /* Otherwise, set flag that we're beginning to apply changes */ + apply = true; + /* * Check for variables having been removed from the config file, and * revert their reset values (and perhaps also effective values) to the * boot-time defaults. If such a variable can't be changed after startup, - * just throw a warning and continue. (This is analogous to the fact that - * set_config_option only throws a warning for a new but different value. - * If we wanted to make it a hard error, we'd need an extra pass over the - * list so that we could throw the error before starting to apply - * changes.) + * report that and continue. */ for (i = 0; i < num_guc_variables; i++) { @@ -249,6 +273,7 @@ ProcessConfigFile(GucContext context) (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM), errmsg("parameter \"%s\" cannot be changed without restarting the server", gconf->name))); + error = true; continue; } @@ -267,12 +292,16 @@ ProcessConfigFile(GucContext context) } /* Now we can re-apply the wired-in default (i.e., the boot_val) */ - set_config_option(gconf->name, NULL, context, PGC_S_DEFAULT, - GUC_ACTION_SET, true); - if (context == PGC_SIGHUP) - ereport(elevel, - (errmsg("parameter \"%s\" removed from configuration file, reset to default", - gconf->name))); + if (set_config_option(gconf->name, NULL, + context, PGC_S_DEFAULT, + GUC_ACTION_SET, true) > 0) + { + /* Log the change if appropriate */ + if (context == PGC_SIGHUP) + ereport(elevel, + (errmsg("parameter \"%s\" removed from configuration file, reset to default", + gconf->name))); + } } /* @@ -298,12 +327,15 @@ ProcessConfigFile(GucContext context) PGC_BACKEND, PGC_S_DYNAMIC_DEFAULT); } - /* If we got here all the options checked out okay, so apply them. */ + /* + * Now apply the values from the config file. + */ for (item = head; item; item = item->next) { char *pre_value = NULL; + int scres; - /* In SIGHUP cases in the postmaster, report changes */ + /* In SIGHUP cases in the postmaster, we want to report changes */ if (context == PGC_SIGHUP && !IsUnderPostmaster) { const char *preval = GetConfigOption(item->name, true, false); @@ -315,12 +347,16 @@ ProcessConfigFile(GucContext context) pre_value = pstrdup(preval); } - if (set_config_option(item->name, item->value, context, - PGC_S_FILE, GUC_ACTION_SET, true)) + scres = set_config_option(item->name, item->value, + context, PGC_S_FILE, + GUC_ACTION_SET, true); + if (scres > 0) { + /* variable was updated, so remember the source location */ set_config_sourcefile(item->name, item->filename, item->sourceline); + /* and log the change if appropriate */ if (pre_value) { const char *post_value = GetConfigOption(item->name, true, false); @@ -333,6 +369,9 @@ ProcessConfigFile(GucContext context) item->name, item->value))); } } + else if (scres == 0) + error = true; + /* else no error but variable was not changed, do nothing */ if (pre_value) pfree(pre_value); @@ -345,11 +384,35 @@ ProcessConfigFile(GucContext context) FreeConfigVariables(head); if (cvc) free(cvc); + + if (error) + { + /* During postmaster startup, any error is fatal */ + if (context == PGC_POSTMASTER) + ereport(ERROR, + (errcode(ERRCODE_CONFIG_FILE_ERROR), + errmsg("configuration file \"%s\" contains errors", + ConfigFileName))); + else if (apply) + ereport(elevel, + (errcode(ERRCODE_CONFIG_FILE_ERROR), + errmsg("configuration file \"%s\" contains errors; unaffected changes were applied", + ConfigFileName))); + else + ereport(elevel, + (errcode(ERRCODE_CONFIG_FILE_ERROR), + errmsg("configuration file \"%s\" contains errors; no changes were applied", + ConfigFileName))); + } } /* - * See next function for details. This one will just work with a config_file - * name rather than an already opened File Descriptor + * Read and parse a single configuration file. This function recurses + * to handle "include" directives. + * + * See ParseConfigFp for details. This one merely adds opening the + * file rather than working from a caller-supplied file descriptor, + * and absolute-ifying the path name if necessary. */ bool ParseConfigFile(const char *config_file, const char *calling_file, @@ -423,9 +486,9 @@ ParseConfigFile(const char *config_file, const char *calling_file, * * Input parameters: * fp: file pointer from AllocateFile for the configuration file to parse - * config_file: absolute or relative path of file to read - * depth: recursion depth (used only to prevent infinite recursion) - * elevel: error logging level determined by ProcessConfigFile() + * config_file: absolute or relative path name of the configuration file + * depth: recursion depth (should be 0 in the outermost call) + * elevel: error logging level to use * Output parameters: * head_p, tail_p: head and tail of linked list of name/value pairs * @@ -435,13 +498,10 @@ ParseConfigFile(const char *config_file, const char *calling_file, * * Returns TRUE if successful, FALSE if an error occurred. The error has * already been ereport'd, it is only necessary for the caller to clean up - * its own state and release the name/value pairs list. + * its own state and release the ConfigVariable list. * * Note: if elevel >= ERROR then an error will not return control to the - * caller, and internal state such as open files will not be cleaned up. - * This case occurs only during postmaster or standalone-backend startup, - * where an error will lead to immediate process exit anyway; so there is - * no point in contorting the code so it can clean up nicely. + * caller, so there is no need to check the return value in that case. */ bool ParseConfigFp(FILE *fp, const char *config_file, int depth, int elevel, @@ -449,6 +509,7 @@ ParseConfigFp(FILE *fp, const char *config_file, int depth, int elevel, { bool OK = true; YY_BUFFER_STATE lex_buffer; + int errorcount; int token; /* @@ -458,11 +519,13 @@ ParseConfigFp(FILE *fp, const char *config_file, int depth, int elevel, yy_switch_to_buffer(lex_buffer); ConfigFileLineno = 1; + errorcount = 0; /* This loop iterates once per logical line */ while ((token = yylex())) { - char *opt_name, *opt_value; + char *opt_name = NULL; + char *opt_value = NULL; ConfigVariable *item; if (token == GUC_EOL) /* empty or comment line */ @@ -512,12 +575,7 @@ ParseConfigFp(FILE *fp, const char *config_file, int depth, int elevel, if (!ParseConfigFile(opt_value, config_file, depth + 1, elevel, head_p, tail_p)) - { - pfree(opt_name); - pfree(opt_value); OK = false; - goto cleanup_exit; - } yy_switch_to_buffer(lex_buffer); ConfigFileLineno = save_ConfigFileLineno; pfree(opt_name); @@ -576,25 +634,53 @@ ParseConfigFp(FILE *fp, const char *config_file, int depth, int elevel, /* break out of loop if read EOF, else loop for next line */ if (token == 0) break; + continue; + + parse_error: + /* release storage if we allocated any on this line */ + if (opt_name) + pfree(opt_name); + if (opt_value) + pfree(opt_value); + + /* report the error */ + if (token == GUC_EOL || token == 0) + ereport(elevel, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("syntax error in file \"%s\" line %u, near end of line", + config_file, ConfigFileLineno - 1))); + else + ereport(elevel, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("syntax error in file \"%s\" line %u, near token \"%s\"", + config_file, ConfigFileLineno, yytext))); + OK = false; + errorcount++; + + /* + * To avoid producing too much noise when fed a totally bogus file, + * give up after 100 syntax errors per file (an arbitrary number). + * Also, if we're only logging the errors at DEBUG level anyway, + * might as well give up immediately. (This prevents postmaster + * children from bloating the logs with duplicate complaints.) + */ + if (errorcount >= 100 || elevel <= DEBUG1) + { + ereport(elevel, + (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), + errmsg("too many syntax errors found, abandoning file \"%s\"", + config_file))); + break; + } + + /* resync to next end-of-line or EOF */ + while (token != GUC_EOL && token != 0) + token = yylex(); + /* break out of loop on EOF */ + if (token == 0) + break; } - /* successful completion of parsing */ - goto cleanup_exit; - - parse_error: - if (token == GUC_EOL || token == 0) - ereport(elevel, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("syntax error in file \"%s\" line %u, near end of line", - config_file, ConfigFileLineno - 1))); - else - ereport(elevel, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("syntax error in file \"%s\" line %u, near token \"%s\"", - config_file, ConfigFileLineno, yytext))); - OK = false; - -cleanup_exit: yy_delete_buffer(lex_buffer); return OK; } diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index a71729c2e7..c5b14522d5 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -5046,11 +5046,12 @@ config_enum_get_options(struct config_enum * record, const char *prefix, /* - * Sets option `name' to given value. The value should be a string - * which is going to be parsed and converted to the appropriate data - * type. The context and source parameters indicate in which context this - * function is being called so it can apply the access restrictions - * properly. + * Sets option `name' to given value. + * + * The value should be a string, which will be parsed and converted to + * the appropriate data type. The context and source parameters indicate + * in which context this function is being called, so that it can apply the + * access restrictions properly. * * If value is NULL, set the option to its default value (normally the * reset_val, but if source == PGC_S_DEFAULT we instead use the boot_val). @@ -5061,18 +5062,20 @@ config_enum_get_options(struct config_enum * record, const char *prefix, * If changeVal is false then don't really set the option but do all * the checks to see if it would work. * + * Return value: + * +1: the value is valid and was successfully applied. + * 0: the name or value is invalid (but see below). + * -1: the value was not applied because of context, priority, or changeVal. + * * If there is an error (non-existing option, invalid value) then an - * ereport(ERROR) is thrown *unless* this is called in a context where we - * don't want to ereport (currently, startup or SIGHUP config file reread). - * In that case we write a suitable error message via ereport(LOG) and - * return false. This is working around the deficiencies in the ereport - * mechanism, so don't blame me. In all other cases, the function - * returns true, including cases where the input is valid but we chose - * not to apply it because of context or source-priority considerations. + * ereport(ERROR) is thrown *unless* this is called for a source for which + * we don't want an ERROR (currently, those are defaults, the config file, + * and per-database or per-user settings). In those cases we write a + * suitable error message via ereport() and return 0. * * See also SetConfigOption for an external interface. */ -bool +int set_config_option(const char *name, const char *value, GucContext context, GucSource source, GucAction action, bool changeVal) @@ -5082,7 +5085,7 @@ set_config_option(const char *name, const char *value, bool prohibitValueChange = false; bool makeDefault; - if (context == PGC_SIGHUP || source == PGC_S_DEFAULT) + if (source == PGC_S_DEFAULT || source == PGC_S_FILE) { /* * To avoid cluttering the log, only the postmaster bleats loudly @@ -5102,18 +5105,9 @@ set_config_option(const char *name, const char *value, ereport(elevel, (errcode(ERRCODE_UNDEFINED_OBJECT), errmsg("unrecognized configuration parameter \"%s\"", name))); - return false; + return 0; } - /* - * If source is postgresql.conf, mark the found record with - * GUC_IS_IN_FILE. This is for the convenience of ProcessConfigFile. Note - * that we do it even if changeVal is false, since ProcessConfigFile wants - * the marking to occur during its testing pass. - */ - if (source == PGC_S_FILE) - record->status |= GUC_IS_IN_FILE; - /* * Check if the option can be set at this time. See guc.h for the precise * rules. @@ -5121,22 +5115,13 @@ set_config_option(const char *name, const char *value, switch (record->context) { case PGC_INTERNAL: - if (context == PGC_SIGHUP) - { - /* - * Historically we've just silently ignored attempts to set - * PGC_INTERNAL variables from the config file. Maybe it'd be - * better to use the prohibitValueChange logic for this? - */ - return true; - } - else if (context != PGC_INTERNAL) + if (context != PGC_INTERNAL) { ereport(elevel, (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM), errmsg("parameter \"%s\" cannot be changed", name))); - return false; + return 0; } break; case PGC_POSTMASTER: @@ -5150,13 +5135,7 @@ set_config_option(const char *name, const char *value, * hooks, etc, we can't just compare the given string directly * to what's stored. Set a flag to check below after we have * the final storable value. - * - * During the "checking" pass we just do nothing, to avoid - * printing the warning twice. */ - if (!changeVal) - return true; - prohibitValueChange = true; } else if (context != PGC_POSTMASTER) @@ -5165,7 +5144,7 @@ set_config_option(const char *name, const char *value, (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM), errmsg("parameter \"%s\" cannot be changed without restarting the server", name))); - return false; + return 0; } break; case PGC_SIGHUP: @@ -5175,7 +5154,7 @@ set_config_option(const char *name, const char *value, (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM), errmsg("parameter \"%s\" cannot be changed now", name))); - return false; + return 0; } /* @@ -5197,7 +5176,7 @@ set_config_option(const char *name, const char *value, * backend start. */ if (IsUnderPostmaster) - return true; + return -1; } else if (context != PGC_POSTMASTER && context != PGC_BACKEND && source != PGC_S_CLIENT) @@ -5206,7 +5185,7 @@ set_config_option(const char *name, const char *value, (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM), errmsg("parameter \"%s\" cannot be set after connection start", name))); - return false; + return 0; } break; case PGC_SUSET: @@ -5216,7 +5195,7 @@ set_config_option(const char *name, const char *value, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("permission denied to set parameter \"%s\"", name))); - return false; + return 0; } break; case PGC_USERSET: @@ -5254,7 +5233,7 @@ set_config_option(const char *name, const char *value, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("cannot set parameter \"%s\" within security-definer function", name))); - return false; + return 0; } if (InSecurityRestrictedOperation()) { @@ -5262,7 +5241,7 @@ set_config_option(const char *name, const char *value, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("cannot set parameter \"%s\" within security-restricted operation", name))); - return false; + return 0; } } @@ -5288,7 +5267,7 @@ set_config_option(const char *name, const char *value, { elog(DEBUG3, "\"%s\": setting ignored because previous source is higher priority", name); - return true; + return -1; } changeVal = false; } @@ -5312,18 +5291,18 @@ set_config_option(const char *name, const char *value, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("parameter \"%s\" requires a Boolean value", name))); - return false; + return 0; } if (!call_bool_check_hook(conf, &newval, &newextra, source, elevel)) - return false; + return 0; } else if (source == PGC_S_DEFAULT) { newval = conf->boot_val; if (!call_bool_check_hook(conf, &newval, &newextra, source, elevel)) - return false; + return 0; } else { @@ -5335,11 +5314,14 @@ set_config_option(const char *name, const char *value, if (prohibitValueChange) { if (*conf->variable != newval) + { ereport(elevel, (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM), errmsg("parameter \"%s\" cannot be changed without restarting the server", name))); - return false; + return 0; + } + return -1; } if (changeVal) @@ -5401,7 +5383,7 @@ set_config_option(const char *name, const char *value, errmsg("invalid value for parameter \"%s\": \"%s\"", name, value), hintmsg ? errhint("%s", _(hintmsg)) : 0)); - return false; + return 0; } if (newval < conf->min || newval > conf->max) { @@ -5409,18 +5391,18 @@ set_config_option(const char *name, const char *value, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("%d is outside the valid range for parameter \"%s\" (%d .. %d)", newval, name, conf->min, conf->max))); - return false; + return 0; } if (!call_int_check_hook(conf, &newval, &newextra, source, elevel)) - return false; + return 0; } else if (source == PGC_S_DEFAULT) { newval = conf->boot_val; if (!call_int_check_hook(conf, &newval, &newextra, source, elevel)) - return false; + return 0; } else { @@ -5432,11 +5414,14 @@ set_config_option(const char *name, const char *value, if (prohibitValueChange) { if (*conf->variable != newval) + { ereport(elevel, (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM), errmsg("parameter \"%s\" cannot be changed without restarting the server", name))); - return false; + return 0; + } + return -1; } if (changeVal) @@ -5495,7 +5480,7 @@ set_config_option(const char *name, const char *value, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("parameter \"%s\" requires a numeric value", name))); - return false; + return 0; } if (newval < conf->min || newval > conf->max) { @@ -5503,18 +5488,18 @@ set_config_option(const char *name, const char *value, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("%g is outside the valid range for parameter \"%s\" (%g .. %g)", newval, name, conf->min, conf->max))); - return false; + return 0; } if (!call_real_check_hook(conf, &newval, &newextra, source, elevel)) - return false; + return 0; } else if (source == PGC_S_DEFAULT) { newval = conf->boot_val; if (!call_real_check_hook(conf, &newval, &newextra, source, elevel)) - return false; + return 0; } else { @@ -5526,11 +5511,14 @@ set_config_option(const char *name, const char *value, if (prohibitValueChange) { if (*conf->variable != newval) + { ereport(elevel, (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM), errmsg("parameter \"%s\" cannot be changed without restarting the server", name))); - return false; + return 0; + } + return -1; } if (changeVal) @@ -5589,7 +5577,7 @@ set_config_option(const char *name, const char *value, */ newval = guc_strdup(elevel, value); if (newval == NULL) - return false; + return 0; /* * The only built-in "parsing" check we have is to apply @@ -5602,7 +5590,7 @@ set_config_option(const char *name, const char *value, source, elevel)) { free(newval); - return false; + return 0; } } else if (source == PGC_S_DEFAULT) @@ -5612,7 +5600,7 @@ set_config_option(const char *name, const char *value, { newval = guc_strdup(elevel, conf->boot_val); if (newval == NULL) - return false; + return 0; } else newval = NULL; @@ -5621,7 +5609,7 @@ set_config_option(const char *name, const char *value, source, elevel)) { free(newval); - return false; + return 0; } } else @@ -5640,11 +5628,14 @@ set_config_option(const char *name, const char *value, /* newval shouldn't be NULL, so we're a bit sloppy here */ if (*conf->variable == NULL || newval == NULL || strcmp(*conf->variable, newval) != 0) + { ereport(elevel, (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM), errmsg("parameter \"%s\" cannot be changed without restarting the server", name))); - return false; + return 0; + } + return -1; } if (changeVal) @@ -5718,18 +5709,18 @@ set_config_option(const char *name, const char *value, if (hintmsg) pfree(hintmsg); - return false; + return 0; } if (!call_enum_check_hook(conf, &newval, &newextra, source, elevel)) - return false; + return 0; } else if (source == PGC_S_DEFAULT) { newval = conf->boot_val; if (!call_enum_check_hook(conf, &newval, &newextra, source, elevel)) - return false; + return 0; } else { @@ -5741,11 +5732,14 @@ set_config_option(const char *name, const char *value, if (prohibitValueChange) { if (*conf->variable != newval) + { ereport(elevel, (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM), errmsg("parameter \"%s\" cannot be changed without restarting the server", name))); - return false; + return 0; + } + return -1; } if (changeVal) @@ -5794,7 +5788,7 @@ set_config_option(const char *name, const char *value, if (changeVal && (record->flags & GUC_REPORT)) ReportGUCOption(record); - return true; + return changeVal ? 1 : -1; } @@ -6095,12 +6089,12 @@ ExecSetVariableStmt(VariableSetStmt *stmt) { case VAR_SET_VALUE: case VAR_SET_CURRENT: - set_config_option(stmt->name, - ExtractSetVariableArgs(stmt), - (superuser() ? PGC_SUSET : PGC_USERSET), - PGC_S_SESSION, - action, - true); + (void) set_config_option(stmt->name, + ExtractSetVariableArgs(stmt), + (superuser() ? PGC_SUSET : PGC_USERSET), + PGC_S_SESSION, + action, + true); break; case VAR_SET_MULTI: @@ -6158,12 +6152,12 @@ ExecSetVariableStmt(VariableSetStmt *stmt) break; case VAR_SET_DEFAULT: case VAR_RESET: - set_config_option(stmt->name, - NULL, - (superuser() ? PGC_SUSET : PGC_USERSET), - PGC_S_SESSION, - action, - true); + (void) set_config_option(stmt->name, + NULL, + (superuser() ? PGC_SUSET : PGC_USERSET), + PGC_S_SESSION, + action, + true); break; case VAR_RESET_ALL: ResetAllOptions(); @@ -6203,12 +6197,12 @@ SetPGVariable(const char *name, List *args, bool is_local) char *argstring = flatten_set_variable_args(name, args); /* Note SET DEFAULT (argstring == NULL) is equivalent to RESET */ - set_config_option(name, - argstring, - (superuser() ? PGC_SUSET : PGC_USERSET), - PGC_S_SESSION, - is_local ? GUC_ACTION_LOCAL : GUC_ACTION_SET, - true); + (void) set_config_option(name, + argstring, + (superuser() ? PGC_SUSET : PGC_USERSET), + PGC_S_SESSION, + is_local ? GUC_ACTION_LOCAL : GUC_ACTION_SET, + true); } /* @@ -6246,12 +6240,12 @@ set_config_by_name(PG_FUNCTION_ARGS) is_local = PG_GETARG_BOOL(2); /* Note SET DEFAULT (argstring == NULL) is equivalent to RESET */ - set_config_option(name, - value, - (superuser() ? PGC_SUSET : PGC_USERSET), - PGC_S_SESSION, - is_local ? GUC_ACTION_LOCAL : GUC_ACTION_SET, - true); + (void) set_config_option(name, + value, + (superuser() ? PGC_SUSET : PGC_USERSET), + PGC_S_SESSION, + is_local ? GUC_ACTION_LOCAL : GUC_ACTION_SET, + true); /* get the new current value */ new_value = GetConfigOptionByName(name, NULL); @@ -6421,7 +6415,7 @@ define_custom_variable(struct config_generic * variable) { if (set_config_option(name, value, phcontext, pHolder->gen.source, - GUC_ACTION_SET, true)) + GUC_ACTION_SET, true) > 0) { /* Also copy over any saved source-location information */ if (pHolder->gen.sourcefile) @@ -7943,9 +7937,9 @@ validate_option_array_item(const char *name, const char *value, /* if a permissions error should be thrown, let set_config_option do it */ /* test for permissions and valid option value */ - set_config_option(name, value, - superuser() ? PGC_SUSET : PGC_USERSET, - PGC_S_TEST, GUC_ACTION_SET, false); + (void) set_config_option(name, value, + superuser() ? PGC_SUSET : PGC_USERSET, + PGC_S_TEST, GUC_ACTION_SET, false); return true; } diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h index d99d624fae..450915aaea 100644 --- a/src/include/utils/guc.h +++ b/src/include/utils/guc.h @@ -313,7 +313,7 @@ extern void ParseLongOption(const char *string, char **name, char **value); extern bool parse_int(const char *value, int *result, int flags, const char **hintmsg); extern bool parse_real(const char *value, double *result); -extern bool set_config_option(const char *name, const char *value, +extern int set_config_option(const char *name, const char *value, GucContext context, GucSource source, GucAction action, bool changeVal); extern char *GetConfigOptionByName(const char *name, const char **varname);