From f4ee82e3d397544a287da65dd54846aa80694610 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Tue, 13 Mar 2007 14:32:25 +0000 Subject: [PATCH] Reverted waiting for further fixes: Make configuration parameters fall back to their default values when they are removed from the configuration file. Joachim Wieland --- src/backend/access/transam/xact.c | 6 +- src/backend/utils/misc/guc-file.l | 210 +----------------------- src/backend/utils/misc/guc.c | 254 ++++-------------------------- src/include/utils/guc_tables.h | 11 +- 4 files changed, 40 insertions(+), 441 deletions(-) diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index 6a2ebd4089..51a41ca445 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -10,7 +10,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.236 2007/03/13 00:33:38 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.237 2007/03/13 14:32:25 petere Exp $ * *------------------------------------------------------------------------- */ @@ -1513,9 +1513,6 @@ CommitTransaction(void) /* NOTIFY commit must come before lower-level cleanup */ AtCommit_Notify(); - /* GUC processing could abort transaction */ - AtEOXact_GUC(true, false); - /* * Update flat files if we changed pg_database, pg_authid or * pg_auth_members. This should be the last step before commit. @@ -1626,6 +1623,7 @@ CommitTransaction(void) /* Check we've released all catcache entries */ AtEOXact_CatCache(true); + AtEOXact_GUC(true, false); AtEOXact_SPI(true); AtEOXact_on_commit_actions(true); AtEOXact_Namespace(true); diff --git a/src/backend/utils/misc/guc-file.l b/src/backend/utils/misc/guc-file.l index b9b62bcabf..d0fc263614 100644 --- a/src/backend/utils/misc/guc-file.l +++ b/src/backend/utils/misc/guc-file.l @@ -4,7 +4,7 @@ * * Copyright (c) 2000-2007, PostgreSQL Global Development Group * - * $PostgreSQL: pgsql/src/backend/utils/misc/guc-file.l,v 1.48 2007/03/12 22:09:27 petere Exp $ + * $PostgreSQL: pgsql/src/backend/utils/misc/guc-file.l,v 1.49 2007/03/13 14:32:25 petere Exp $ */ %{ @@ -54,8 +54,6 @@ static bool ParseConfigFile(const char *config_file, const char *calling_file, static void free_name_value_list(struct name_value_pair * list); static char *GUC_scanstr(const char *s); -static char *custom_variable_classes_backup; - %} %option 8bit @@ -118,10 +116,6 @@ ProcessConfigFile(GucContext context) { int elevel; struct name_value_pair *item, *head, *tail; - int i; - bool *in_conffile = NULL; - const char *var; - bool success = false; Assert(context == PGC_POSTMASTER || context == PGC_SIGHUP); @@ -138,18 +132,6 @@ ProcessConfigFile(GucContext context) head = tail = NULL; - /* - * We do not know whether we will read the configuration file - * without error. If we encounter an error we have to restore the - * previous value of the custom_variable_classes variable for - * consistency. Therefore we have to save its value. - */ - var = GetConfigOption("custom_variable_classes"); - if (var) - custom_variable_classes_backup = pstrdup(var); - set_config_option("custom_variable_classes", NULL, context, - PGC_S_DEFAULT, false, true); - if (!ParseConfigFile(ConfigFileName, NULL, 0, context, elevel, &head, &tail)) @@ -158,198 +140,20 @@ ProcessConfigFile(GucContext context) /* Check if all options are valid */ for (item = head; item; item = item->next) { - char *sep = strchr(item->name, GUC_QUALIFIER_SEPARATOR); - if (sep && !is_custom_class(item->name, sep - item->name)) - { - ereport(elevel, - (errcode(ERRCODE_UNDEFINED_OBJECT), - errmsg("unrecognized configuration parameter \"%s\"", - item->name))); - goto cleanup_list; - } - if (!set_config_option(item->name, item->value, context, PGC_S_FILE, false, false)) goto cleanup_list; } - - /* - * Mark all variables as not showing up in the config file. The - * allocation has to take place after ParseConfigFile() since this - * function can change num_guc_variables due to custom variables. - * It would be easier to add a new field or status bit to struct - * conf_generic, but that way we would expose internal information - * that is just needed here in the following few lines. The price - * to pay for this separation are a few more loops over the set of - * configuration options, but those are expected to be rather few - * and we only have to pay the cost at SIGHUP. We initialize - * in_conffile only here because set_config_option() makes - * guc_variables grow with custom variables. - */ - in_conffile = guc_malloc(elevel, num_guc_variables * sizeof(bool)); - if (!in_conffile) - goto cleanup_list; - for (i = 0; i < num_guc_variables; i++) - in_conffile[i] = false; - - for (item = head; item; item = item->next) - { - /* - * After set_config_option() the variable name item->name is - * known to exist. - */ - Assert(guc_get_index(item->name) >= 0); - in_conffile[guc_get_index(item->name)] = true; - } - - for (i = 0; i < num_guc_variables; i++) - { - struct config_generic *gconf = guc_variables[i]; - if (!in_conffile[i] && strchr(gconf->name, GUC_QUALIFIER_SEPARATOR)) - { - /* handle custom variables here */ - int j; - - /* - * guc_delete_variable will re-sort the array of - * configuration options and decrease num_guc_variables by - * one. - */ - guc_delete_custom_variable(gconf->name); - - /* - * Since the array is always sorted we just have to shift - * all boolean values after position i by one position to - * the left. - */ - for (j = i; j < num_guc_variables; j++) - in_conffile[j] = in_conffile[j+1]; - - /* - * Decrease the loop variable to re-test the entry at the - * current position that is now a different one. - */ - i--; - - continue; - } - else if (!in_conffile[i] && gconf->source == PGC_S_FILE) - { - if (gconf->context < PGC_SIGHUP) - ereport(elevel, - (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM), - errmsg("parameter \"%s\" cannot be changed after server start; configuration file change ignored", - gconf->name))); - else - { - /* prepare */ - GucStack *stack; - if (gconf->reset_source == PGC_S_FILE) - gconf->reset_source = PGC_S_DEFAULT; - for (stack = gconf->stack; stack; stack = stack->prev) - if (stack->source == PGC_S_FILE) - stack->source = PGC_S_DEFAULT; - /* apply the default */ - set_config_option(gconf->name, NULL, context, - PGC_S_DEFAULT, false, true); - } - } - else if (!in_conffile[i] && gconf->reset_source == PGC_S_FILE) - { - /*------ - * Change the reset_val to default_val. Here's an - * example: In the configuration file we have - * - * seq_page_cost = 3.00 - * - * Now we execute in a session - * - * SET seq_page_cost TO 4.00; - * - * Then we remove this option from the configuration file - * and send SIGHUP. Now when you execute - * - * RESET seq_page_cost; - * - * it should fall back to 1.00 (the default value for - * seq_page_cost) and not to 3.00 (which is the current - * reset_val). - */ - - switch (gconf->vartype) - { - case PGC_BOOL: - { - struct config_bool *conf; - conf = (struct config_bool *) gconf; - conf->reset_val = conf->boot_val; - break; - } - case PGC_INT: - { - struct config_int *conf; - conf = (struct config_int *) gconf; - conf->reset_val = conf->boot_val; - break; - } - case PGC_REAL: - { - struct config_real *conf; - conf = (struct config_real *) gconf; - conf->reset_val = conf->boot_val; - break; - } - case PGC_STRING: - { - struct config_string *conf; - conf = (struct config_string *) gconf; - /* - * We can cast away the const here because we - * won't free the address. It is protected by - * set_string_field() and string_field_used(). - */ - conf->reset_val = (char *) conf->boot_val; - break; - } - } - } - } - /* If we got here all the options checked out okay, so apply them. */ for (item = head; item; item = item->next) + { set_config_option(item->name, item->value, context, PGC_S_FILE, false, true); - - /* - * Reset variables to the value of environment variables - * (PGC_S_ENV_VAR overrules PGC_S_FILE). PGPORT is ignored, - * because it cannot be changed without restart. - */ - var = getenv("PGDATESTYLE"); - if (var != NULL) - set_config_option("datestyle", var, context, - PGC_S_ENV_VAR, false, true); - - var = getenv("PGCLIENTENCODING"); - if (var != NULL) - set_config_option("client_encoding", var, context, - PGC_S_ENV_VAR, false, true); - - success = true; + } cleanup_list: - free(in_conffile); free_name_value_list(head); - if (!success) - set_config_option("custom_variable_classes", - custom_variable_classes_backup, - context, PGC_S_FILE, false, true); - if (custom_variable_classes_backup) - { - pfree(custom_variable_classes_backup); - custom_variable_classes_backup = NULL; - } } @@ -508,14 +312,14 @@ ParseConfigFile(const char *config_file, const char *calling_file, { pfree(opt_name); pfree(opt_value); - - /* We assume the error message was logged already. */ + /* we assume error message was logged already */ OK = false; goto cleanup_exit; } + pfree(opt_name); + pfree(opt_value); } - - if (pg_strcasecmp(opt_name, "include") != 0) + else { /* append to list */ struct name_value_pair *item; diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index b311ce7dae..8088432bac 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -10,7 +10,7 @@ * Written by Peter Eisentraut . * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.381 2007/03/13 09:11:05 mha Exp $ + * $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.382 2007/03/13 14:32:25 petere Exp $ * *-------------------------------------------------------------------- */ @@ -2439,8 +2439,7 @@ set_string_field(struct config_string * conf, char **field, char *newval) if (oldval == NULL || oldval == *(conf->variable) || oldval == conf->reset_val || - oldval == conf->tentative_val || - oldval == conf->boot_val) + oldval == conf->tentative_val) return; for (stack = conf->gen.stack; stack; stack = stack->prev) { @@ -2463,8 +2462,7 @@ string_field_used(struct config_string * conf, char *strval) if (strval == *(conf->variable) || strval == conf->reset_val || - strval == conf->tentative_val || - strval == conf->boot_val) + strval == conf->tentative_val) return true; for (stack = conf->gen.stack; stack; stack = stack->prev) { @@ -2633,105 +2631,6 @@ add_guc_variable(struct config_generic * var, int elevel) return true; } -static int -guc_get_index(const char *name) -{ - int i; - - for (i = 0; i < num_guc_variables; i++) - if (pg_strcasecmp(name, guc_variables[i]->name) == 0) - return i; - - return -1; -} - -static void -guc_delete_variable(const char *name) -{ - struct config_generic *gen; - int idx; - GucStack *stack, *prev; - struct config_string *conf; - - idx = guc_get_index(name); - Assert(idx >= 0); - - gen = guc_variables[idx]; - - /* - * Even though this function could delete other types of variables as well, - * at the moment we only call it for custom variables that always have type - * string. - */ - Assert(gen->group == CUSTOM_OPTIONS); - Assert(gen->vartype == PGC_STRING); - - conf = (struct config_string *) gen; - set_string_field(conf, &conf->reset_val, NULL); - set_string_field(conf, &conf->tentative_val, NULL); - for (stack = conf->gen.stack; stack; stack = prev) - { - set_string_field(conf, &stack->tentative_val.stringval, NULL); - set_string_field(conf, &stack->value.stringval, NULL); - prev = stack->prev; - pfree(stack); - } - - /* no pfree() here, gen has been allocated via guc_malloc */ - free(gen); - guc_variables[idx] = guc_variables[num_guc_variables - 1]; - num_guc_variables--; - qsort((void *) guc_variables, num_guc_variables, - sizeof(struct config_generic *), guc_var_compare); -} - -static void -guc_delete_custom_variable(const char *name) -{ - struct config_generic *gen; - struct config_string *conf; - int idx; - - idx = guc_get_index(name); - Assert(idx >= 0); - - gen = guc_variables[idx]; - - Assert(gen->group == CUSTOM_OPTIONS); - Assert(gen->vartype == PGC_STRING); - - conf = (struct config_string *) gen; - - /* - * Here we check whether it is safe to really delete the variable - * or if we have to wait for the end of the transaction. We do - * not remove the physical entry of a custom variable if it has - * been SET to another value in the transaction but has been - * removed from the configuration file. - */ - if (gen->stack) - { - /* - * Make sure this custom variable (which has a definition in - * the configuration file) behaves as any other custom - * variable from now on. We have deleted its former - * definition; that's why "RESET foo.bar" should not fall back - * to this deleted definition from the configuration file. - * The analogy is that any other custom variable that gets - * introduced in a session has no reset value either. And a - * variable that has been SET within a transaction and has - * then been deleted from the configuration file should behave - * as if it had been introduced in the session. - */ - Assert(gen->vartype == PGC_STRING); - gen->reset_source = PGC_S_DEFAULT; - set_string_field(conf, &conf->reset_val, NULL); - } - else - guc_delete_variable(name); -} - - /* * Create and add a placeholder variable. It's presumed to belong * to a valid custom variable class at this point. @@ -2910,39 +2809,39 @@ InitializeGUCOptions(void) struct config_bool *conf = (struct config_bool *) gconf; if (conf->assign_hook) - if (!(*conf->assign_hook) (conf->boot_val, true, + if (!(*conf->assign_hook) (conf->reset_val, true, PGC_S_DEFAULT)) elog(FATAL, "failed to initialize %s to %d", - conf->gen.name, (int) conf->boot_val); - *conf->variable = conf->reset_val = conf->boot_val; + conf->gen.name, (int) conf->reset_val); + *conf->variable = conf->reset_val; break; } case PGC_INT: { struct config_int *conf = (struct config_int *) gconf; - Assert(conf->boot_val >= conf->min); - Assert(conf->boot_val <= conf->max); + Assert(conf->reset_val >= conf->min); + Assert(conf->reset_val <= conf->max); if (conf->assign_hook) - if (!(*conf->assign_hook) (conf->boot_val, true, + if (!(*conf->assign_hook) (conf->reset_val, true, PGC_S_DEFAULT)) elog(FATAL, "failed to initialize %s to %d", - conf->gen.name, conf->boot_val); - *conf->variable = conf->reset_val = conf->boot_val; + conf->gen.name, conf->reset_val); + *conf->variable = conf->reset_val; break; } case PGC_REAL: { struct config_real *conf = (struct config_real *) gconf; - Assert(conf->boot_val >= conf->min); - Assert(conf->boot_val <= conf->max); + Assert(conf->reset_val >= conf->min); + Assert(conf->reset_val <= conf->max); if (conf->assign_hook) - if (!(*conf->assign_hook) (conf->boot_val, true, + if (!(*conf->assign_hook) (conf->reset_val, true, PGC_S_DEFAULT)) elog(FATAL, "failed to initialize %s to %g", - conf->gen.name, conf->boot_val); - *conf->variable = conf->reset_val = conf->boot_val; + conf->gen.name, conf->reset_val); + *conf->variable = conf->reset_val; break; } case PGC_STRING: @@ -3397,25 +3296,8 @@ push_old_value(struct config_generic * gconf) } } -/*------ +/* * Do GUC processing at transaction or subtransaction commit or abort. - * - * GUC can abort the transaction in exactly one case, namely when you - * delete a custom variable class while a still-open transaction has - * SET a custom variable within this class. - * - * Consider the following example. In the configuration file we could - * have: - * - * custom_variable_classes = "foo" - * - * begin; - * set foo.bar to 1; - * - * commit; - * - * This will result in an error because foo.bar is no longer available - * but commit would have to guarantee that the value is preserved. */ void AtEOXact_GUC(bool isCommit, bool isSubXact) @@ -3453,33 +3335,6 @@ AtEOXact_GUC(bool isCommit, bool isSubXact) continue; Assert(stack->nest_level == my_level); - if (!isSubXact && gconf->group == CUSTOM_OPTIONS) - { - char *dot = strchr(gconf->name, GUC_QUALIFIER_SEPARATOR); - Assert(dot != NULL); - if (!is_custom_class(gconf->name, dot - gconf->name)) - { - if (!isCommit) - { - /* We do not commit the transaction. Delete the variable. */ - guc_delete_variable(gconf->name); - /* Call the loop with the same i again, which is - * the next variable. */ - i--; - continue; - } - else - { - /* We commit the transaction. Throw an error that - * the respective custom class does not exist. */ - ereport(ERROR, - (errcode(ERRCODE_UNDEFINED_OBJECT), - errmsg("no valid custom variable class available for " - "parameter \"%s\"", gconf->name))); - } - } - } - /* * We will pop the stack entry. Start by restoring outer xact status * (since we may want to modify it below). Be careful to use @@ -4143,26 +3998,19 @@ set_config_option(const char *name, const char *value, } /* - * Should we set reset/stacked values? (If so, the behavior is not - * transactional.) This is done either when we get a default - * value from the database's/user's/client's default settings or - * when we reset a value to its default. + * Should we set reset/stacked values? (If so, the behavior is not + * transactional.) */ - makeDefault = changeVal && (source <= PGC_S_OVERRIDE) - && ((value != NULL) || source == PGC_S_DEFAULT); + makeDefault = changeVal && (source <= PGC_S_OVERRIDE) && (value != NULL); /* - * Ignore attempted set if overridden by previously processed - * setting. However, if changeVal is false then plow ahead anyway - * since we are trying to find out if the value is potentially - * good, not actually use it. Also keep going if makeDefault is - * true, since we may want to set the reset/stacked values even if - * we can't set the variable itself. There's one exception to - * this rule: if we want to apply the default value to variables - * that were removed from the configuration file. This is - * indicated by source == PGC_S_DEFAULT. + * Ignore attempted set if overridden by previously processed setting. + * However, if changeVal is false then plow ahead anyway since we are + * trying to find out if the value is potentially good, not actually use + * it. Also keep going if makeDefault is true, since we may want to set + * the reset/stacked values even if we can't set the variable itself. */ - if (record->source > source && source != PGC_S_DEFAULT) + if (record->source > source) { if (changeVal && !makeDefault) { @@ -4194,14 +4042,6 @@ set_config_option(const char *name, const char *value, return false; } } - /* - * If value == NULL and source == PGC_S_DEFAULT then - * we reset some value to its default (removed from - * configuration file). - */ - else if (source == PGC_S_DEFAULT) - newval = conf->boot_val; - /* else we handle a "RESET varname" command */ else { newval = conf->reset_val; @@ -4286,14 +4126,6 @@ set_config_option(const char *name, const char *value, return false; } } - /* - * If value == NULL and source == PGC_S_DEFAULT then - * we reset some value to its default (removed from - * configuration file). - */ - else if (source == PGC_S_DEFAULT) - newval = conf->boot_val; - /* else we handle a "RESET varname" command */ else { newval = conf->reset_val; @@ -4378,14 +4210,6 @@ set_config_option(const char *name, const char *value, return false; } } - /* - * If value == NULL and source == PGC_S_DEFAULT then - * we reset some value to its default (removed from - * configuration file). - */ - else if (source == PGC_S_DEFAULT) - newval = conf->boot_val; - /* else we handle a "RESET varname" command */ else { newval = conf->reset_val; @@ -4464,23 +4288,6 @@ set_config_option(const char *name, const char *value, if (conf->gen.flags & GUC_IS_NAME) truncate_identifier(newval, strlen(newval), true); } - /* - * If value == NULL and source == PGC_S_DEFAULT then - * we reset some value to its default (removed from - * configuration file). - */ - else if (source == PGC_S_DEFAULT) - { - if (conf->boot_val == NULL) - newval = NULL; - else - { - newval = guc_strdup(elevel, conf->boot_val); - if (newval == NULL) - return false; - } - } - /* else we handle a "RESET varname" command */ else if (conf->reset_val) { /* @@ -6497,13 +6304,6 @@ assign_custom_variable_classes(const char *newval, bool doit, GucSource source) int c; StringInfoData buf; - /* - * Resetting custom_variable_classes by removing it from the - * configuration file will lead to newval = NULL - */ - if (newval == NULL) - return guc_strdup(ERROR, ""); - initStringInfo(&buf); while ((c = *cp++) != 0) { @@ -6548,7 +6348,7 @@ assign_custom_variable_classes(const char *newval, bool doit, GucSource source) if (buf.len == 0) newval = NULL; else if (doit) - newval = guc_strdup(ERROR, buf.data); + newval = strdup(buf.data); pfree(buf.data); return newval; diff --git a/src/include/utils/guc_tables.h b/src/include/utils/guc_tables.h index 19dd16d15e..611afb8083 100644 --- a/src/include/utils/guc_tables.h +++ b/src/include/utils/guc_tables.h @@ -7,7 +7,7 @@ * * Portions Copyright (c) 1996-2007, PostgreSQL Global Development Group * - * $PostgreSQL: pgsql/src/include/utils/guc_tables.h,v 1.31 2007/03/12 22:09:28 petere Exp $ + * $PostgreSQL: pgsql/src/include/utils/guc_tables.h,v 1.32 2007/03/13 14:32:25 petere Exp $ * *------------------------------------------------------------------------- */ @@ -154,12 +154,11 @@ struct config_bool /* these fields must be set correctly in initial value: */ /* (all but reset_val are constants) */ bool *variable; - bool boot_val; + bool reset_val; GucBoolAssignHook assign_hook; GucShowHook show_hook; /* variable fields, initialized at runtime: */ bool tentative_val; - bool reset_val; }; struct config_int @@ -168,14 +167,13 @@ struct config_int /* these fields must be set correctly in initial value: */ /* (all but reset_val are constants) */ int *variable; - int boot_val; + int reset_val; int min; int max; GucIntAssignHook assign_hook; GucShowHook show_hook; /* variable fields, initialized at runtime: */ int tentative_val; - int reset_val; }; struct config_real @@ -184,14 +182,13 @@ struct config_real /* these fields must be set correctly in initial value: */ /* (all but reset_val are constants) */ double *variable; - double boot_val; + double reset_val; double min; double max; GucRealAssignHook assign_hook; GucShowHook show_hook; /* variable fields, initialized at runtime: */ double tentative_val; - double reset_val; }; struct config_string