From 40fda15dcebdd07ee75ac1f55dad145f91297b99 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Mon, 10 Sep 2007 00:57:22 +0000
Subject: [PATCH] Code review for GUC
revert-values-if-removed-from-postgresql.conf patch; and in passing, fix some
bogosities dating from the custom_variable_classes patch. Fix guc-file.l to
correctly check changes in custom_variable_classes that are attempted
concurrently with additions/removals of custom variables, and don't allow the
new setting to be applied in advance of checking it. Clean up messy and
undocumented situation for string variables with NULL boot_val. Fix
DefineCustomVariable functions to initialize boot_val correctly. Prevent
find_option from inserting bogus placeholders for custom variables that are
simply inquired about rather than being set.
---
src/backend/utils/cache/ts_cache.c | 12 +-
src/backend/utils/misc/README | 10 +-
src/backend/utils/misc/guc-file.l | 313 +++++++++++---------
src/backend/utils/misc/guc.c | 443 ++++++++++++++---------------
src/include/utils/guc_tables.h | 21 +-
5 files changed, 407 insertions(+), 392 deletions(-)
diff --git a/src/backend/utils/cache/ts_cache.c b/src/backend/utils/cache/ts_cache.c
index 94051b8c32f..63c57905565 100644
--- a/src/backend/utils/cache/ts_cache.c
+++ b/src/backend/utils/cache/ts_cache.c
@@ -20,7 +20,7 @@
* Copyright (c) 2006-2007, PostgreSQL Global Development Group
*
* IDENTIFICATION
- * $PostgreSQL: pgsql/src/backend/utils/cache/ts_cache.c,v 1.2 2007/08/22 01:39:45 tgl Exp $
+ * $PostgreSQL: pgsql/src/backend/utils/cache/ts_cache.c,v 1.3 2007/09/10 00:57:21 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -592,14 +592,6 @@ getTSCurrentConfig(bool emitError)
const char *
assignTSCurrentConfig(const char *newval, bool doit, GucSource source)
{
- /* do nothing during initial GUC setup */
- if (newval == NULL)
- {
- if (doit)
- TSCurrentConfigCache = InvalidOid;
- return newval;
- }
-
/*
* If we aren't inside a transaction, we cannot do database access so
* cannot verify the config name. Must accept it on faith.
@@ -637,7 +629,7 @@ assignTSCurrentConfig(const char *newval, bool doit, GucSource source)
newval = strdup(buf);
pfree(buf);
- if (doit)
+ if (doit && newval)
TSCurrentConfigCache = cfgId;
}
else
diff --git a/src/backend/utils/misc/README b/src/backend/utils/misc/README
index 3ea838b1f53..bf1e3b65455 100644
--- a/src/backend/utils/misc/README
+++ b/src/backend/utils/misc/README
@@ -1,4 +1,4 @@
-$PostgreSQL: pgsql/src/backend/utils/misc/README,v 1.5 2004/07/01 00:51:24 tgl Exp $
+$PostgreSQL: pgsql/src/backend/utils/misc/README,v 1.6 2007/09/10 00:57:21 tgl Exp $
GUC IMPLEMENTATION NOTES
@@ -53,6 +53,14 @@ The third choice is allowed in case the assign_hook wants to return a
datestyle always returns a string that includes both output and input
datestyle options, although the input might have specified only one.
+Note that a string variable's assign_hook will NEVER be called with a NULL
+value for newvalue, since there would be no way to distinguish success
+and failure returns. If the boot_val or reset_val for a string variable
+is NULL, it will just be assigned without calling the assign_hook.
+Therefore, a NULL boot_val should never be used in combination with an
+assign_hook that has side-effects, as the side-effects wouldn't happen
+during a RESET that re-institutes the boot-time setting.
+
If a show_hook is provided, it points to a function of the signature
const char *show_hook(void)
This hook allows variable-specific computation of the value displayed
diff --git a/src/backend/utils/misc/guc-file.l b/src/backend/utils/misc/guc-file.l
index 4327601320e..124dfbf62e1 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.50 2007/04/21 20:02:40 petere Exp $
+ * $PostgreSQL: pgsql/src/backend/utils/misc/guc-file.l,v 1.51 2007/09/10 00:57:21 tgl Exp $
*/
%{
@@ -116,9 +116,10 @@ ProcessConfigFile(GucContext context)
{
int elevel;
struct name_value_pair *item, *head, *tail;
+ char *cvc = NULL;
+ struct config_string *cvc_struct;
+ const char *envvar;
int i;
- bool *in_conffile = NULL;
- const char *var;
Assert(context == PGC_POSTMASTER || context == PGC_SIGHUP);
@@ -133,6 +134,7 @@ ProcessConfigFile(GucContext context)
else
elevel = ERROR;
+ /* Parse the file into a list of option names and values */
head = tail = NULL;
if (!ParseConfigFile(ConfigFileName, NULL,
@@ -140,162 +142,178 @@ ProcessConfigFile(GucContext context)
&head, &tail))
goto cleanup_list;
- /* Check if all options are valid */
- for (item = head; item; item = item->next)
+ /*
+ * We need the proposed new value of custom_variable_classes to check
+ * custom variables with. ParseConfigFile ensured that if it's in
+ * the file, it's first in the list. But first check to see if we
+ * have an active value from the command line, which should override
+ * the file in any case. (Since there's no relevant env var, the
+ * only possible nondefault sources are the file and ARGV.)
+ */
+ cvc_struct = (struct config_string *)
+ find_option("custom_variable_classes", false, elevel);
+ if (cvc_struct && cvc_struct->gen.reset_source > PGC_S_FILE)
{
- char *sep = strchr(item->name, GUC_QUALIFIER_SEPARATOR);
- if (sep && !is_custom_class(item->name, sep - item->name))
+ cvc = guc_strdup(elevel, cvc_struct->reset_val);
+ if (cvc == NULL)
+ goto cleanup_list;
+ }
+ else if (head != NULL &&
+ guc_name_compare(head->name, "custom_variable_classes") == 0)
+ {
+ /*
+ * Need to canonicalize the value via the assign hook. Casting away
+ * const is a bit ugly, but we know the result is malloc'd.
+ */
+ cvc = (char *) assign_custom_variable_classes(head->value,
+ false, PGC_S_FILE);
+ if (cvc == NULL)
{
ereport(elevel,
- (errcode(ERRCODE_UNDEFINED_OBJECT),
- errmsg("unrecognized configuration parameter \"%s\"",
- item->name)));
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("invalid value for parameter \"%s\": \"%s\"",
+ head->name, head->value)));
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.
+ * Mark all extant GUC variables as not present in the config file.
+ * We need this so that we can tell below which ones have been removed
+ * from the file since we last processed it.
*/
- 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;
+ {
+ struct config_generic *gconf = guc_variables[i];
+
+ gconf->status &= ~GUC_IS_IN_FILE;
+ }
+ /*
+ * 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.
+ */
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;
+ char *sep = strchr(item->name, GUC_QUALIFIER_SEPARATOR);
+
+ 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.
+ */
+ 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)
+ 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, false, false))
+ goto cleanup_list;
}
+ /*
+ * 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.)
+ */
for (i = 0; i < num_guc_variables; i++)
{
struct config_generic *gconf = guc_variables[i];
- if (!in_conffile[i] && gconf->source == PGC_S_FILE)
+ GucStack *stack;
+
+ if (gconf->reset_source != PGC_S_FILE ||
+ (gconf->status & GUC_IS_IN_FILE))
+ continue;
+ if (gconf->context < PGC_SIGHUP)
{
- if (gconf->context < PGC_SIGHUP)
- ereport(elevel,
+ 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);
- }
+ errmsg("parameter \"%s\" cannot be changed after server start; configuration file change ignored",
+ gconf->name)));
+ continue;
}
- 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;
- }
- }
+ /*
+ * Reset any "file" sources to "default", else set_config_option
+ * will not override those settings. tentative_source should
+ * never be "file".
+ */
+ if (gconf->reset_source == PGC_S_FILE)
+ gconf->reset_source = PGC_S_DEFAULT;
+ Assert(gconf->tentative_source != PGC_S_FILE);
+ if (gconf->source == PGC_S_FILE)
+ gconf->source = PGC_S_DEFAULT;
+ for (stack = gconf->stack; stack; stack = stack->prev)
+ {
+ Assert(stack->tentative_source != PGC_S_FILE);
+ if (stack->source == PGC_S_FILE)
+ stack->source = PGC_S_DEFAULT;
}
- }
- /* 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);
+ /* Now we can re-apply the wired-in default */
+ set_config_option(gconf->name, NULL, context, PGC_S_DEFAULT,
+ 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.
+ * Restore any variables determined by environment variables. This
+ * is a no-op except in the case where one of these had been in the
+ * config file and is now removed. PGC_S_ENV_VAR will override the
+ * wired-in default we just applied, but cannot override any other source.
+ * PGPORT can be ignored, because it cannot be changed without restart.
+ * Keep this list in sync with InitializeGUCOptions()!
*/
- var = getenv("PGDATESTYLE");
- if (var != NULL)
- set_config_option("datestyle", var, context,
+ envvar = getenv("PGDATESTYLE");
+ if (envvar != NULL)
+ set_config_option("datestyle", envvar, PGC_POSTMASTER,
PGC_S_ENV_VAR, false, true);
- var = getenv("PGCLIENTENCODING");
- if (var != NULL)
- set_config_option("client_encoding", var, context,
+ envvar = getenv("PGCLIENTENCODING");
+ if (envvar != NULL)
+ set_config_option("client_encoding", envvar, PGC_POSTMASTER,
PGC_S_ENV_VAR, false, true);
+
+ /* 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);
+ }
+
cleanup_list:
- free(in_conffile);
free_name_value_list(head);
+ if (cvc)
+ free(cvc);
}
@@ -389,6 +407,7 @@ ParseConfigFile(const char *config_file, const char *calling_file,
while ((token = yylex()))
{
char *opt_name, *opt_value;
+ struct name_value_pair *item;
if (token == GUC_EOL) /* empty or comment line */
continue;
@@ -421,7 +440,7 @@ ParseConfigFile(const char *config_file, const char *calling_file,
goto parse_error;
/* OK, process the option name and value */
- if (pg_strcasecmp(opt_name, "include") == 0)
+ if (guc_name_compare(opt_name, "include") == 0)
{
/*
* An include directive isn't a variable and should be processed
@@ -443,29 +462,39 @@ ParseConfigFile(const char *config_file, const char *calling_file,
pfree(opt_name);
pfree(opt_value);
}
- else if (pg_strcasecmp(opt_name, "custom_variable_classes") == 0)
+ else if (guc_name_compare(opt_name, "custom_variable_classes") == 0)
{
/*
* This variable must be processed first as it controls
- * the validity of other variables; so apply immediately.
+ * the validity of other variables; so it goes at the head
+ * of the result list. If we already found a value for it,
+ * replace with this one.
*/
- if (!set_config_option(opt_name, opt_value, context,
- PGC_S_FILE, false, true))
+ item = *head_p;
+ if (item != NULL &&
+ guc_name_compare(item->name, "custom_variable_classes") == 0)
{
- pfree(opt_name);
- pfree(opt_value);
-
- /* We assume the error message was logged already. */
- OK = false;
- goto cleanup_exit;
+ /* replace existing head item */
+ pfree(item->name);
+ pfree(item->value);
+ item->name = opt_name;
+ item->value = opt_value;
+ }
+ else
+ {
+ /* prepend to list */
+ item = palloc(sizeof *item);
+ item->name = opt_name;
+ item->value = opt_value;
+ item->next = *head_p;
+ *head_p = item;
+ if (*tail_p == NULL)
+ *tail_p = item;
}
}
-
- if (pg_strcasecmp(opt_name, "include") != 0)
+ else
{
- /* append to list */
- struct name_value_pair *item;
-
+ /* ordinary variable, append to list */
item = palloc(sizeof *item);
item->name = opt_name;
item->value = opt_value;
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 60f7ed5ca32..6cdc87df6db 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -10,7 +10,7 @@
* Written by Peter Eisentraut <peter_e@gmx.net>.
*
* IDENTIFICATION
- * $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.416 2007/09/03 18:46:30 tgl Exp $
+ * $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.417 2007/09/10 00:57:21 tgl Exp $
*
*--------------------------------------------------------------------
*/
@@ -2505,7 +2505,7 @@ static void ReportGUCOption(struct config_generic * record);
static void ShowGUCConfigOption(const char *name, DestReceiver *dest);
static void ShowAllGUCConfig(DestReceiver *dest);
static char *_ShowOption(struct config_generic * record, bool use_units);
-static bool is_newvalue_equal(struct config_generic * record, const char *newvalue);
+static bool is_newvalue_equal(struct config_generic *record, const char *newvalue);
/*
@@ -2690,40 +2690,6 @@ build_guc_variables(void)
sizeof(struct config_generic *), guc_var_compare);
}
-static bool
-is_custom_class(const char *name, int dotPos)
-{
- /*
- * assign_custom_variable_classes() has made sure no empty identifiers or
- * whitespace exists in the variable
- */
- bool result = false;
- const char *ccs = GetConfigOption("custom_variable_classes");
-
- if (ccs != NULL)
- {
- const char *start = ccs;
-
- for (;; ++ccs)
- {
- int c = *ccs;
-
- if (c == 0 || c == ',')
- {
- if (dotPos == ccs - start && strncmp(start, name, dotPos) == 0)
- {
- result = true;
- break;
- }
- if (c == 0)
- break;
- start = ccs + 1;
- }
- }
- }
- return result;
-}
-
/*
* Add a new GUC variable to the list of known variables. The
* list is expanded if needed.
@@ -2767,7 +2733,7 @@ add_guc_variable(struct config_generic * var, int elevel)
* Create and add a placeholder variable. It's presumed to belong
* to a valid custom variable class at this point.
*/
-static struct config_string *
+static struct config_generic *
add_placeholder_variable(const char *name, int elevel)
{
size_t sz = sizeof(struct config_string) + sizeof(char *);
@@ -2777,9 +2743,8 @@ add_placeholder_variable(const char *name, int elevel)
var = (struct config_string *) guc_malloc(elevel, sz);
if (var == NULL)
return NULL;
-
- gen = &var->gen;
memset(var, 0, sz);
+ gen = &var->gen;
gen->name = guc_strdup(elevel, name);
if (gen->name == NULL)
@@ -2796,7 +2761,8 @@ add_placeholder_variable(const char *name, int elevel)
/*
* The char* is allocated at the end of the struct since we have no
- * 'static' place to point to.
+ * 'static' place to point to. Note that the current value, as well
+ * as the boot and reset values, start out NULL.
*/
var->variable = (char **) (var + 1);
@@ -2807,17 +2773,53 @@ add_placeholder_variable(const char *name, int elevel)
return NULL;
}
- return var;
+ return gen;
+}
+
+/*
+ * Detect whether the portion of "name" before dotPos matches any custom
+ * variable class name listed in custom_var_classes. The latter must be
+ * formatted the way that assign_custom_variable_classes does it, ie,
+ * no whitespace. NULL is valid for custom_var_classes.
+ */
+static bool
+is_custom_class(const char *name, int dotPos, const char *custom_var_classes)
+{
+ bool result = false;
+ const char *ccs = custom_var_classes;
+
+ if (ccs != NULL)
+ {
+ const char *start = ccs;
+
+ for (;; ++ccs)
+ {
+ char c = *ccs;
+
+ if (c == '\0' || c == ',')
+ {
+ if (dotPos == ccs - start && strncmp(start, name, dotPos) == 0)
+ {
+ result = true;
+ break;
+ }
+ if (c == '\0')
+ break;
+ start = ccs + 1;
+ }
+ }
+ }
+ return result;
}
/*
- * Look up option NAME. If it exists, return a pointer to its record,
- * else return NULL.
+ * Look up option NAME. If it exists, return a pointer to its record,
+ * else return NULL. If create_placeholders is TRUE, we'll create a
+ * placeholder record for a valid-looking custom variable name.
*/
static struct config_generic *
-find_option(const char *name, int elevel)
+find_option(const char *name, bool create_placeholders, int elevel)
{
- const char *dot;
const char **key = &name;
struct config_generic **res;
int i;
@@ -2844,17 +2846,21 @@ find_option(const char *name, int elevel)
for (i = 0; map_old_guc_names[i] != NULL; i += 2)
{
if (guc_name_compare(name, map_old_guc_names[i]) == 0)
- return find_option(map_old_guc_names[i + 1], elevel);
+ return find_option(map_old_guc_names[i + 1], false, elevel);
}
- /*
- * Check if the name is qualified, and if so, check if the qualifier maps
- * to a custom variable class.
- */
- dot = strchr(name, GUC_QUALIFIER_SEPARATOR);
- if (dot != NULL && is_custom_class(name, dot - name))
- /* Add a placeholder variable for this name */
- return (struct config_generic *) add_placeholder_variable(name, elevel);
+ if (create_placeholders)
+ {
+ /*
+ * Check if the name is qualified, and if so, check if the qualifier
+ * matches any custom variable class. If so, add a placeholder.
+ */
+ const char *dot = strchr(name, GUC_QUALIFIER_SEPARATOR);
+
+ if (dot != NULL &&
+ is_custom_class(name, dot - name, custom_variable_classes))
+ return add_placeholder_variable(name, elevel);
+ }
/* Unknown name */
return NULL;
@@ -2902,30 +2908,6 @@ guc_name_compare(const char *namea, const char *nameb)
}
-static int
-guc_get_index(const char *name)
-{
- const char **key = &name;
- struct config_generic **res;
-
- Assert(name);
-
- /*
- * By equating const char ** with struct config_generic *, we are assuming
- * the name field is first in config_generic.
- */
- res = (struct config_generic **) bsearch((void *) &key,
- (void *) guc_variables,
- num_guc_variables,
- sizeof(struct config_generic *),
- guc_var_compare);
- if (!res)
- return -1;
-
- return res - guc_variables;
-}
-
-
/*
* Initialize GUC options during program startup.
*
@@ -3017,7 +2999,7 @@ InitializeGUCOptions(void)
if (conf->boot_val == NULL)
{
- /* Cannot set value yet */
+ /* leave the value NULL, do not call assign hook */
break;
}
@@ -3067,7 +3049,8 @@ InitializeGUCOptions(void)
/*
* For historical reasons, some GUC parameters can receive defaults from
- * environment variables. Process those settings.
+ * environment variables. Process those settings. NB: if you add or
+ * remove anything here, see also ProcessConfigFile().
*/
env = getenv("PGPORT");
@@ -3335,16 +3318,10 @@ ResetAllOptions(void)
struct config_string *conf = (struct config_string *) gconf;
char *str;
- if (conf->reset_val == NULL)
- {
- /* Nothing to reset to, as yet; so do nothing */
- break;
- }
-
/* We need not strdup here */
str = conf->reset_val;
- if (conf->assign_hook)
+ if (conf->assign_hook && str)
{
const char *newstr;
@@ -3517,7 +3494,9 @@ AtEOXact_GUC(bool isCommit, int nestLevel)
/*
* Skip if nothing's happened to this var in this transaction
*/
- if (my_status == 0)
+ if ((my_status & (GUC_HAVE_TENTATIVE |
+ GUC_HAVE_LOCAL |
+ GUC_HAVE_STACK)) == 0)
{
Assert(stack == NULL);
continue;
@@ -3679,7 +3658,7 @@ AtEOXact_GUC(bool isCommit, int nestLevel)
if (*conf->variable != newval)
{
- if (conf->assign_hook)
+ if (conf->assign_hook && newval)
{
const char *newstr;
@@ -4182,7 +4161,7 @@ set_config_option(const char *name, const char *value,
else
elevel = ERROR;
- record = find_option(name, elevel);
+ record = find_option(name, true, elevel);
if (record == NULL)
{
ereport(elevel,
@@ -4192,11 +4171,13 @@ set_config_option(const char *name, const char *value,
}
/*
- * Do not replace a value that has been set on the command line by a SIGHUP
- * reload
+ * 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 (context == PGC_SIGHUP && record->source == PGC_S_ARGV)
- return true;
+ 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
@@ -4220,12 +4201,17 @@ set_config_option(const char *name, const char *value,
case PGC_POSTMASTER:
if (context == PGC_SIGHUP)
{
+ /*
+ * We are reading a PGC_POSTMASTER var from postgresql.conf.
+ * We can't change the setting, so give a warning if the DBA
+ * tries to change it. (Throwing an error would be more
+ * consistent, but seems overly rigid.)
+ */
if (changeVal && !is_newvalue_equal(record, value))
ereport(elevel,
(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
errmsg("parameter \"%s\" cannot be changed after server start; configuration file change ignored",
name)));
-
return true;
}
if (context != PGC_POSTMASTER)
@@ -4298,22 +4284,17 @@ set_config_option(const char *name, const char *value,
* value from the database's/user's/client's default settings or
* when we reset a value to its default.
*/
- makeDefault = changeVal && (source <= PGC_S_OVERRIDE)
- && ((value != NULL) || source == PGC_S_DEFAULT);
+ makeDefault = changeVal && (source <= PGC_S_OVERRIDE) &&
+ ((value != NULL) || 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. 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 and context == PGC_SIGHUP.
+ * 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 && context == PGC_SIGHUP))
+ if (record->source > source)
{
if (changeVal && !makeDefault)
{
@@ -4326,6 +4307,9 @@ set_config_option(const char *name, const char *value,
/*
* Evaluate value and set variable.
+ *
+ * Note: if value == NULL then we are supposed to set to the reset_val,
+ * except when source == PGC_S_DEFAULT; then we set to the boot_val.
*/
switch (record->vartype)
{
@@ -4345,14 +4329,8 @@ 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;
@@ -4440,14 +4418,8 @@ 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;
@@ -4532,14 +4504,8 @@ 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;
@@ -4618,11 +4584,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)
@@ -4634,25 +4595,25 @@ set_config_option(const char *name, const char *value,
return false;
}
}
- /* else we handle a "RESET varname" command */
- else if (conf->reset_val)
+ else
{
/*
* We could possibly avoid strdup here, but easier to make
- * this case work the same as the normal assignment case.
+ * this case work the same as the normal assignment case;
+ * note the possible free of newval below.
*/
- newval = guc_strdup(elevel, conf->reset_val);
- if (newval == NULL)
- return false;
+ if (conf->reset_val == NULL)
+ newval = NULL;
+ else
+ {
+ newval = guc_strdup(elevel, conf->reset_val);
+ if (newval == NULL)
+ return false;
+ }
source = conf->gen.reset_source;
}
- else
- {
- /* Nothing to reset to, as yet; so do nothing */
- break;
- }
- if (conf->assign_hook)
+ if (conf->assign_hook && newval)
{
const char *hookresult;
@@ -4718,7 +4679,7 @@ set_config_option(const char *name, const char *value,
}
}
/* Perhaps we didn't install newval anywhere */
- if (!string_field_used(conf, newval))
+ if (newval && !string_field_used(conf, newval))
free(newval);
}
else if (isLocal)
@@ -4734,7 +4695,7 @@ set_config_option(const char *name, const char *value,
guc_dirty = true;
}
}
- else
+ else if (newval)
free(newval);
break;
}
@@ -4774,7 +4735,7 @@ GetConfigOption(const char *name)
struct config_generic *record;
static char buffer[256];
- record = find_option(name, ERROR);
+ record = find_option(name, false, ERROR);
if (record == NULL)
ereport(ERROR,
(errcode(ERRCODE_UNDEFINED_OBJECT),
@@ -4807,6 +4768,10 @@ GetConfigOption(const char *name)
/*
* Get the RESET value associated with the given option.
+ *
+ * Note: this is not re-entrant, due to use of static result buffer;
+ * not to mention that a string variable could have its reset_val changed.
+ * Beware of assuming the result value is good for very long.
*/
const char *
GetConfigOptionResetString(const char *name)
@@ -4814,7 +4779,7 @@ GetConfigOptionResetString(const char *name)
struct config_generic *record;
static char buffer[256];
- record = find_option(name, ERROR);
+ record = find_option(name, false, ERROR);
if (record == NULL)
ereport(ERROR,
(errcode(ERRCODE_UNDEFINED_OBJECT),
@@ -4854,7 +4819,7 @@ IsSuperuserConfigOption(const char *name)
{
struct config_generic *record;
- record = find_option(name, ERROR);
+ record = find_option(name, false, ERROR);
/* On an unrecognized name, don't error, just return false. */
if (record == NULL)
return false;
@@ -4886,7 +4851,7 @@ flatten_set_variable_args(const char *name, List *args)
return NULL;
/* Else get flags for the variable */
- record = find_option(name, ERROR);
+ record = find_option(name, true, ERROR);
if (record == NULL)
ereport(ERROR,
(errcode(ERRCODE_UNDEFINED_OBJECT),
@@ -5145,22 +5110,54 @@ set_config_by_name(PG_FUNCTION_ARGS)
}
+/*
+ * Common code for DefineCustomXXXVariable subroutines: allocate the
+ * new variable's config struct and fill in generic fields.
+ */
+static struct config_generic *
+init_custom_variable(const char *name,
+ const char *short_desc,
+ const char *long_desc,
+ GucContext context,
+ enum config_type type,
+ size_t sz)
+{
+ struct config_generic *gen;
+
+ gen = (struct config_generic *) guc_malloc(ERROR, sz);
+ memset(gen, 0, sz);
+
+ gen->name = guc_strdup(ERROR, name);
+ gen->context = context;
+ gen->group = CUSTOM_OPTIONS;
+ gen->short_desc = short_desc;
+ gen->long_desc = long_desc;
+ gen->vartype = type;
+
+ return gen;
+}
+
+/*
+ * Common code for DefineCustomXXXVariable subroutines: insert the new
+ * variable into the GUC variable array, replacing any placeholder.
+ */
static void
-define_custom_variable(struct config_generic * variable)
+define_custom_variable(struct config_generic *variable)
{
const char *name = variable->name;
const char **nameAddr = &name;
const char *value;
struct config_string *pHolder;
- struct config_generic **res = (struct config_generic **) bsearch(
- (void *) &nameAddr,
- (void *) guc_variables,
- num_guc_variables,
- sizeof(struct config_generic *),
- guc_var_compare);
+ struct config_generic **res;
+ res = (struct config_generic **) bsearch((void *) &nameAddr,
+ (void *) guc_variables,
+ num_guc_variables,
+ sizeof(struct config_generic *),
+ guc_var_compare);
if (res == NULL)
{
+ /* No placeholder to replace, so just add it */
add_guc_variable(variable, ERROR);
return;
}
@@ -5174,13 +5171,14 @@ define_custom_variable(struct config_generic * variable)
errmsg("attempt to redefine parameter \"%s\"", name)));
Assert((*res)->vartype == PGC_STRING);
- pHolder = (struct config_string *) * res;
+ pHolder = (struct config_string *) (*res);
- /* We have the same name, no sorting is necessary */
+ /*
+ * Replace the placeholder.
+ * We aren't changing the name, so no re-sorting is necessary
+ */
*res = variable;
- value = *pHolder->variable;
-
/*
* Assign the string value stored in the placeholder to the real variable.
*
@@ -5188,9 +5186,12 @@ define_custom_variable(struct config_generic * variable)
* assignment, since we don't want it to roll back if the current xact
* fails later.
*/
- set_config_option(name, value,
- pHolder->gen.context, pHolder->gen.source,
- false, true);
+ value = *pHolder->variable;
+
+ if (value)
+ set_config_option(name, value,
+ pHolder->gen.context, pHolder->gen.source,
+ false, true);
/*
* Free up as much as we conveniently can of the placeholder structure
@@ -5203,22 +5204,6 @@ define_custom_variable(struct config_generic * variable)
free(pHolder);
}
-static void
-init_custom_variable(struct config_generic * gen,
- const char *name,
- const char *short_desc,
- const char *long_desc,
- GucContext context,
- enum config_type type)
-{
- gen->name = guc_strdup(ERROR, name);
- gen->context = context;
- gen->group = CUSTOM_OPTIONS;
- gen->short_desc = short_desc;
- gen->long_desc = long_desc;
- gen->vartype = type;
-}
-
void
DefineCustomBoolVariable(const char *name,
const char *short_desc,
@@ -5228,13 +5213,13 @@ DefineCustomBoolVariable(const char *name,
GucBoolAssignHook assign_hook,
GucShowHook show_hook)
{
- size_t sz = sizeof(struct config_bool);
- struct config_bool *var = (struct config_bool *) guc_malloc(ERROR, sz);
-
- memset(var, 0, sz);
- init_custom_variable(&var->gen, name, short_desc, long_desc, context, PGC_BOOL);
+ struct config_bool *var;
+ var = (struct config_bool *)
+ init_custom_variable(name, short_desc, long_desc, context,
+ PGC_BOOL, sizeof(struct config_bool));
var->variable = valueAddr;
+ var->boot_val = *valueAddr;
var->reset_val = *valueAddr;
var->assign_hook = assign_hook;
var->show_hook = show_hook;
@@ -5252,13 +5237,13 @@ DefineCustomIntVariable(const char *name,
GucIntAssignHook assign_hook,
GucShowHook show_hook)
{
- size_t sz = sizeof(struct config_int);
- struct config_int *var = (struct config_int *) guc_malloc(ERROR, sz);
-
- memset(var, 0, sz);
- init_custom_variable(&var->gen, name, short_desc, long_desc, context, PGC_INT);
+ struct config_int *var;
+ var = (struct config_int *)
+ init_custom_variable(name, short_desc, long_desc, context,
+ PGC_INT, sizeof(struct config_int));
var->variable = valueAddr;
+ var->boot_val = *valueAddr;
var->reset_val = *valueAddr;
var->min = minValue;
var->max = maxValue;
@@ -5278,13 +5263,13 @@ DefineCustomRealVariable(const char *name,
GucRealAssignHook assign_hook,
GucShowHook show_hook)
{
- size_t sz = sizeof(struct config_real);
- struct config_real *var = (struct config_real *) guc_malloc(ERROR, sz);
-
- memset(var, 0, sz);
- init_custom_variable(&var->gen, name, short_desc, long_desc, context, PGC_REAL);
+ struct config_real *var;
+ var = (struct config_real *)
+ init_custom_variable(name, short_desc, long_desc, context,
+ PGC_REAL, sizeof(struct config_real));
var->variable = valueAddr;
+ var->boot_val = *valueAddr;
var->reset_val = *valueAddr;
var->min = minValue;
var->max = maxValue;
@@ -5302,14 +5287,16 @@ DefineCustomStringVariable(const char *name,
GucStringAssignHook assign_hook,
GucShowHook show_hook)
{
- size_t sz = sizeof(struct config_string);
- struct config_string *var = (struct config_string *) guc_malloc(ERROR, sz);
-
- memset(var, 0, sz);
- init_custom_variable(&var->gen, name, short_desc, long_desc, context, PGC_STRING);
+ struct config_string *var;
+ var = (struct config_string *)
+ init_custom_variable(name, short_desc, long_desc, context,
+ PGC_STRING, sizeof(struct config_string));
var->variable = valueAddr;
- var->reset_val = *valueAddr;
+ var->boot_val = *valueAddr;
+ /* we could probably do without strdup, but keep it like normal case */
+ if (var->boot_val)
+ var->reset_val = guc_strdup(ERROR, var->boot_val);
var->assign_hook = assign_hook;
var->show_hook = show_hook;
define_custom_variable(&var->gen);
@@ -5345,7 +5332,7 @@ EmitWarningsOnPlaceholders(const char *className)
void
GetPGVariable(const char *name, DestReceiver *dest)
{
- if (pg_strcasecmp(name, "all") == 0)
+ if (guc_name_compare(name, "all") == 0)
ShowAllGUCConfig(dest);
else
ShowGUCConfigOption(name, dest);
@@ -5356,7 +5343,7 @@ GetPGVariableResultDesc(const char *name)
{
TupleDesc tupdesc;
- if (pg_strcasecmp(name, "all") == 0)
+ if (guc_name_compare(name, "all") == 0)
{
/* need a tuple descriptor representing three TEXT columns */
tupdesc = CreateTemplateTupleDesc(3, false);
@@ -5470,7 +5457,7 @@ GetConfigOptionByName(const char *name, const char **varname)
{
struct config_generic *record;
- record = find_option(name, ERROR);
+ record = find_option(name, false, ERROR);
if (record == NULL)
ereport(ERROR,
(errcode(ERRCODE_UNDEFINED_OBJECT),
@@ -5927,9 +5914,19 @@ _ShowOption(struct config_generic * record, bool use_units)
}
+/*
+ * Attempt (badly) to detect if a proposed new GUC setting is the same
+ * as the current value.
+ *
+ * XXX this does not really work because it doesn't account for the
+ * effects of canonicalization of string values by assign_hooks.
+ */
static bool
-is_newvalue_equal(struct config_generic * record, const char *newvalue)
+is_newvalue_equal(struct config_generic *record, const char *newvalue)
{
+ /* newvalue == NULL isn't supported */
+ Assert(newvalue != NULL);
+
switch (record->vartype)
{
case PGC_BOOL:
@@ -5960,7 +5957,8 @@ is_newvalue_equal(struct config_generic * record, const char *newvalue)
{
struct config_string *conf = (struct config_string *) record;
- return strcmp(*conf->variable, newvalue) == 0;
+ return *conf->variable != NULL &&
+ strcmp(*conf->variable, newvalue) == 0;
}
}
@@ -6140,7 +6138,7 @@ read_nondefault_variables(void)
if ((varname = read_string_with_null(fp)) == NULL)
break;
- if ((record = find_option(varname, FATAL)) == NULL)
+ if ((record = find_option(varname, true, FATAL)) == NULL)
elog(FATAL, "failed to locate variable %s in exec config params file", varname);
if ((varvalue = read_string_with_null(fp)) == NULL)
elog(FATAL, "invalid format of exec config params file");
@@ -6576,8 +6574,7 @@ assign_session_replication_role(const char *newval, bool doit, GucSource source)
}
static const char *
-assign_log_min_messages(const char *newval,
- bool doit, GucSource source)
+assign_log_min_messages(const char *newval, bool doit, GucSource source)
{
return (assign_msglvl(&log_min_messages, newval, doit, source));
}
@@ -6756,23 +6753,16 @@ assign_custom_variable_classes(const char *newval, bool doit, GucSource source)
{
/*
* Check syntax. newval must be a comma separated list of identifiers.
- * Whitespace is allowed but skipped.
+ * Whitespace is allowed but removed from the result.
*/
bool hasSpaceAfterToken = false;
const char *cp = newval;
int symLen = 0;
- int c;
+ char 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)
+ while ((c = *cp++) != '\0')
{
if (isspace((unsigned char) c))
{
@@ -6783,12 +6773,12 @@ assign_custom_variable_classes(const char *newval, bool doit, GucSource source)
if (c == ',')
{
- hasSpaceAfterToken = false;
- if (symLen > 0)
+ if (symLen > 0) /* terminate identifier */
{
- symLen = 0;
appendStringInfoChar(&buf, ',');
+ symLen = 0;
}
+ hasSpaceAfterToken = false;
continue;
}
@@ -6798,24 +6788,19 @@ assign_custom_variable_classes(const char *newval, bool doit, GucSource source)
* Syntax error due to token following space after token or non
* alpha numeric character
*/
- ereport(LOG,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("invalid syntax for \"custom_variable_classes\": \"%s\"", newval)));
pfree(buf.data);
return NULL;
}
+ appendStringInfoChar(&buf, c);
symLen++;
- appendStringInfoChar(&buf, (char) c);
}
/* Remove stray ',' at end */
if (symLen == 0 && buf.len > 0)
buf.data[--buf.len] = '\0';
- if (buf.len == 0)
- newval = NULL;
- else if (doit)
- newval = guc_strdup(ERROR, buf.data);
+ /* GUC wants the result malloc'd */
+ newval = guc_strdup(LOG, buf.data);
pfree(buf.data);
return newval;
diff --git a/src/include/utils/guc_tables.h b/src/include/utils/guc_tables.h
index f68a814f799..0bc74ccdd0b 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.33 2007/04/21 20:02:41 petere Exp $
+ * $PostgreSQL: pgsql/src/include/utils/guc_tables.h,v 1.34 2007/09/10 00:57:22 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -130,7 +130,7 @@ struct config_generic
#define GUC_SUPERUSER_ONLY 0x0100 /* show only to superusers */
#define GUC_IS_NAME 0x0200 /* limit string to NAMEDATALEN-1 */
-#define GUC_UNIT_KB 0x0400 /* value is in 1 kB */
+#define GUC_UNIT_KB 0x0400 /* value is in kilobytes */
#define GUC_UNIT_BLOCKS 0x0800 /* value is in blocks */
#define GUC_UNIT_XBLOCKS 0x0C00 /* value is in xlog blocks */
#define GUC_UNIT_MEMORY 0x0C00 /* mask for KB, BLOCKS, XBLOCKS */
@@ -144,6 +144,11 @@ struct config_generic
#define GUC_HAVE_TENTATIVE 0x0001 /* tentative value is defined */
#define GUC_HAVE_LOCAL 0x0002 /* a SET LOCAL has been executed */
#define GUC_HAVE_STACK 0x0004 /* we have stacked prior value(s) */
+#define GUC_IS_IN_FILE 0x0008 /* found it in config file */
+/*
+ * Caution: the GUC_IS_IN_FILE bit is transient state for ProcessConfigFile.
+ * Do not assume that its value represents useful information elsewhere.
+ */
/* GUC records for specific variable types */
@@ -151,8 +156,7 @@ struct config_generic
struct config_bool
{
struct config_generic gen;
- /* these fields must be set correctly in initial value: */
- /* (all but reset_val are constants) */
+ /* constant fields, must be set correctly in initial value: */
bool *variable;
bool boot_val;
GucBoolAssignHook assign_hook;
@@ -165,8 +169,7 @@ struct config_bool
struct config_int
{
struct config_generic gen;
- /* these fields must be set correctly in initial value: */
- /* (all but reset_val are constants) */
+ /* constant fields, must be set correctly in initial value: */
int *variable;
int boot_val;
int min;
@@ -181,8 +184,7 @@ struct config_int
struct config_real
{
struct config_generic gen;
- /* these fields must be set correctly in initial value: */
- /* (all but reset_val are constants) */
+ /* constant fields, must be set correctly in initial value: */
double *variable;
double boot_val;
double min;
@@ -197,8 +199,7 @@ struct config_real
struct config_string
{
struct config_generic gen;
- /* these fields must be set correctly in initial value: */
- /* (all are constants) */
+ /* constant fields, must be set correctly in initial value: */
char **variable;
const char *boot_val;
GucStringAssignHook assign_hook;
--
GitLab