Fix another oversight in logging of changes in postgresql.conf settings.

tglsfdc · tglsfdc · commit 9a9e530713d3 · 2011-07-08T17:03:12.000-04:00
We were using GetConfigOption to collect the old value of each setting,
overlooking the possibility that it didn't exist yet.  This does happen
in the case of adding a new entry within a custom variable class, as
exhibited in bug #6097 from Maxim Boguk.

To fix, add a missing_ok parameter to GetConfigOption, but only in 9.1
and HEAD --- it seems possible that some third-party code is using that
function, so changing its API in a minor release would cause problems.
In 9.0, create a near-duplicate function instead.
diff --git a/src/backend/utils/misc/guc-file.l b/src/backend/utils/misc/guc-file.l
@@ -317,9 +317,9 @@ ProcessConfigFile(GucContext context)
 		/* In SIGHUP cases in the postmaster, report changes */
 		if (context == PGC_SIGHUP && !IsUnderPostmaster)
 		{
-			const char *preval = GetConfigOption(item->name, false);
+			const char *preval = GetConfigOptionNoError(item->name);
 
-			/* string variables could be NULL; treat that as empty */
+			/* If option doesn't exist yet or is NULL, treat as empty string */
 			if (!preval)
 				preval = "";
 			/* must dup, else might have dangling pointer below */
@@ -334,7 +334,7 @@ ProcessConfigFile(GucContext context)
 
 			if (pre_value)
 			{
-				const char *post_value = GetConfigOption(item->name, false);
+				const char *post_value = GetConfigOptionNoError(item->name);
 
 				if (!post_value)
 					post_value = "";
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
@@ -5391,6 +5391,46 @@ GetConfigOption(const char *name, bool restrict_superuser)
 	return NULL;
 }
 
+/*
+ * As above, but return NULL if no such option.
+ * (This is a stopgap to avoid changing GetConfigOption's API within the
+ * 9.0.x release series.)
+ */
+const char *
+GetConfigOptionNoError(const char *name)
+{
+	struct config_generic *record;
+	static char buffer[256];
+
+	record = find_option(name, false, ERROR);
+	if (record == NULL)
+		return NULL;
+
+	switch (record->vartype)
+	{
+		case PGC_BOOL:
+			return *((struct config_bool *) record)->variable ? "on" : "off";
+
+		case PGC_INT:
+			snprintf(buffer, sizeof(buffer), "%d",
+					 *((struct config_int *) record)->variable);
+			return buffer;
+
+		case PGC_REAL:
+			snprintf(buffer, sizeof(buffer), "%g",
+					 *((struct config_real *) record)->variable);
+			return buffer;
+
+		case PGC_STRING:
+			return *((struct config_string *) record)->variable;
+
+		case PGC_ENUM:
+			return config_enum_lookup_by_value((struct config_enum *) record,
+								 *((struct config_enum *) record)->variable);
+	}
+	return NULL;
+}
+
 /*
  * Get the RESET value associated with the given option.
  *
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
@@ -253,6 +253,7 @@ extern void DefineCustomEnumVariable(
 extern void EmitWarningsOnPlaceholders(const char *className);
 
 extern const char *GetConfigOption(const char *name, bool restrict_superuser);
+extern const char *GetConfigOptionNoError(const char *name);
 extern const char *GetConfigOptionResetString(const char *name);
 extern void ProcessConfigFile(GucContext context);
 extern void InitializeGUCOptions(void);
