From e05b866447899211a0c2df31bf0671faac4fc3e5 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 11 May 2011 19:57:38 -0400 Subject: [PATCH] Split PGC_S_DEFAULT into two values, for true boot_val vs computed default. Failure to distinguish these cases is the real cause behind the recent reports of Windows builds crashing on 'infinity'::timestamp, which was directly due to failure to establish a value of timezone_abbreviations in postmaster child processes. The postmaster had the desired value, but write_one_nondefault_variable() didn't transmit it to backends. To fix that, invent a new value PGC_S_DYNAMIC_DEFAULT, and be sure to use that or PGC_S_ENV_VAR (as appropriate) for "default" settings that are computed during initialization. (We need both because there's at least one variable that could receive a value from either source.) This commit also fixes ProcessConfigFile's failure to restore the correct default value for certain GUC variables if they are set in postgresql.conf and then removed/commented out of the file. We have to recompute and reinstall the value for any GUC variable that could have received a value from PGC_S_DYNAMIC_DEFAULT or PGC_S_ENV_VAR sources, and there were a number of oversights. (That whole thing is a crock that needs to be redesigned, but not today.) However, I intentionally didn't make it work "exactly right" for the cases of timezone and log_timezone. The exactly right behavior would involve running select_default_timezone, which we'd have to do independently in each postgres process, causing the whole database to become entirely unresponsive for as much as several seconds. That didn't seem like a good idea, especially since the variable's removal from postgresql.conf might be just an accidental edit. Instead the behavior is to adopt the previously active setting as if it were default. Note that this patch creates an ABI break for extensions that use any of the PGC_S_XXX constants; they'll need to be recompiled. --- src/backend/utils/init/postinit.c | 2 +- src/backend/utils/misc/guc-file.l | 41 +++++++++++++++++-------------- src/backend/utils/misc/guc.c | 33 ++++++++++++++++++------- src/include/utils/guc.h | 3 ++- src/timezone/pgtz.c | 37 ++++++++++++++++++++-------- 5 files changed, 76 insertions(+), 40 deletions(-) diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c index 1f6fba5f75..3ac3254afb 100644 --- a/src/backend/utils/init/postinit.c +++ b/src/backend/utils/init/postinit.c @@ -324,7 +324,7 @@ CheckMyDatabase(const char *name, bool am_superuser) PGC_INTERNAL, PGC_S_OVERRIDE); /* If we have no other source of client_encoding, use server encoding */ SetConfigOption("client_encoding", GetDatabaseEncodingName(), - PGC_BACKEND, PGC_S_DEFAULT); + PGC_BACKEND, PGC_S_DYNAMIC_DEFAULT); /* assign locale variables */ collate = NameStr(dbform->datcollate); diff --git a/src/backend/utils/misc/guc-file.l b/src/backend/utils/misc/guc-file.l index 10ef12eb24..78907b939d 100644 --- a/src/backend/utils/misc/guc-file.l +++ b/src/backend/utils/misc/guc-file.l @@ -14,6 +14,7 @@ #include #include +#include "mb/pg_wchar.h" #include "miscadmin.h" #include "storage/fd.h" #include "utils/guc.h" @@ -109,7 +110,6 @@ ProcessConfigFile(GucContext context) *tail; char *cvc = NULL; struct config_string *cvc_struct; - const char *envvar; int i; Assert(context == PGC_POSTMASTER || context == PGC_SIGHUP); @@ -265,7 +265,7 @@ ProcessConfigFile(GucContext context) stack->source = PGC_S_DEFAULT; } - /* Now we can re-apply the wired-in default */ + /* 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) @@ -275,25 +275,28 @@ ProcessConfigFile(GucContext context) } /* - * 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. + * Restore any variables determined by environment variables or + * dynamically-computed defaults. This is a no-op except in the case + * where one of these had been in the config file and is now removed. * - * Keep this list in sync with InitializeGUCOptions()! - * PGPORT can be ignored, because it cannot be changed without restart. - * We assume rlimit hasn't changed, either. + * In particular, we *must not* do this during the postmaster's + * initial loading of the file, since the timezone functions in + * particular should be run only after initialization is complete. + * + * XXX this is an unmaintainable crock, because we have to know how + * to set (or at least what to call to set) every variable that could + * potentially have PGC_S_DYNAMIC_DEFAULT or PGC_S_ENV_VAR source. + * However, there's no time to redesign it for 9.1. */ - envvar = getenv("PGDATESTYLE"); - if (envvar != NULL) - set_config_option("datestyle", envvar, PGC_POSTMASTER, - PGC_S_ENV_VAR, GUC_ACTION_SET, true); - - envvar = getenv("PGCLIENTENCODING"); - if (envvar != NULL) - set_config_option("client_encoding", envvar, PGC_POSTMASTER, - PGC_S_ENV_VAR, GUC_ACTION_SET, true); - + if (context == PGC_SIGHUP) + { + InitializeGUCOptionsFromEnvironment(); + pg_timezone_initialize(); + pg_timezone_abbrev_initialize(); + /* this selects SQL_ASCII in processes not connected to a database */ + SetConfigOption("client_encoding", GetDatabaseEncodingName(), + PGC_BACKEND, PGC_S_DYNAMIC_DEFAULT); + } /* If we got here all the options checked out okay, so apply them. */ for (item = head; item; item = item->next) diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index 738e2152ba..92391eda2f 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -502,6 +502,7 @@ const char *const GucContext_Names[] = const char *const GucSource_Names[] = { /* PGC_S_DEFAULT */ "default", + /* PGC_S_DYNAMIC_DEFAULT */ "default", /* PGC_S_ENV_VAR */ "environment variable", /* PGC_S_FILE */ "configuration file", /* PGC_S_ARGV */ "command line", @@ -3269,6 +3270,7 @@ static int GUCNestLevel = 0; /* 1 when in main transaction */ static int guc_var_compare(const void *a, const void *b); static int guc_name_compare(const char *namea, const char *nameb); +static void InitializeGUCOptionsFromEnvironment(void); static void InitializeOneGUCOption(struct config_generic * gconf); static void push_old_value(struct config_generic * gconf, GucAction action); static void ReportGUCOption(struct config_generic * record); @@ -3812,8 +3814,6 @@ void InitializeGUCOptions(void) { int i; - char *env; - long stack_rlimit; /* * Before log_line_prefix could possibly receive a nonempty setting, make @@ -3852,9 +3852,25 @@ InitializeGUCOptions(void) /* * For historical reasons, some GUC parameters can receive defaults from - * environment variables. Process those settings. NB: if you add or - * remove anything here, see also ProcessConfigFile(). + * environment variables. Process those settings. */ + InitializeGUCOptionsFromEnvironment(); +} + +/* + * Assign any GUC values that can come from the server's environment. + * + * This is called from InitializeGUCOptions, and also from ProcessConfigFile + * to deal with the possibility that a setting has been removed from + * postgresql.conf and should now get a value from the environment. + * (The latter is a kludge that should probably go away someday; if so, + * fold this back into InitializeGUCOptions.) + */ +static void +InitializeGUCOptionsFromEnvironment(void) +{ + char *env; + long stack_rlimit; env = getenv("PGPORT"); if (env != NULL) @@ -6334,6 +6350,7 @@ define_custom_variable(struct config_generic * variable) switch (pHolder->gen.source) { case PGC_S_DEFAULT: + case PGC_S_DYNAMIC_DEFAULT: case PGC_S_ENV_VAR: case PGC_S_FILE: case PGC_S_ARGV: @@ -8420,15 +8437,13 @@ assign_timezone_abbreviations(const char *newval, void *extra) * * This is called after initial loading of postgresql.conf. If no * timezone_abbreviations setting was found therein, select default. + * If a non-default value is already installed, nothing will happen. */ void pg_timezone_abbrev_initialize(void) { - if (timezone_abbreviations_string == NULL) - { - SetConfigOption("timezone_abbreviations", "Default", - PGC_POSTMASTER, PGC_S_DEFAULT); - } + SetConfigOption("timezone_abbreviations", "Default", + PGC_POSTMASTER, PGC_S_DYNAMIC_DEFAULT); } static const char * diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h index 5a42d8cec3..ee52cd735e 100644 --- a/src/include/utils/guc.h +++ b/src/include/utils/guc.h @@ -82,7 +82,8 @@ typedef enum */ typedef enum { - PGC_S_DEFAULT, /* wired-in default */ + PGC_S_DEFAULT, /* hard-wired default ("boot_val") */ + PGC_S_DYNAMIC_DEFAULT, /* default computed during initialization */ PGC_S_ENV_VAR, /* postmaster environment variable */ PGC_S_FILE, /* postgresql.conf */ PGC_S_ARGV, /* postmaster command line */ diff --git a/src/timezone/pgtz.c b/src/timezone/pgtz.c index cb66f6380b..622cf94c5a 100644 --- a/src/timezone/pgtz.c +++ b/src/timezone/pgtz.c @@ -1438,6 +1438,10 @@ pg_timezone_pre_initialize(void) * This is called after initial loading of postgresql.conf. If no TimeZone * setting was found therein, we try to derive one from the environment. * Likewise for log_timezone. + * + * Note: this is also called from ProcessConfigFile, to re-establish valid + * GUC settings if the GUCs have been reset to default following their + * removal from postgresql.conf. */ void pg_timezone_initialize(void) @@ -1463,21 +1467,34 @@ pg_timezone_initialize(void) log_timezone = def_tz; } - /* Now, set the timezone GUC if it's not already set */ - if (GetConfigOption("timezone", false) == NULL) - { - /* Tell GUC about the value. Will redundantly call pg_tzset() */ + /* + * Now, set the timezone and log_timezone GUCs if they're still default. + * (This will redundantly call pg_tzset().) + * + * We choose to label these values PGC_S_ENV_VAR, rather than + * PGC_S_DYNAMIC_DEFAULT which would be functionally equivalent, because + * they came either from getenv("TZ") or from libc behavior that's + * determined by process environment of some kind. + * + * Note: in the case where a setting has just been removed from + * postgresql.conf, this code will not do what you might expect, namely + * call select_default_timezone() and install that value as the setting. + * Rather, the previously active setting --- typically the one from + * postgresql.conf --- will be reinstalled, relabeled as PGC_S_ENV_VAR. + * If we did try to install the "correct" default value, the effect would + * be that each postmaster child would independently run an extremely + * expensive search of the timezone database, bringing the database to its + * knees for possibly multiple seconds. This is so unpleasant, and could + * so easily be triggered quite unintentionally, that it seems better to + * violate the principle of least astonishment. + */ + if (GetConfigOptionResetString("timezone") == NULL) SetConfigOption("timezone", pg_get_timezone_name(session_timezone), PGC_POSTMASTER, PGC_S_ENV_VAR); - } - /* Likewise for log timezone */ - if (GetConfigOption("log_timezone", false) == NULL) - { - /* Tell GUC about the value. Will redundantly call pg_tzset() */ + if (GetConfigOptionResetString("log_timezone") == NULL) SetConfigOption("log_timezone", pg_get_timezone_name(log_timezone), PGC_POSTMASTER, PGC_S_ENV_VAR); - } }