Revise the API for GUC variable assign hooks.

The previous functions of assign hooks are now split between check hooks
and assign hooks, where the former can fail but the latter shouldn't.
Aside from being conceptually clearer, this approach exposes the
"canonicalized" form of the variable value to guc.c without having to do
an actual assignment.  And that lets us fix the problem recently noted by
Bernd Helmle that the auto-tune patch for wal_buffers resulted in bogus
log messages about "parameter "wal_buffers" cannot be changed without
restarting the server".  There may be some speed advantage too, because
this design lets hook functions avoid re-parsing variable values when
restoring a previous state after a rollback (they can store a pre-parsed
representation of the value instead).  This patch also resolves a
longstanding annoyance about custom error messages from variable assign
hooks: they should modify, not appear separately from, guc.c's own message
about "invalid parameter value".

contrib/auth_delay/auth_delay.c

@@ -53,7 +53,7 @@ auth_delay_checks(Port *port, int status)
 void
 _PG_init(void)
 {
-	/* Define custome GUC variables */
+	/* Define custom GUC variables */
 	DefineCustomIntVariable("auth_delay.milliseconds",
 							"Milliseconds to delay before reporting authentication failure",
 							NULL,
@@ -63,6 +63,7 @@ _PG_init(void)
 							PGC_SIGHUP,
 							GUC_UNIT_MS,
 							NULL,
+							NULL,
 							NULL);
 	/* Install Hooks */
 	original_client_auth_hook = ClientAuthentication_hook;

contrib/auto_explain/auto_explain.c

@@ -74,6 +74,7 @@ _PG_init(void)
 							PGC_SUSET,
 							GUC_UNIT_MS,
 							NULL,
+							NULL,
 							NULL);
 
 	DefineCustomBoolVariable("auto_explain.log_analyze",
@@ -84,6 +85,7 @@ _PG_init(void)
 							 PGC_SUSET,
 							 0,
 							 NULL,
+							 NULL,
 							 NULL);
 
 	DefineCustomBoolVariable("auto_explain.log_verbose",
@@ -94,6 +96,7 @@ _PG_init(void)
 							 PGC_SUSET,
 							 0,
 							 NULL,
+							 NULL,
 							 NULL);
 
 	DefineCustomBoolVariable("auto_explain.log_buffers",
@@ -104,6 +107,7 @@ _PG_init(void)
 							 PGC_SUSET,
 							 0,
 							 NULL,
+							 NULL,
 							 NULL);
 
 	DefineCustomEnumVariable("auto_explain.log_format",
@@ -115,6 +119,7 @@ _PG_init(void)
 							 PGC_SUSET,
 							 0,
 							 NULL,
+							 NULL,
 							 NULL);
 
 	DefineCustomBoolVariable("auto_explain.log_nested_statements",
@@ -125,6 +130,7 @@ _PG_init(void)
 							 PGC_SUSET,
 							 0,
 							 NULL,
+							 NULL,
 							 NULL);
 
 	EmitWarningsOnPlaceholders("auto_explain");

contrib/pg_stat_statements/pg_stat_statements.c

@@ -219,6 +219,7 @@ _PG_init(void)
 							PGC_POSTMASTER,
 							0,
 							NULL,
+							NULL,
 							NULL);
 
 	DefineCustomEnumVariable("pg_stat_statements.track",
@@ -230,6 +231,7 @@ _PG_init(void)
 							 PGC_SUSET,
 							 0,
 							 NULL,
+							 NULL,
 							 NULL);
 
 	DefineCustomBoolVariable("pg_stat_statements.track_utility",
@@ -240,6 +242,7 @@ _PG_init(void)
 							 PGC_SUSET,
 							 0,
 							 NULL,
+							 NULL,
 							 NULL);
 
 	DefineCustomBoolVariable("pg_stat_statements.save",
@@ -250,6 +253,7 @@ _PG_init(void)
 							 PGC_SIGHUP,
 							 0,
 							 NULL,
+							 NULL,
 							 NULL);
 
 	EmitWarningsOnPlaceholders("pg_stat_statements");

contrib/sepgsql/hooks.c

@@ -394,6 +394,7 @@ _PG_init(void)
 							 PGC_SIGHUP,
 							 GUC_NOT_IN_SAMPLE,
 							 NULL,
+							 NULL,
 							 NULL);
 
 	/*
@@ -412,6 +413,7 @@ _PG_init(void)
 							 PGC_USERSET,
 							 GUC_NOT_IN_SAMPLE,
 							 NULL,
+							 NULL,
 							 NULL);
 
 	/*

src/backend/access/transam/xlog.c

@@ -4887,36 +4887,60 @@ GetSystemIdentifier(void)
 /*
  * Auto-tune the number of XLOG buffers.
  *
- * If the user-set value of wal_buffers is -1, we auto-tune to about 3% of
- * shared_buffers, with a maximum of one XLOG segment and a minimum of 8
- * blocks (8 was the default value prior to PostgreSQL 9.1, when auto-tuning
- * was added).  We also clamp manually-set values to at least 4 blocks; prior
- * to PostgreSQL 9.1, a minimum of 4 was enforced by guc.c, but since that
- * is no longer possible, we just silently treat such values as a request for
- * the minimum.
+ * The preferred setting for wal_buffers is about 3% of shared_buffers, with
+ * a maximum of one XLOG segment (there is little reason to think that more
+ * is helpful, at least so long as we force an fsync when switching log files)
+ * and a minimum of 8 blocks (which was the default value prior to PostgreSQL
+ * 9.1, when auto-tuning was added).
+ *
+ * This should not be called until NBuffers has received its final value.
  */
-static void
-XLOGTuneNumBuffers(void)
+static int
+XLOGChooseNumBuffers(void)
 {
-	int			xbuffers = XLOGbuffers;
-	char		buf[32];
+	int			xbuffers;
 
-	if (xbuffers == -1)
-	{
-		xbuffers = NBuffers / 32;
-		if (xbuffers > XLOG_SEG_SIZE / XLOG_BLCKSZ)
-			xbuffers = XLOG_SEG_SIZE / XLOG_BLCKSZ;
-		if (xbuffers < 8)
-			xbuffers = 8;
-	}
-	else if (xbuffers < 4)
-		xbuffers = 4;
+	xbuffers = NBuffers / 32;
+	if (xbuffers > XLOG_SEG_SIZE / XLOG_BLCKSZ)
+		xbuffers = XLOG_SEG_SIZE / XLOG_BLCKSZ;
+	if (xbuffers < 8)
+		xbuffers = 8;
+	return xbuffers;
+}
 
-	if (xbuffers != XLOGbuffers)
+/*
+ * GUC check_hook for wal_buffers
+ */
+bool
+check_wal_buffers(int *newval, void **extra, GucSource source)
+{
+	/*
+	 * -1 indicates a request for auto-tune.
+	 */
+	if (*newval == -1)
 	{
-		snprintf(buf, sizeof(buf), "%d", xbuffers);
-		SetConfigOption("wal_buffers", buf, PGC_POSTMASTER, PGC_S_OVERRIDE);
+		/*
+		 * If we haven't yet changed the boot_val default of -1, just let it
+		 * be.  We'll fix it when XLOGShmemSize is called.
+		 */
+		if (XLOGbuffers == -1)
+			return true;
+
+		/* Otherwise, substitute the auto-tune value */
+		*newval = XLOGChooseNumBuffers();
 	}
+
+	/*
+	 * We clamp manually-set values to at least 4 blocks.  Prior to PostgreSQL
+	 * 9.1, a minimum of 4 was enforced by guc.c, but since that is no longer
+	 * the case, we just silently treat such values as a request for the
+	 * minimum.  (We could throw an error instead, but that doesn't seem very
+	 * helpful.)
+	 */
+	if (*newval < 4)
+		*newval = 4;
+
+	return true;
 }
 
 /*
@@ -4927,8 +4951,19 @@ XLOGShmemSize(void)
 {
 	Size		size;
 
-	/* Figure out how many XLOG buffers we need. */
-	XLOGTuneNumBuffers();
+	/*
+	 * If the value of wal_buffers is -1, use the preferred auto-tune value.
+	 * This isn't an amazingly clean place to do this, but we must wait till
+	 * NBuffers has received its final value, and must do it before using
+	 * the value of XLOGbuffers to do anything important.
+	 */
+	if (XLOGbuffers == -1)
+	{
+		char		buf[32];
+
+		snprintf(buf, sizeof(buf), "%d", XLOGChooseNumBuffers());
+		SetConfigOption("wal_buffers", buf, PGC_POSTMASTER, PGC_S_OVERRIDE);
+	}
 	Assert(XLOGbuffers > 0);
 
 	/* XLogCtl */
@@ -8653,12 +8688,9 @@ get_sync_bit(int method)
 /*
  * GUC support
  */
-bool
-assign_xlog_sync_method(int new_sync_method, bool doit, GucSource source)
+void
+assign_xlog_sync_method(int new_sync_method, void *extra)
 {
-	if (!doit)
-		return true;
-
 	if (sync_method != new_sync_method)
 	{
 		/*
@@ -8678,8 +8710,6 @@ assign_xlog_sync_method(int new_sync_method, bool doit, GucSource source)
 				XLogFileClose();
 		}
 	}
-
-	return true;
 }
 
 

src/backend/catalog/namespace.c

@@ -3468,31 +3468,33 @@ ResetTempTableNamespace(void)
  * Routines for handling the GUC variable 'search_path'.
  */
 
-/* assign_hook: validate new search_path, do extra actions as needed */
-const char *
-assign_search_path(const char *newval, bool doit, GucSource source)
+/* check_hook: validate new search_path, if possible */
+bool
+check_search_path(char **newval, void **extra, GucSource source)
 {
+	bool		result = true;
 	char	   *rawname;
 	List	   *namelist;
 	ListCell   *l;
 
 	/* Need a modifiable copy of string */
-	rawname = pstrdup(newval);
+	rawname = pstrdup(*newval);
 
 	/* Parse string into list of identifiers */
 	if (!SplitIdentifierString(rawname, ',', &namelist))
 	{
 		/* syntax error in name list */
+		GUC_check_errdetail("List syntax is invalid.");
 		pfree(rawname);
 		list_free(namelist);
-		return NULL;
+		return false;
 	}
 
 	/*
 	 * If we aren't inside a transaction, we cannot do database access so
 	 * cannot verify the individual names.	Must accept the list on faith.
 	 */
-	if (source >= PGC_S_INTERACTIVE && IsTransactionState())
+	if (IsTransactionState())
 	{
 		/*
 		 * Verify that all the names are either valid namespace names or
@@ -3504,7 +3506,7 @@ assign_search_path(const char *newval, bool doit, GucSource source)
 		 * DATABASE SET or ALTER USER SET command.	It could be that the
 		 * intended use of the search path is for some other database, so we
 		 * should not error out if it mentions schemas not present in the
-		 * current database.  We reduce the message to NOTICE instead.
+		 * current database.  We issue a NOTICE instead.
 		 */
 		foreach(l, namelist)
 		{
@@ -3516,24 +3518,37 @@ assign_search_path(const char *newval, bool doit, GucSource source)
 				continue;
 			if (!SearchSysCacheExists1(NAMESPACENAME,
 									   CStringGetDatum(curname)))
-				ereport((source == PGC_S_TEST) ? NOTICE : ERROR,
-						(errcode(ERRCODE_UNDEFINED_SCHEMA),
-						 errmsg("schema \"%s\" does not exist", curname)));
+			{
+				if (source == PGC_S_TEST)
+					ereport(NOTICE,
+							(errcode(ERRCODE_UNDEFINED_SCHEMA),
+							 errmsg("schema \"%s\" does not exist", curname)));
+				else
+				{
+					GUC_check_errdetail("schema \"%s\" does not exist", curname);
+					result = false;
+					break;
+				}
+			}
 		}
 	}
 
 	pfree(rawname);
 	list_free(namelist);
 
+	return result;
+}
+
+/* assign_hook: do extra actions as needed */
+void
+assign_search_path(const char *newval, void *extra)
+{
 	/*
 	 * We mark the path as needing recomputation, but don't do anything until
 	 * it's needed.  This avoids trying to do database access during GUC
-	 * initialization.
+	 * initialization, or outside a transaction.
 	 */
-	if (doit)
-		baseSearchPathValid = false;
-
-	return newval;
+	baseSearchPathValid = false;
 }
 
 /*

src/backend/commands/tablespace.c

@@ -1023,9 +1023,9 @@ AlterTableSpaceOptions(AlterTableSpaceOptionsStmt *stmt)
  * Routines for handling the GUC variable 'default_tablespace'.
  */
 
-/* assign_hook: validate new default_tablespace, do extra actions as needed */
-const char *
-assign_default_tablespace(const char *newval, bool doit, GucSource source)
+/* check_hook: validate new default_tablespace */
+bool
+check_default_tablespace(char **newval, void **extra, GucSource source)
 {
 	/*
 	 * If we aren't inside a transaction, we cannot do database access so
@@ -1033,18 +1033,16 @@ assign_default_tablespace(const char *newval, bool doit, GucSource source)
 	 */
 	if (IsTransactionState())
 	{
-		if (newval[0] != '\0' &&
-			!OidIsValid(get_tablespace_oid(newval, true)))
+		if (**newval != '\0' &&
+			!OidIsValid(get_tablespace_oid(*newval, true)))
 		{
-			ereport(GUC_complaint_elevel(source),
-					(errcode(ERRCODE_UNDEFINED_OBJECT),
-					 errmsg("tablespace \"%s\" does not exist",
-							newval)));
-			return NULL;
+			GUC_check_errdetail("Tablespace \"%s\" does not exist.",
+								*newval);
+			return false;
 		}
 	}
 
-	return newval;
+	return true;
 }
 
 /*
@@ -1100,23 +1098,30 @@ GetDefaultTablespace(char relpersistence)
  * Routines for handling the GUC variable 'temp_tablespaces'.
  */
 
-/* assign_hook: validate new temp_tablespaces, do extra actions as needed */
-const char *
-assign_temp_tablespaces(const char *newval, bool doit, GucSource source)
+typedef struct
+{
+	int			numSpcs;
+	Oid			tblSpcs[1];		/* VARIABLE LENGTH ARRAY */
+} temp_tablespaces_extra;
+
+/* check_hook: validate new temp_tablespaces */
+bool
+check_temp_tablespaces(char **newval, void **extra, GucSource source)
 {
 	char	   *rawname;
 	List	   *namelist;
 
 	/* Need a modifiable copy of string */
-	rawname = pstrdup(newval);
+	rawname = pstrdup(*newval);
 
 	/* Parse string into list of identifiers */
 	if (!SplitIdentifierString(rawname, ',', &namelist))
 	{
 		/* syntax error in name list */
+		GUC_check_errdetail("List syntax is invalid.");
 		pfree(rawname);
 		list_free(namelist);
-		return NULL;
+		return false;
 	}
 
 	/*
@@ -1126,17 +1131,13 @@ assign_temp_tablespaces(const char *newval, bool doit, GucSource source)
 	 */
 	if (IsTransactionState())
 	{
-		/*
-		 * If we error out below, or if we are called multiple times in one
-		 * transaction, we'll leak a bit of TopTransactionContext memory.
-		 * Doesn't seem worth worrying about.
-		 */
+		temp_tablespaces_extra *myextra;
 		Oid		   *tblSpcs;
 		int			numSpcs;
 		ListCell   *l;
 
-		tblSpcs = (Oid *) MemoryContextAlloc(TopTransactionContext,
-										list_length(namelist) * sizeof(Oid));
+		/* temporary workspace until we are done verifying the list */
+		tblSpcs = (Oid *) palloc(list_length(namelist) * sizeof(Oid));
 		numSpcs = 0;
 		foreach(l, namelist)
 		{
@@ -1169,7 +1170,7 @@ assign_temp_tablespaces(const char *newval, bool doit, GucSource source)
 				continue;
 			}
 
-			/* Check permissions similarly */
+			/* Check permissions, similarly complaining only if interactive */
 			aclresult = pg_tablespace_aclcheck(curoid, GetUserId(),
 											   ACL_CREATE);
 			if (aclresult != ACLCHECK_OK)
@@ -1182,17 +1183,41 @@ assign_temp_tablespaces(const char *newval, bool doit, GucSource source)
 			tblSpcs[numSpcs++] = curoid;
 		}
 
-		/* If actively "doing it", give the new list to fd.c */
-		if (doit)
-			SetTempTablespaces(tblSpcs, numSpcs);
-		else
-			pfree(tblSpcs);
+		/* Now prepare an "extra" struct for assign_temp_tablespaces */
+		myextra = malloc(offsetof(temp_tablespaces_extra, tblSpcs) +
+						 numSpcs * sizeof(Oid));
+		if (!myextra)
+			return false;
+		myextra->numSpcs = numSpcs;
+		memcpy(myextra->tblSpcs, tblSpcs, numSpcs * sizeof(Oid));
+		*extra = (void *) myextra;
+
+		pfree(tblSpcs);
 	}
 
 	pfree(rawname);
 	list_free(namelist);
 
-	return newval;
+	return true;
+}
+
+/* assign_hook: do extra actions as needed */
+void
+assign_temp_tablespaces(const char *newval, void *extra)
+{
+	temp_tablespaces_extra *myextra = (temp_tablespaces_extra *) extra;
+
+	/*
+	 * If check_temp_tablespaces was executed inside a transaction, then pass
+	 * the list it made to fd.c.  Otherwise, clear fd.c's list; we must be
+	 * still outside a transaction, or else restoring during transaction exit,
+	 * and in either case we can just let the next PrepareTempTablespaces call
+	 * make things sane.
+	 */
+	if (myextra)
+		SetTempTablespaces(myextra->tblSpcs, myextra->numSpcs);
+	else
+		SetTempTablespaces(NULL, 0);
 }
 
 /*

src/backend/nls.mk

@@ -2,7 +2,10 @@
 CATALOG_NAME	:= postgres
 AVAIL_LANGUAGES	:= de es fr ja pt_BR tr
 GETTEXT_FILES	:= + gettext-files
-GETTEXT_TRIGGERS:= _ errmsg errmsg_plural:1,2 errdetail errdetail_log errdetail_plural:1,2 errhint errcontext write_stderr yyerror parser_yyerror
+GETTEXT_TRIGGERS:= _ errmsg errmsg_plural:1,2 errdetail errdetail_log \
+    errdetail_plural:1,2 errhint errcontext \
+    GUC_check_errmsg GUC_check_errdetail GUC_check_errhint \
+    write_stderr yyerror parser_yyerror
 
 gettext-files: distprep
 	find $(srcdir)/ $(srcdir)/../port/ -name '*.c' -print >$@

src/backend/replication/syncrep.c

@@ -639,25 +639,23 @@ SyncRepQueueIsOrderedByLSN(void)
  * ===========================================================
  */
 
-const char *
-assign_synchronous_standby_names(const char *newval, bool doit, GucSource source)
+bool
+check_synchronous_standby_names(char **newval, void **extra, GucSource source)
 {
 	char	   *rawstring;
 	List	   *elemlist;
 
 	/* Need a modifiable copy of string */
-	rawstring = pstrdup(newval);
+	rawstring = pstrdup(*newval);
 
 	/* Parse string into list of identifiers */
 	if (!SplitIdentifierString(rawstring, ',', &elemlist))
 	{
 		/* syntax error in list */
+		GUC_check_errdetail("List syntax is invalid.");
 		pfree(rawstring);
 		list_free(elemlist);
-		ereport(GUC_complaint_elevel(source),
-				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-		   errmsg("invalid list syntax for parameter \"synchronous_standby_names\"")));
-		return NULL;
+		return false;
 	}
 
 	/*
@@ -671,5 +669,5 @@ assign_synchronous_standby_names(const char *newval, bool doit, GucSource source
 	pfree(rawstring);
 	list_free(elemlist);
 
-	return newval;
+	return true;
 }

src/backend/tcop/postgres.c

@@ -2804,7 +2804,8 @@ RecoveryConflictInterrupt(ProcSignalReason reason)
 				break;
 
 			default:
-				elog(FATAL, "Unknown conflict mode");
+				elog(FATAL, "unrecognized conflict mode: %d",
+					 (int) reason);
 		}
 
 		Assert(RecoveryConflictPending && (QueryCancelPending || ProcDiePending));
@@ -3062,27 +3063,32 @@ check_stack_depth(void)
 #endif /* IA64 */
 }
 
-/* GUC assign hook for max_stack_depth */
+/* GUC check hook for max_stack_depth */
 bool
-assign_max_stack_depth(int newval, bool doit, GucSource source)
+check_max_stack_depth(int *newval, void **extra, GucSource source)
 {
-	long		newval_bytes = newval * 1024L;
+	long		newval_bytes = *newval * 1024L;
 	long		stack_rlimit = get_stack_depth_rlimit();
 
 	if (stack_rlimit > 0 && newval_bytes > stack_rlimit - STACK_DEPTH_SLOP)
 	{
-		ereport(GUC_complaint_elevel(source),
-				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-				 errmsg("\"max_stack_depth\" must not exceed %ldkB",
-						(stack_rlimit - STACK_DEPTH_SLOP) / 1024L),
-				 errhint("Increase the platform's stack depth limit via \"ulimit -s\" or local equivalent.")));
+		GUC_check_errdetail("\"max_stack_depth\" must not exceed %ldkB.",
+							(stack_rlimit - STACK_DEPTH_SLOP) / 1024L);
+		GUC_check_errhint("Increase the platform's stack depth limit via \"ulimit -s\" or local equivalent.");
 		return false;
 	}
-	if (doit)
-		max_stack_depth_bytes = newval_bytes;
 	return true;
 }
 
+/* GUC assign hook for max_stack_depth */
+void
+assign_max_stack_depth(int newval, void *extra)
+{
+	long		newval_bytes = newval * 1024L;
+
+	max_stack_depth_bytes = newval_bytes;
+}
+
 
 /*
  * set_debug_options --- apply "-d N" command line option

src/backend/utils/adt/datetime.c

@@ -4140,20 +4140,17 @@ CheckDateTokenTables(void)
 /*
  * This function gets called during timezone config file load or reload
  * to create the final array of timezone tokens.  The argument array
- * is already sorted in name order.  This data is in a temporary memory
- * context and must be copied to somewhere permanent.
+ * is already sorted in name order.  The data is converted to datetkn
+ * format and installed in *tbl, which must be allocated by the caller.
  */
 void
-InstallTimeZoneAbbrevs(tzEntry *abbrevs, int n)
+ConvertTimeZoneAbbrevs(TimeZoneAbbrevTable *tbl,
+					   struct tzEntry *abbrevs, int n)
 {
-	datetkn    *newtbl;
+	datetkn    *newtbl = tbl->abbrevs;
 	int			i;
 
-	/*
-	 * Copy the data into TopMemoryContext and convert to datetkn format.
-	 */
-	newtbl = (datetkn *) MemoryContextAlloc(TopMemoryContext,
-											n * sizeof(datetkn));
+	tbl->numabbrevs = n;
 	for (i = 0; i < n; i++)
 	{
 		strncpy(newtbl[i].token, abbrevs[i].abbrev, TOKMAXLEN);
@@ -4163,12 +4160,20 @@ InstallTimeZoneAbbrevs(tzEntry *abbrevs, int n)
 
 	/* Check the ordering, if testing */
 	Assert(CheckDateTokenTable("timezone offset", newtbl, n));
+}
 
-	/* Now safe to replace existing table (if any) */
-	if (timezonetktbl)
-		pfree(timezonetktbl);
-	timezonetktbl = newtbl;
-	sztimezonetktbl = n;
+/*
+ * Install a TimeZoneAbbrevTable as the active table.
+ *
+ * Caller is responsible that the passed table doesn't go away while in use.
+ */
+void
+InstallTimeZoneAbbrevs(TimeZoneAbbrevTable *tbl)
+{
+	int			i;
+
+	timezonetktbl = tbl->abbrevs;
+	sztimezonetktbl = tbl->numabbrevs;
 
 	/* clear date cache in case it contains any stale timezone names */
 	for (i = 0; i < MAXDATEFIELDS; i++)

src/backend/utils/adt/pg_locale.c

@@ -236,52 +236,53 @@ check_locale(int category, const char *value)
 	return ret;
 }
 
-/* GUC assign hooks */
 
 /*
- * This is common code for several locale categories.  This doesn't
- * actually set the locale permanently, it only tests if the locale is
- * valid.  (See explanation at the top of this file.)
+ * GUC check/assign hooks
+ *
+ * For most locale categories, the assign hook doesn't actually set the locale
+ * permanently, just reset flags so that the next use will cache the
+ * appropriate values.  (See explanation at the top of this file.)
  *
  * Note: we accept value = "" as selecting the postmaster's environment
  * value, whatever it was (so long as the environment setting is legal).
  * This will have been locked down by an earlier call to pg_perm_setlocale.
  */
-static const char *
-locale_xxx_assign(int category, const char *value, bool doit, GucSource source)
+bool
+check_locale_monetary(char **newval, void **extra, GucSource source)
 {
-	if (!check_locale(category, value))
-		value = NULL;			/* set failure return marker */
-
-	/* need to reload cache next time? */
-	if (doit && value != NULL)
-	{
-		CurrentLocaleConvValid = false;
-		CurrentLCTimeValid = false;
-	}
-
-	return value;
+	return check_locale(LC_MONETARY, *newval);
 }
 
-
-const char *
-locale_monetary_assign(const char *value, bool doit, GucSource source)
+void
+assign_locale_monetary(const char *newval, void *extra)
 {
-	return locale_xxx_assign(LC_MONETARY, value, doit, source);
+	CurrentLocaleConvValid = false;
 }
 
-const char *
-locale_numeric_assign(const char *value, bool doit, GucSource source)
+bool
+check_locale_numeric(char **newval, void **extra, GucSource source)
 {
-	return locale_xxx_assign(LC_NUMERIC, value, doit, source);
+	return check_locale(LC_NUMERIC, *newval);
 }
 
-const char *
-locale_time_assign(const char *value, bool doit, GucSource source)
+void
+assign_locale_numeric(const char *newval, void *extra)
 {
-	return locale_xxx_assign(LC_TIME, value, doit, source);
+	CurrentLocaleConvValid = false;
 }
 
+bool
+check_locale_time(char **newval, void **extra, GucSource source)
+{
+	return check_locale(LC_TIME, *newval);
+}
+
+void
+assign_locale_time(const char *newval, void *extra)
+{
+	CurrentLCTimeValid = false;
+}
 
 /*
  * We allow LC_MESSAGES to actually be set globally.
@@ -293,31 +294,39 @@ locale_time_assign(const char *value, bool doit, GucSource source)
  * The idea there is just to accept the environment setting *if possible*
  * during startup, until we can read the proper value from postgresql.conf.
  */
-const char *
-locale_messages_assign(const char *value, bool doit, GucSource source)
+bool
+check_locale_messages(char **newval, void **extra, GucSource source)
 {
-	if (*value == '\0' && source != PGC_S_DEFAULT)
-		return NULL;
+	if (**newval == '\0')
+	{
+		if (source == PGC_S_DEFAULT)
+			return true;
+		else
+			return false;
+	}
 
 	/*
 	 * LC_MESSAGES category does not exist everywhere, but accept it anyway
 	 *
-	 * On Windows, we can't even check the value, so the non-doit case is a
-	 * no-op
+	 * On Windows, we can't even check the value, so accept blindly
+	 */
+#if defined(LC_MESSAGES) && !defined(WIN32)
+	return check_locale(LC_MESSAGES, *newval);
+#else
+	return true;
+#endif
+}
+
+void
+assign_locale_messages(const char *newval, void *extra)
+{
+	/*
+	 * LC_MESSAGES category does not exist everywhere, but accept it anyway.
+	 * We ignore failure, as per comment above.
 	 */
 #ifdef LC_MESSAGES
-	if (doit)
-	{
-		if (!pg_perm_setlocale(LC_MESSAGES, value))
-			if (source != PGC_S_DEFAULT)
-				return NULL;
-	}
-#ifndef WIN32
-	else
-		value = locale_xxx_assign(LC_MESSAGES, value, false, source);
-#endif   /* WIN32 */
-#endif   /* LC_MESSAGES */
-	return value;
+	(void) pg_perm_setlocale(LC_MESSAGES, newval);
+#endif
 }
 
 

src/backend/utils/cache/ts_cache.c

@@ -587,8 +587,9 @@ getTSCurrentConfig(bool emitError)
 	return TSCurrentConfigCache;
 }
 
-const char *
-assignTSCurrentConfig(const char *newval, bool doit, GucSource source)
+/* GUC check_hook for default_text_search_config */
+bool
+check_TSCurrentConfig(char **newval, void **extra, GucSource source)
 {
 	/*
 	 * If we aren't inside a transaction, we cannot do database access so
@@ -601,10 +602,10 @@ assignTSCurrentConfig(const char *newval, bool doit, GucSource source)
 		Form_pg_ts_config cfg;
 		char	   *buf;
 
-		cfgId = get_ts_config_oid(stringToQualifiedNameList(newval), true);
+		cfgId = get_ts_config_oid(stringToQualifiedNameList(*newval), true);
 
 		if (!OidIsValid(cfgId))
-			return NULL;
+			return false;
 
 		/*
 		 * Modify the actually stored value to be fully qualified, to ensure
@@ -622,17 +623,20 @@ assignTSCurrentConfig(const char *newval, bool doit, GucSource source)
 		ReleaseSysCache(tuple);
 
 		/* GUC wants it malloc'd not palloc'd */
-		newval = strdup(buf);
+		free(*newval);
+		*newval = strdup(buf);
 		pfree(buf);
-
-		if (doit && newval)
-			TSCurrentConfigCache = cfgId;
-	}
-	else
-	{
-		if (doit)
-			TSCurrentConfigCache = InvalidOid;
+		if (!newval)
+			return false;
 	}
 
-	return newval;
+	return true;
+}
+
+/* GUC assign_hook for default_text_search_config */
+void
+assign_TSCurrentConfig(const char *newval, void *extra)
+{
+	/* Just reset the cache to force a lookup on first use */
+	TSCurrentConfigCache = InvalidOid;
 }

src/backend/utils/error/elog.c

@@ -1156,6 +1156,62 @@ elog_finish(int elevel, const char *fmt,...)
 	errfinish(0);
 }
 
+
+/*
+ * Functions to allow construction of error message strings separately from
+ * the ereport() call itself.
+ *
+ * The expected calling convention is
+ *
+ *	pre_format_elog_string(errno, domain), var = format_elog_string(format,...)
+ *
+ * which can be hidden behind a macro such as GUC_check_errdetail().  We
+ * assume that any functions called in the arguments of format_elog_string()
+ * cannot result in re-entrant use of these functions --- otherwise the wrong
+ * text domain might be used, or the wrong errno substituted for %m.  This is
+ * okay for the current usage with GUC check hooks, but might need further
+ * effort someday.
+ *
+ * The result of format_elog_string() is stored in ErrorContext, and will
+ * therefore survive until FlushErrorState() is called.
+ */
+static int save_format_errnumber;
+static const char *save_format_domain;
+
+void
+pre_format_elog_string(int errnumber, const char *domain)
+{
+	/* Save errno before evaluation of argument functions can change it */
+	save_format_errnumber = errnumber;
+	/* Save caller's text domain */
+	save_format_domain = domain;
+}
+
+char *
+format_elog_string(const char *fmt, ...)
+{
+	ErrorData	errdata;
+	ErrorData  *edata;
+	MemoryContext oldcontext;
+
+	/* Initialize a mostly-dummy error frame */
+	edata = &errdata;
+	MemSet(edata, 0, sizeof(ErrorData));
+	/* the default text domain is the backend's */
+	edata->domain = save_format_domain ? save_format_domain : PG_TEXTDOMAIN("postgres");
+	/* set the errno to be used to interpret %m */
+	edata->saved_errno = save_format_errnumber;
+
+	oldcontext = MemoryContextSwitchTo(ErrorContext);
+
+	EVALUATE_MESSAGE(message, false, true);
+
+	MemoryContextSwitchTo(oldcontext);
+
+	return edata->message;
+}
+
+
 /*
  * Actual output of the top-of-stack error message
  *

src/backend/utils/mb/mbutils.c

@@ -77,12 +77,16 @@ static int	cliplen(const char *str, int len, int limit);
 
 
 /*
- * Set the client encoding and save fmgrinfo for the conversion
- * function if necessary.  Returns 0 if okay, -1 if not (bad encoding
- * or can't support conversion)
+ * Prepare for a future call to SetClientEncoding.  Success should mean
+ * that SetClientEncoding is guaranteed to succeed for this encoding request.
+ *
+ * (But note that success before backend_startup_complete does not guarantee
+ * success after ...)
+ *
+ * Returns 0 if okay, -1 if not (bad encoding or can't support conversion)
  */
 int
-SetClientEncoding(int encoding, bool doit)
+PrepareClientEncoding(int encoding)
 {
 	int			current_server_encoding;
 	ListCell   *lc;
@@ -92,11 +96,7 @@ SetClientEncoding(int encoding, bool doit)
 
 	/* Can't do anything during startup, per notes above */
 	if (!backend_startup_complete)
-	{
-		if (doit)
-			pending_client_encoding = encoding;
 		return 0;
-	}
 
 	current_server_encoding = GetDatabaseEncoding();
 
@@ -106,15 +106,7 @@ SetClientEncoding(int encoding, bool doit)
 	if (current_server_encoding == encoding ||
 		current_server_encoding == PG_SQL_ASCII ||
 		encoding == PG_SQL_ASCII)
-	{
-		if (doit)
-		{
-			ClientEncoding = &pg_enc2name_tbl[encoding];
-			ToServerConvProc = NULL;
-			ToClientConvProc = NULL;
-		}
 		return 0;
-	}
 
 	if (IsTransactionState())
 	{
@@ -138,12 +130,6 @@ SetClientEncoding(int encoding, bool doit)
 		if (!OidIsValid(to_client_proc))
 			return -1;
 
-		/*
-		 * Done if not wanting to actually apply setting.
-		 */
-		if (!doit)
-			return 0;
-
 		/*
 		 * Load the fmgr info into TopMemoryContext (could still fail here)
 		 */
@@ -162,30 +148,9 @@ SetClientEncoding(int encoding, bool doit)
 		MemoryContextSwitchTo(oldcontext);
 
 		/*
-		 * Everything is okay, so apply the setting.
+		 * We cannot yet remove any older entry for the same encoding pair,
+		 * since it could still be in use.  SetClientEncoding will clean up.
 		 */
-		ClientEncoding = &pg_enc2name_tbl[encoding];
-		ToServerConvProc = &convinfo->to_server_info;
-		ToClientConvProc = &convinfo->to_client_info;
-
-		/*
-		 * Remove any older entry for the same encoding pair (this is just to
-		 * avoid memory leakage).
-		 */
-		foreach(lc, ConvProcList)
-		{
-			ConvProcInfo *oldinfo = (ConvProcInfo *) lfirst(lc);
-
-			if (oldinfo == convinfo)
-				continue;
-			if (oldinfo->s_encoding == convinfo->s_encoding &&
-				oldinfo->c_encoding == convinfo->c_encoding)
-			{
-				ConvProcList = list_delete_ptr(ConvProcList, oldinfo);
-				pfree(oldinfo);
-				break;			/* need not look further */
-			}
-		}
 
 		return 0;				/* success */
 	}
@@ -205,15 +170,7 @@ SetClientEncoding(int encoding, bool doit)
 
 			if (oldinfo->s_encoding == current_server_encoding &&
 				oldinfo->c_encoding == encoding)
-			{
-				if (doit)
-				{
-					ClientEncoding = &pg_enc2name_tbl[encoding];
-					ToServerConvProc = &oldinfo->to_server_info;
-					ToClientConvProc = &oldinfo->to_client_info;
-				}
 				return 0;
-			}
 		}
 
 		return -1;				/* it's not cached, so fail */
@@ -221,8 +178,83 @@ SetClientEncoding(int encoding, bool doit)
 }
 
 /*
- * Initialize client encoding if necessary.
- *		called from InitPostgres() once during backend startup.
+ * Set the active client encoding and set up the conversion-function pointers.
+ * PrepareClientEncoding should have been called previously for this encoding.
+ *
+ * Returns 0 if okay, -1 if not (bad encoding or can't support conversion)
+ */
+int
+SetClientEncoding(int encoding)
+{
+	int			current_server_encoding;
+	bool		found;
+	ListCell   *lc;
+
+	if (!PG_VALID_FE_ENCODING(encoding))
+		return -1;
+
+	/* Can't do anything during startup, per notes above */
+	if (!backend_startup_complete)
+	{
+		pending_client_encoding = encoding;
+		return 0;
+	}
+
+	current_server_encoding = GetDatabaseEncoding();
+
+	/*
+	 * Check for cases that require no conversion function.
+	 */
+	if (current_server_encoding == encoding ||
+		current_server_encoding == PG_SQL_ASCII ||
+		encoding == PG_SQL_ASCII)
+	{
+		ClientEncoding = &pg_enc2name_tbl[encoding];
+		ToServerConvProc = NULL;
+		ToClientConvProc = NULL;
+		return 0;
+	}
+
+	/*
+	 * Search the cache for the entry previously prepared by
+	 * PrepareClientEncoding; if there isn't one, we lose.  While at it,
+	 * release any duplicate entries so that repeated Prepare/Set cycles
+	 * don't leak memory.
+	 */
+	found = false;
+	foreach(lc, ConvProcList)
+	{
+		ConvProcInfo *convinfo = (ConvProcInfo *) lfirst(lc);
+
+		if (convinfo->s_encoding == current_server_encoding &&
+			convinfo->c_encoding == encoding)
+		{
+			if (!found)
+			{
+				/* Found newest entry, so set up */
+				ClientEncoding = &pg_enc2name_tbl[encoding];
+				ToServerConvProc = &convinfo->to_server_info;
+				ToClientConvProc = &convinfo->to_client_info;
+				found = true;
+			}
+			else
+			{
+				/* Duplicate entry, release it */
+				ConvProcList = list_delete_ptr(ConvProcList, convinfo);
+				pfree(convinfo);
+			}
+		}
+	}
+
+	if (found)
+		return 0;				/* success */
+	else
+		return -1;				/* it's not cached, so fail */
+}
+
+/*
+ * Initialize client encoding conversions.
+ *		Called from InitPostgres() once during backend startup.
  */
 void
 InitializeClientEncoding(void)
@@ -230,7 +262,8 @@ InitializeClientEncoding(void)
 	Assert(!backend_startup_complete);
 	backend_startup_complete = true;
 
-	if (SetClientEncoding(pending_client_encoding, true) < 0)
+	if (PrepareClientEncoding(pending_client_encoding) < 0 ||
+		SetClientEncoding(pending_client_encoding) < 0)
 	{
 		/*
 		 * Oops, the requested conversion is not available. We couldn't fail

src/backend/utils/misc/README

@@ -1,10 +1,10 @@
 src/backend/utils/misc/README
 
-Guc Implementation Notes
+GUC Implementation Notes
 ========================
 
 The GUC (Grand Unified Configuration) module implements configuration
-variables of multiple types (currently boolean, enum, int, float, and string).
+variables of multiple types (currently boolean, enum, int, real, and string).
 Variable settings can come from various places, with a priority ordering
 determining which setting is used.
 
@@ -12,65 +12,112 @@ determining which setting is used.
 Per-Variable Hooks
 ------------------
 
-Each variable known to GUC can optionally have an assign_hook and/or
-a show_hook to provide customized behavior.  Assign hooks are used to
-perform validity checking on variable values (above and beyond what
-GUC can do).  They are also used to update any derived state that needs
-to change when a GUC variable is set.  Show hooks are used to modify
-the default SHOW display for a variable.
+Each variable known to GUC can optionally have a check_hook, an
+assign_hook, and/or a show_hook to provide customized behavior.
+Check hooks are used to perform validity checking on variable values
+(above and beyond what GUC can do), to compute derived settings when
+nontrivial work is needed to do that, and optionally to "canonicalize"
+user-supplied values.  Assign hooks are used to update any derived state
+that needs to change when a GUC variable is set.  Show hooks are used to
+modify the default SHOW display for a variable.
+
+
+If a check_hook is provided, it points to a function of the signature
+	bool check_hook(datatype *newvalue, void **extra, GucSource source)
+The "newvalue" argument is of type bool *, int *, double *, or char **
+for bool, int/enum, real, or string variables respectively.  The check
+function should validate the proposed new value, and return true if it is
+OK or false if not.  The function can optionally do a few other things:
+
+* When rejecting a bad proposed value, it may be useful to append some
+additional information to the generic "invalid value for parameter FOO"
+complaint that guc.c will emit.  To do that, call
+	void GUC_check_errdetail(const char *format, ...)
+where the format string and additional arguments follow the rules for
+errdetail() arguments.  The resulting string will be emitted as the
+DETAIL line of guc.c's error report, so it should follow the message style
+guidelines for DETAIL messages.  There is also
+	void GUC_check_errhint(const char *format, ...)
+which can be used in the same way to append a HINT message.
+Occasionally it may even be appropriate to override guc.c's generic primary
+message or error code, which can be done with
+	void GUC_check_errcode(int sqlerrcode)
+	void GUC_check_errmsg(const char *format, ...)
+In general, check_hooks should avoid throwing errors directly if possible,
+though this may be impractical to avoid for some corner cases such as
+out-of-memory.
+
+* Since the newvalue is pass-by-reference, the function can modify it.
+This might be used for example to canonicalize the spelling of a string
+value, round off a buffer size to the nearest supported value, or replace
+a special value such as "-1" with a computed default value.  If the
+function wishes to replace a string value, it must malloc (not palloc)
+the replacement value, and be sure to free() the previous value.
+
+* Derived information, such as the role OID represented by a user name,
+can be stored for use by the assign hook.  To do this, malloc (not palloc)
+storage space for the information, and return its address at *extra.
+guc.c will automatically free() this space when the associated GUC setting
+is no longer of interest.  *extra is initialized to NULL before call, so
+it can be ignored if not needed.
+
+The "source" argument indicates the source of the proposed new value,
+If it is >= PGC_S_INTERACTIVE, then we are performing an interactive
+assignment (e.g., a SET command).  But when source < PGC_S_INTERACTIVE,
+we are reading a non-interactive option source, such as postgresql.conf.
+This is sometimes needed to determine whether a setting should be
+allowed.  The check_hook might also look at the current actual value of
+the variable to determine what is allowed.
+
+Note that check hooks are sometimes called just to validate a value,
+without any intention of actually changing the setting.  Therefore the
+check hook must *not* take any action based on the assumption that an
+assignment will occur.
+
 
 If an assign_hook is provided, it points to a function of the signature
-	bool assign_hook(newvalue, bool doit, GucSource source)
-where the type of "newvalue" matches the kind of variable.  This function
-is called immediately before actually setting the variable's value (so it
-can look at the actual variable to determine the old value).  If the
-function returns "true" then the assignment is completed; if it returns
-"false" then newvalue is considered invalid and the assignment is not
-performed.  If "doit" is false then the function should simply check
-validity of newvalue and not change any derived state.  The "source" parameter
-indicates where the new value came from.  If it is >= PGC_S_INTERACTIVE,
-then we are performing an interactive assignment (e.g., a SET command), and
-ereport(ERROR) is safe to do.  But when source < PGC_S_INTERACTIVE, we are
-reading a non-interactive option source, such as postgresql.conf.  In this
-case the assign_hook should *not* ereport but should just return false if it
-doesn't like the newvalue.
+	void assign_hook(datatype newvalue, void *extra)
+where the type of "newvalue" matches the kind of variable, and "extra"
+is the derived-information pointer returned by the check_hook (always
+NULL if there is no check_hook).  This function is called immediately
+before actually setting the variable's value (so it can look at the actual
+variable to determine the old value, for example to avoid doing work when
+the value isn't really changing).
 
-If an assign_hook returns false then guc.c will report a generic "invalid
-value for option FOO" error message.  If you feel the need to provide a more
-specific error message, ereport() it using "GUC_complaint_elevel(source)"
-as the error level.  Note that this might return either ERROR or a lower level
-such as LOG, so the ereport call might or might not return.  If it does
-return, return false out of the assign_hook.
+Note that there is no provision for a failure result code.  assign_hooks
+should never fail except under the most dire circumstances, since a failure
+may for example result in GUC settings not being rolled back properly during
+transaction abort.  In general, try to do anything that could conceivably
+fail in a check_hook instead, and pass along the results in an "extra"
+struct, so that the assign hook has little to do beyond copying the data to
+someplace.  This applies particularly to catalog lookups: any required
+lookups must be done in the check_hook, since the assign_hook may be
+executed during transaction rollback when lookups will be unsafe.
 
-For string variables, the signature for assign hooks is a bit different:
-	const char *assign_hook(const char *newvalue,
-				bool doit,
-				GucSource source)
-The meanings of the parameters are the same as for the other types of GUC
-variables, but the return value is handled differently:
-	NULL --- assignment fails (like returning false for other datatypes)
-	newvalue --- assignment succeeds, assign the newvalue as-is
-	malloc'd (not palloc'd!!!) string --- assign that value instead
-The third choice is allowed in case the assign_hook wants to return a
-"canonical" version of the new value.  For example, the assign_hook for
-datestyle always returns a string that includes both output and input
-datestyle options, although the input might have specified only one.
+Note that check_hooks are sometimes called outside any transaction, too.
+This happens when processing the wired-in "bootstrap" value, values coming
+from the postmaster command line or environment, or values coming from
+postgresql.conf.  Therefore, any catalog lookups done in a check_hook
+should be guarded with an IsTransactionState() test, and there must be a
+fallback path to allow derived values to be computed during the first
+subsequent use of the GUC setting within a transaction.  A typical
+arrangement is for the catalog values computed by the check_hook and
+installed by the assign_hook to be used only for the remainder of the
+transaction in which the new setting is made.  Each subsequent transaction
+looks up the values afresh on first use.  This arrangement is useful to
+prevent use of stale catalog values, independently of the problem of
+needing to check GUC values outside a transaction.
 
-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
-by SHOW.
+by SHOW (and other SQL features for showing GUC variable values).
+The return value can point to a static buffer, since show functions are
+not used re-entrantly.
 
 
-Saving/Restoring Guc Variable Values
+Saving/Restoring GUC Variable Values
 ------------------------------------
 
 Prior values of configuration variables must be remembered in order to deal
@@ -200,27 +247,49 @@ these has a current source priority <= PGC_S_FILE.  (It is thus possible
 for reset_val to track the config-file setting even if there is
 currently a different interactive value of the actual variable.)
 
-The assign_hook and show_hook routines work only with the actual variable,
-and are not directly aware of the additional values maintained by GUC.
-This is not a problem for normal usage, since we can assign first to the
-actual variable and then (if that succeeds) to the additional values as
-needed.  However, for SIGHUP rereads we may not want to assign to the
-actual variable.  Our procedure in that case is to call the assign_hook
-with doit = false so that the value is validated, but no derived state is
-changed.
+The check_hook, assign_hook and show_hook routines work only with the
+actual variable, and are not directly aware of the additional values
+maintained by GUC.
 
 
-String Memory Handling
-----------------------
+GUC Memory Handling
+-------------------
 
-String option values are allocated with strdup, not with the
-pstrdup/palloc mechanisms.  We would need to keep them in a permanent
-context anyway, and strdup gives us more control over handling
+String variable values are allocated with malloc/strdup, not with the
+palloc/pstrdup mechanisms.  We would need to keep them in a permanent
+context anyway, and malloc gives us more control over handling
 out-of-memory failures.
 
 We allow a string variable's actual value, reset_val, boot_val, and stacked
 values to point at the same storage.  This makes it slightly harder to free
 space (we must test whether a value to be freed isn't equal to any of the
 other pointers in the GUC entry or associated stack items).  The main
-advantage is that we never need to strdup during transaction commit/abort,
+advantage is that we never need to malloc during transaction commit/abort,
 so cannot cause an out-of-memory failure there.
+
+"Extra" structs returned by check_hook routines are managed in the same
+way as string values.  Note that we support "extra" structs for all types
+of GUC variables, although they are mainly useful with strings.
+
+
+GUC and Null String Variables
+-----------------------------
+
+A GUC string variable can have a boot_val of NULL.  guc.c handles this
+unsurprisingly, assigning the NULL to the underlying C variable.  Any code
+using such a variable, as well as any hook functions for it, must then be
+prepared to deal with a NULL value.
+
+However, it is not possible to assign a NULL value to a GUC string
+variable in any other way: values coming from SET, postgresql.conf, etc,
+might be empty strings, but they'll never be NULL.  And SHOW displays
+a NULL the same as an empty string.  It is therefore not appropriate to
+treat a NULL value as a distinct user-visible setting.  A typical use
+for a NULL boot_val is to denote that a value hasn't yet been set for
+a variable that will receive a real value later in startup.
+
+If it's undesirable for code using the underlying C variable to have to
+worry about NULL values ever, the variable can be given a non-null static
+initializer as well as a non-null boot_val.  guc.c will overwrite the
+static initializer pointer with a copy of the boot_val during
+InitializeGUCOptions, but the variable will never contain a NULL.

src/backend/utils/misc/guc-file.l

@@ -141,7 +141,8 @@ ProcessConfigFile(GucContext context)
 	 */
 	cvc_struct = (struct config_string *)
 		find_option("custom_variable_classes", false, elevel);
-	if (cvc_struct && cvc_struct->gen.reset_source > PGC_S_FILE)
+	Assert(cvc_struct);
+	if (cvc_struct->gen.reset_source > PGC_S_FILE)
 	{
 		cvc = guc_strdup(elevel, cvc_struct->reset_val);
 		if (cvc == NULL)
@@ -151,19 +152,18 @@ ProcessConfigFile(GucContext context)
 			 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.
+		 * Need to canonicalize the value by calling the check hook.
 		 */
-		cvc = (char *) assign_custom_variable_classes(head->value,
-													  false, PGC_S_FILE);
+		void   *extra = NULL;
+
+		cvc = guc_strdup(elevel, head->value);
 		if (cvc == NULL)
-		{
-			ereport(elevel,
-					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-					 errmsg("invalid value for parameter \"%s\": \"%s\"",
-							head->name, head->value)));
 			goto cleanup_list;
-		}
+		if (!call_string_check_hook(cvc_struct, &cvc, &extra,
+									PGC_S_FILE, elevel))
+			goto cleanup_list;
+		if (extra)
+			free(extra);
 	}
 
 	/*

src/backend/utils/misc/tzparser.c

@@ -3,10 +3,12 @@
  * tzparser.c
  *	  Functions for parsing timezone offset files
  *
- * Note: we generally should not throw any errors in this file, but instead
- * try to return an error code.  This is not completely bulletproof at
- * present --- in particular out-of-memory will throw an error.  Could
- * probably fix with PG_TRY if necessary.
+ * Note: this code is invoked from the check_hook for the GUC variable
+ * timezone_abbreviations.  Therefore, it should report problems using
+ * GUC_check_errmsg() and related functions, and try to avoid throwing
+ * elog(ERROR).  This is not completely bulletproof at present --- in
+ * particular out-of-memory will throw an error.  Could probably fix with
+ * PG_TRY if necessary.
  *
  *
  * Portions Copyright (c) 1996-2011, PostgreSQL Global Development Group
@@ -24,15 +26,13 @@
 
 #include "miscadmin.h"
 #include "storage/fd.h"
-#include "utils/datetime.h"
+#include "utils/guc.h"
 #include "utils/memutils.h"
 #include "utils/tzparser.h"
 
 
 #define WHITESPACE " \t\n\r"
 
-static int	tz_elevel;			/* to avoid passing this around a lot */
-
 static bool validateTzEntry(tzEntry *tzentry);
 static bool splitTzLine(const char *filename, int lineno,
 			char *line, tzEntry *tzentry);
@@ -58,20 +58,16 @@ validateTzEntry(tzEntry *tzentry)
 	 */
 	if (strlen(tzentry->abbrev) > TOKMAXLEN)
 	{
-		ereport(tz_elevel,
-				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-				 errmsg("time zone abbreviation \"%s\" is too long (maximum %d characters) in time zone file \"%s\", line %d",
-						tzentry->abbrev, TOKMAXLEN,
-						tzentry->filename, tzentry->lineno)));
+		GUC_check_errmsg("time zone abbreviation \"%s\" is too long (maximum %d characters) in time zone file \"%s\", line %d",
+						 tzentry->abbrev, TOKMAXLEN,
+						 tzentry->filename, tzentry->lineno);
 		return false;
 	}
 	if (tzentry->offset % 900 != 0)
 	{
-		ereport(tz_elevel,
-				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-				 errmsg("time zone offset %d is not a multiple of 900 sec (15 min) in time zone file \"%s\", line %d",
-						tzentry->offset,
-						tzentry->filename, tzentry->lineno)));
+		GUC_check_errmsg("time zone offset %d is not a multiple of 900 sec (15 min) in time zone file \"%s\", line %d",
+						 tzentry->offset,
+						 tzentry->filename, tzentry->lineno);
 		return false;
 	}
 
@@ -81,11 +77,9 @@ validateTzEntry(tzEntry *tzentry)
 	if (tzentry->offset > 14 * 60 * 60 ||
 		tzentry->offset < -14 * 60 * 60)
 	{
-		ereport(tz_elevel,
-				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-				 errmsg("time zone offset %d is out of range in time zone file \"%s\", line %d",
-						tzentry->offset,
-						tzentry->filename, tzentry->lineno)));
+		GUC_check_errmsg("time zone offset %d is out of range in time zone file \"%s\", line %d",
+						 tzentry->offset,
+						 tzentry->filename, tzentry->lineno);
 		return false;
 	}
 
@@ -118,10 +112,8 @@ splitTzLine(const char *filename, int lineno, char *line, tzEntry *tzentry)
 	abbrev = strtok(line, WHITESPACE);
 	if (!abbrev)
 	{
-		ereport(tz_elevel,
-				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-				 errmsg("missing time zone abbreviation in time zone file \"%s\", line %d",
-						filename, lineno)));
+		GUC_check_errmsg("missing time zone abbreviation in time zone file \"%s\", line %d",
+						 filename, lineno);
 		return false;
 	}
 	tzentry->abbrev = abbrev;
@@ -129,19 +121,15 @@ splitTzLine(const char *filename, int lineno, char *line, tzEntry *tzentry)
 	offset = strtok(NULL, WHITESPACE);
 	if (!offset)
 	{
-		ereport(tz_elevel,
-				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-		 errmsg("missing time zone offset in time zone file \"%s\", line %d",
-				filename, lineno)));
+		GUC_check_errmsg("missing time zone offset in time zone file \"%s\", line %d",
+						 filename, lineno);
 		return false;
 	}
 	tzentry->offset = strtol(offset, &offset_endptr, 10);
 	if (offset_endptr == offset || *offset_endptr != '\0')
 	{
-		ereport(tz_elevel,
-				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-				 errmsg("invalid number for time zone offset in time zone file \"%s\", line %d",
-						filename, lineno)));
+		GUC_check_errmsg("invalid number for time zone offset in time zone file \"%s\", line %d",
+						 filename, lineno);
 		return false;
 	}
 
@@ -163,10 +151,8 @@ splitTzLine(const char *filename, int lineno, char *line, tzEntry *tzentry)
 
 	if (remain[0] != '#')		/* must be a comment */
 	{
-		ereport(tz_elevel,
-				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-				 errmsg("invalid syntax in time zone file \"%s\", line %d",
-						filename, lineno)));
+		GUC_check_errmsg("invalid syntax in time zone file \"%s\", line %d",
+						 filename, lineno);
 		return false;
 	}
 	return true;
@@ -229,13 +215,11 @@ addToArray(tzEntry **base, int *arraysize, int n,
 				return n;
 			}
 			/* same abbrev but something is different, complain */
-			ereport(tz_elevel,
-					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-				  errmsg("time zone abbreviation \"%s\" is multiply defined",
-						 entry->abbrev),
-					 errdetail("Entry in time zone file \"%s\", line %d, conflicts with entry in file \"%s\", line %d.",
-							   midptr->filename, midptr->lineno,
-							   entry->filename, entry->lineno)));
+			GUC_check_errmsg("time zone abbreviation \"%s\" is multiply defined",
+							 entry->abbrev);
+			GUC_check_errdetail("Entry in time zone file \"%s\", line %d, conflicts with entry in file \"%s\", line %d.",
+								midptr->filename, midptr->lineno,
+								entry->filename, entry->lineno);
 			return -1;
 		}
 	}
@@ -296,12 +280,10 @@ ParseTzFile(const char *filename, int depth,
 	{
 		if (!isalpha((unsigned char) *p))
 		{
-			/* at level 0, we need no ereport since guc.c will say enough */
+			/* at level 0, just use guc.c's regular "invalid value" message */
 			if (depth > 0)
-				ereport(tz_elevel,
-						(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-						 errmsg("invalid time zone file name \"%s\"",
-								filename)));
+				GUC_check_errmsg("invalid time zone file name \"%s\"",
+								 filename);
 			return -1;
 		}
 	}
@@ -313,10 +295,8 @@ ParseTzFile(const char *filename, int depth,
 	 */
 	if (depth > 3)
 	{
-		ereport(tz_elevel,
-				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-			 errmsg("time zone file recursion limit exceeded in file \"%s\"",
-					filename)));
+		GUC_check_errmsg("time zone file recursion limit exceeded in file \"%s\"",
+						 filename);
 		return -1;
 	}
 
@@ -340,12 +320,10 @@ ParseTzFile(const char *filename, int depth,
 		tzdir = AllocateDir(file_path);
 		if (tzdir == NULL)
 		{
-			ereport(tz_elevel,
-					(errcode_for_file_access(),
-					 errmsg("could not open directory \"%s\": %m",
-							file_path),
-					 errhint("This may indicate an incomplete PostgreSQL installation, or that the file \"%s\" has been moved away from its proper location.",
-							 my_exec_path)));
+			GUC_check_errmsg("could not open directory \"%s\": %m",
+							 file_path);
+			GUC_check_errhint("This may indicate an incomplete PostgreSQL installation, or that the file \"%s\" has been moved away from its proper location.",
+							  my_exec_path);
 			return -1;
 		}
 		FreeDir(tzdir);
@@ -356,10 +334,8 @@ ParseTzFile(const char *filename, int depth,
 		 * complaint is enough
 		 */
 		if (errno != ENOENT || depth > 0)
-			ereport(tz_elevel,
-					(errcode_for_file_access(),
-					 errmsg("could not read time zone file \"%s\": %m",
-							filename)));
+			GUC_check_errmsg("could not read time zone file \"%s\": %m",
+							 filename);
 
 		return -1;
 	}
@@ -371,10 +347,8 @@ ParseTzFile(const char *filename, int depth,
 		{
 			if (ferror(tzFile))
 			{
-				ereport(tz_elevel,
-						(errcode_for_file_access(),
-						 errmsg("could not read time zone file \"%s\": %m",
-								filename)));
+				GUC_check_errmsg("could not read time zone file \"%s\": %m",
+								 filename);
 				return -1;
 			}
 			/* else we're at EOF after all */
@@ -383,10 +357,8 @@ ParseTzFile(const char *filename, int depth,
 		if (strlen(tzbuf) == sizeof(tzbuf) - 1)
 		{
 			/* the line is too long for tzbuf */
-			ereport(tz_elevel,
-					(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
-				 errmsg("line is too long in time zone file \"%s\", line %d",
-						filename, lineno)));
+			GUC_check_errmsg("line is too long in time zone file \"%s\", line %d",
+							 filename, lineno);
 			return -1;
 		}
 
@@ -408,10 +380,8 @@ ParseTzFile(const char *filename, int depth,
 			includeFile = strtok(includeFile, WHITESPACE);
 			if (!includeFile || !*includeFile)
 			{
-				ereport(tz_elevel,
-						(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-						 errmsg("@INCLUDE without file name in time zone file \"%s\", line %d",
-								filename, lineno)));
+				GUC_check_errmsg("@INCLUDE without file name in time zone file \"%s\", line %d",
+								 filename, lineno);
 				return -1;
 			}
 			n = ParseTzFile(includeFile, depth + 1,
@@ -444,23 +414,20 @@ ParseTzFile(const char *filename, int depth,
 /*
  * load_tzoffsets --- read and parse the specified timezone offset file
  *
- * filename: name specified by user
- * doit: whether to actually apply the new values, or just check
- * elevel: elog reporting level (will be less than ERROR)
- *
- * Returns TRUE if OK, FALSE if not; should avoid erroring out
+ * On success, return a filled-in TimeZoneAbbrevTable, which must have been
+ * malloc'd not palloc'd.  On failure, return NULL, using GUC_check_errmsg
+ * and friends to give details of the problem.
  */
-bool
-load_tzoffsets(const char *filename, bool doit, int elevel)
+TimeZoneAbbrevTable *
+load_tzoffsets(const char *filename)
 {
+	TimeZoneAbbrevTable *result = NULL;
 	MemoryContext tmpContext;
 	MemoryContext oldContext;
 	tzEntry    *array;
 	int			arraysize;
 	int			n;
 
-	tz_elevel = elevel;
-
 	/*
 	 * Create a temp memory context to work in.  This makes it easy to clean
 	 * up afterwards.
@@ -479,13 +446,20 @@ load_tzoffsets(const char *filename, bool doit, int elevel)
 	/* Parse the file(s) */
 	n = ParseTzFile(filename, 0, &array, &arraysize, 0);
 
-	/* If no errors and we should apply the result, pass it to datetime.c */
-	if (n >= 0 && doit)
-		InstallTimeZoneAbbrevs(array, n);
+	/* If no errors so far, allocate result and let datetime.c convert data */
+	if (n >= 0)
+	{
+		result = malloc(offsetof(TimeZoneAbbrevTable, abbrevs) +
+						n * sizeof(datetkn));
+		if (!result)
+			GUC_check_errmsg("out of memory");
+		else
+			ConvertTimeZoneAbbrevs(result, array, n);
+	}
 
 	/* Clean up */
 	MemoryContextSwitchTo(oldContext);
 	MemoryContextDelete(tmpContext);
 
-	return (n >= 0);
+	return result;
 }

src/include/commands/variable.h

@@ -13,31 +13,28 @@
 #include "utils/guc.h"
 
 
-extern const char *assign_datestyle(const char *value,
-				 bool doit, GucSource source);
-extern const char *assign_timezone(const char *value,
-				bool doit, GucSource source);
+extern bool check_datestyle(char **newval, void **extra, GucSource source);
+extern void assign_datestyle(const char *newval, void *extra);
+extern bool check_timezone(char **newval, void **extra, GucSource source);
+extern void assign_timezone(const char *newval, void *extra);
 extern const char *show_timezone(void);
-extern const char *assign_log_timezone(const char *value,
-					bool doit, GucSource source);
+extern bool check_log_timezone(char **newval, void **extra, GucSource source);
+extern void assign_log_timezone(const char *newval, void *extra);
 extern const char *show_log_timezone(void);
-extern bool assign_transaction_read_only(bool value,
-					bool doit, GucSource source);
-extern const char *assign_XactIsoLevel(const char *value,
-					bool doit, GucSource source);
+extern bool check_transaction_read_only(bool *newval, void **extra, GucSource source);
+extern bool check_XactIsoLevel(char **newval, void **extra, GucSource source);
+extern void assign_XactIsoLevel(const char *newval, void *extra);
 extern const char *show_XactIsoLevel(void);
-extern bool assign_transaction_deferrable(bool newval, bool doit,
-					GucSource source);
-extern bool assign_random_seed(double value,
-				   bool doit, GucSource source);
+extern bool check_transaction_deferrable(bool *newval, void **extra, GucSource source);
+extern bool check_random_seed(double *newval, void **extra, GucSource source);
+extern void assign_random_seed(double newval, void *extra);
 extern const char *show_random_seed(void);
-extern const char *assign_client_encoding(const char *value,
-					   bool doit, GucSource source);
-extern const char *assign_role(const char *value,
-			bool doit, GucSource source);
+extern bool check_client_encoding(char **newval, void **extra, GucSource source);
+extern void assign_client_encoding(const char *newval, void *extra);
+extern bool check_session_authorization(char **newval, void **extra, GucSource source);
+extern void assign_session_authorization(const char *newval, void *extra);
+extern bool check_role(char **newval, void **extra, GucSource source);
+extern void assign_role(const char *newval, void *extra);
 extern const char *show_role(void);
-extern const char *assign_session_authorization(const char *value,
-							 bool doit, GucSource source);
-extern const char *show_session_authorization(void);
 
 #endif   /* VARIABLE_H */

src/include/mb/pg_wchar.h

@@ -397,7 +397,8 @@ extern size_t wchar2char(char *to, const wchar_t *from, size_t tolen, Oid collat
 extern size_t char2wchar(wchar_t *to, size_t tolen, const char *from, size_t fromlen, Oid collation);
 #endif
 
-extern int	SetClientEncoding(int encoding, bool doit);
+extern int	PrepareClientEncoding(int encoding);
+extern int	SetClientEncoding(int encoding);
 extern void InitializeClientEncoding(void);
 extern int	pg_get_client_encoding(void);
 extern const char *pg_get_client_encoding_name(void);

src/include/replication/syncrep.h

@@ -45,6 +45,6 @@ extern void SyncRepUpdateSyncStandbysDefined(void);
 
 /* called by various procs */
 extern int SyncRepWakeQueue(bool all);
-extern const char *assign_synchronous_standby_names(const char *newval, bool doit, GucSource source);
+extern bool check_synchronous_standby_names(char **newval, void **extra, GucSource source);
 
 #endif   /* _SYNCREP_H */

src/include/tcop/tcopprot.h

@@ -57,7 +57,8 @@ extern PlannedStmt *pg_plan_query(Query *querytree, int cursorOptions,
 extern List *pg_plan_queries(List *querytrees, int cursorOptions,
 				ParamListInfo boundParams);
 
-extern bool assign_max_stack_depth(int newval, bool doit, GucSource source);
+extern bool check_max_stack_depth(int *newval, void **extra, GucSource source);
+extern void assign_max_stack_depth(int newval, void *extra);
 
 extern void die(SIGNAL_ARGS);
 extern void quickdie(SIGNAL_ARGS);

src/include/tsearch/ts_cache.h

@@ -93,6 +93,7 @@ extern TSDictionaryCacheEntry *lookup_ts_dictionary_cache(Oid dictId);
 extern TSConfigCacheEntry *lookup_ts_config_cache(Oid cfgId);
 
 extern Oid	getTSCurrentConfig(bool emitError);
-extern const char *assignTSCurrentConfig(const char *newval, bool doit, GucSource source);
+extern bool check_TSCurrentConfig(char **newval, void **extra, GucSource source);
+extern void assign_TSCurrentConfig(const char *newval, void *extra);
 
 #endif   /* TS_CACHE_H */

src/include/utils/datetime.h

@@ -20,7 +20,9 @@
 #include <math.h>
 
 #include "utils/timestamp.h"
-#include "utils/tzparser.h"
+
+/* this struct is declared in utils/tzparser.h: */
+struct tzEntry;
 
 
 /* ----------------------------------------------------------------
@@ -203,6 +205,13 @@ typedef struct
 	char		value;			/* this may be unsigned, alas */
 } datetkn;
 
+/* one of its uses is in tables of time zone abbreviations */
+typedef struct TimeZoneAbbrevTable
+{
+	int			numabbrevs;
+	datetkn		abbrevs[1];		/* VARIABLE LENGTH ARRAY */
+} TimeZoneAbbrevTable;
+
 
 /* FMODULO()
  * Macro to replace modf(), which is broken on some platforms.
@@ -317,7 +326,10 @@ extern int	DecodeUnits(int field, char *lowtoken, int *val);
 extern int	j2day(int jd);
 
 extern bool CheckDateTokenTables(void);
-extern void InstallTimeZoneAbbrevs(tzEntry *abbrevs, int n);
+
+extern void ConvertTimeZoneAbbrevs(TimeZoneAbbrevTable *tbl,
+								   struct tzEntry *abbrevs, int n);
+extern void InstallTimeZoneAbbrevs(TimeZoneAbbrevTable *tbl);
 
 extern Datum pg_timezone_abbrevs(PG_FUNCTION_ARGS);
 extern Datum pg_timezone_names(PG_FUNCTION_ARGS);

src/include/utils/elog.h

@@ -200,6 +200,15 @@ elog_finish(int elevel, const char *fmt,...)
 __attribute__((format(printf, 2, 3)));
 
 
+/* Support for constructing error strings separately from ereport() calls */
+
+extern void pre_format_elog_string(int errnumber, const char *domain);
+extern char *format_elog_string(const char *fmt,...)
+/* This extension allows gcc to check the format string for consistency with
+   the supplied arguments. */
+__attribute__((format(printf, 1, 2)));
+
+
 /* Support for attaching context information to error reports */
 
 typedef struct ErrorContextCallback

src/include/utils/guc.h

@@ -97,7 +97,8 @@ typedef enum
 } GucSource;
 
 /*
- * Parsing the configuation file will return a list of name-value pairs
+ * Parsing the configuration file will return a list of name-value pairs
+ * with source location info.
  */
 typedef struct ConfigVariable
 {
@@ -117,7 +118,9 @@ extern bool ParseConfigFp(FILE *fp, const char *config_file,
 extern void FreeConfigVariables(ConfigVariable *list);
 
 /*
- * Enum values are made up of an array of name-value pairs
+ * The possible values of an enum variable are specified by an array of
+ * name-value pairs.  The "hidden" flag means the value is accepted but
+ * won't be displayed when guc.c is asked for a list of acceptable values.
  */
 struct config_enum_entry
 {
@@ -126,15 +129,26 @@ struct config_enum_entry
 	bool		hidden;
 };
 
+/*
+ * Signatures for per-variable check/assign/show hook functions
+ */
+typedef bool (*GucBoolCheckHook) (bool *newval, void **extra, GucSource source);
+typedef bool (*GucIntCheckHook) (int *newval, void **extra, GucSource source);
+typedef bool (*GucRealCheckHook) (double *newval, void **extra, GucSource source);
+typedef bool (*GucStringCheckHook) (char **newval, void **extra, GucSource source);
+typedef bool (*GucEnumCheckHook) (int *newval, void **extra, GucSource source);
 
-typedef const char *(*GucStringAssignHook) (const char *newval, bool doit, GucSource source);
-typedef bool (*GucBoolAssignHook) (bool newval, bool doit, GucSource source);
-typedef bool (*GucIntAssignHook) (int newval, bool doit, GucSource source);
-typedef bool (*GucRealAssignHook) (double newval, bool doit, GucSource source);
-typedef bool (*GucEnumAssignHook) (int newval, bool doit, GucSource source);
+typedef void (*GucBoolAssignHook) (bool newval, void *extra);
+typedef void (*GucIntAssignHook) (int newval, void *extra);
+typedef void (*GucRealAssignHook) (double newval, void *extra);
+typedef void (*GucStringAssignHook) (const char *newval, void *extra);
+typedef void (*GucEnumAssignHook) (int newval, void *extra);
 
 typedef const char *(*GucShowHook) (void);
 
+/*
+ * Miscellaneous
+ */
 typedef enum
 {
 	/* Types of set_config_option actions */
@@ -201,7 +215,6 @@ extern char *ConfigFileName;
 extern char *HbaFileName;
 extern char *IdentFileName;
 extern char *external_pid_file;
-extern char *XactIsoLevel_string;
 
 extern char *application_name;
 
@@ -209,6 +222,9 @@ extern int	tcp_keepalives_idle;
 extern int	tcp_keepalives_interval;
 extern int	tcp_keepalives_count;
 
+/*
+ * Functions exported by guc.c
+ */
 extern void SetConfigOption(const char *name, const char *value,
 				GucContext context, GucSource source);
 
@@ -220,6 +236,7 @@ extern void DefineCustomBoolVariable(
 						 bool bootValue,
 						 GucContext context,
 						 int flags,
+						 GucBoolCheckHook check_hook,
 						 GucBoolAssignHook assign_hook,
 						 GucShowHook show_hook);
 
@@ -233,6 +250,7 @@ extern void DefineCustomIntVariable(
 						int maxValue,
 						GucContext context,
 						int flags,
+						GucIntCheckHook check_hook,
 						GucIntAssignHook assign_hook,
 						GucShowHook show_hook);
 
@@ -246,6 +264,7 @@ extern void DefineCustomRealVariable(
 						 double maxValue,
 						 GucContext context,
 						 int flags,
+						 GucRealCheckHook check_hook,
 						 GucRealAssignHook assign_hook,
 						 GucShowHook show_hook);
 
@@ -257,6 +276,7 @@ extern void DefineCustomStringVariable(
 						   const char *bootValue,
 						   GucContext context,
 						   int flags,
+						   GucStringCheckHook check_hook,
 						   GucStringAssignHook assign_hook,
 						   GucShowHook show_hook);
 
@@ -269,6 +289,7 @@ extern void DefineCustomEnumVariable(
 						 const struct config_enum_entry * options,
 						 GucContext context,
 						 int flags,
+						 GucEnumCheckHook check_hook,
 						 GucEnumAssignHook assign_hook,
 						 GucShowHook show_hook);
 
@@ -308,8 +329,6 @@ extern ArrayType *GUCArrayAdd(ArrayType *array, const char *name, const char *va
 extern ArrayType *GUCArrayDelete(ArrayType *array, const char *name);
 extern ArrayType *GUCArrayReset(ArrayType *array);
 
-extern int	GUC_complaint_elevel(GucSource source);
-
 extern void pg_timezone_abbrev_initialize(void);
 
 #ifdef EXEC_BACKEND
@@ -317,6 +336,27 @@ extern void write_nondefault_variables(GucContext context);
 extern void read_nondefault_variables(void);
 #endif
 
+/* Support for messages reported from GUC check hooks */
+
+extern PGDLLIMPORT char *GUC_check_errmsg_string;
+extern PGDLLIMPORT char *GUC_check_errdetail_string;
+extern PGDLLIMPORT char *GUC_check_errhint_string;
+
+extern void GUC_check_errcode(int sqlerrcode);
+
+#define GUC_check_errmsg \
+	pre_format_elog_string(errno, TEXTDOMAIN), \
+	GUC_check_errmsg_string = format_elog_string
+
+#define GUC_check_errdetail \
+	pre_format_elog_string(errno, TEXTDOMAIN), \
+	GUC_check_errdetail_string = format_elog_string
+
+#define GUC_check_errhint \
+	pre_format_elog_string(errno, TEXTDOMAIN), \
+	GUC_check_errhint_string = format_elog_string
+
+
 /*
  * The following functions are not in guc.c, but are declared here to avoid
  * having to include guc.h in some widely used headers that it really doesn't
@@ -324,17 +364,16 @@ extern void read_nondefault_variables(void);
  */
 
 /* in commands/tablespace.c */
-extern const char *assign_default_tablespace(const char *newval,
-						  bool doit, GucSource source);
-extern const char *assign_temp_tablespaces(const char *newval,
-						bool doit, GucSource source);
+extern bool check_default_tablespace(char **newval, void **extra, GucSource source);
+extern bool check_temp_tablespaces(char **newval, void **extra, GucSource source);
+extern void assign_temp_tablespaces(const char *newval, void *extra);
 
 /* in catalog/namespace.c */
-extern const char *assign_search_path(const char *newval,
-				   bool doit, GucSource source);
+extern bool check_search_path(char **newval, void **extra, GucSource source);
+extern void assign_search_path(const char *newval, void *extra);
 
 /* in access/transam/xlog.c */
-extern bool assign_xlog_sync_method(int newval,
-						bool doit, GucSource source);
+extern bool check_wal_buffers(int *newval, void **extra, GucSource source);
+extern void assign_xlog_sync_method(int new_sync_method, void *extra);
 
 #endif   /* GUC_H */

src/include/utils/guc_tables.h

@@ -28,7 +28,7 @@ enum config_type
 	PGC_ENUM
 };
 
-union config_var_value
+union config_var_val
 {
 	bool		boolval;
 	int			intval;
@@ -37,6 +37,16 @@ union config_var_value
 	int			enumval;
 };
 
+/*
+ * The actual value of a GUC variable can include a malloc'd opaque struct
+ * "extra", which is created by its check_hook and used by its assign_hook.
+ */
+typedef struct config_var_value
+{
+	union config_var_val val;
+	void	   *extra;
+} config_var_value;
+
 /*
  * Groupings to help organize all the run-time options for display
  */
@@ -105,8 +115,8 @@ typedef struct guc_stack
 	int			nest_level;		/* nesting depth at which we made entry */
 	GucStackState state;		/* see enum above */
 	GucSource	source;			/* source of the prior value */
-	union config_var_value prior;		/* previous value of variable */
-	union config_var_value masked;		/* SET value in a GUC_SET_LOCAL entry */
+	config_var_value prior;		/* previous value of variable */
+	config_var_value masked;	/* SET value in a GUC_SET_LOCAL entry */
 	/* masked value's source must be PGC_S_SESSION, so no need to store it */
 } GucStack;
 
@@ -116,6 +126,11 @@ typedef struct guc_stack
  * The short description should be less than 80 chars in length. Some
  * applications may use the long description as well, and will append
  * it to the short description. (separated by a newline or '. ')
+ *
+ * Note that sourcefile/sourceline are kept here, and not pushed into stacked
+ * values, although in principle they belong with some stacked value if the
+ * active value is session- or transaction-local.  This is to avoid bloating
+ * stack entries.  We know they are only relevant when source == PGC_S_FILE.
  */
 struct config_generic
 {
@@ -132,7 +147,8 @@ struct config_generic
 	GucSource	reset_source;	/* source of the reset_value */
 	GucSource	source;			/* source of the current actual value */
 	GucStack   *stack;			/* stacked prior values */
-	char	   *sourcefile;		/* file this settings is from (NULL if not
+	void	   *extra;			/* "extra" pointer for current actual value */
+	char	   *sourcefile;		/* file current setting is from (NULL if not
 								 * file) */
 	int			sourceline;		/* line in source file */
 };
@@ -155,10 +171,12 @@ struct config_bool
 	/* constant fields, must be set correctly in initial value: */
 	bool	   *variable;
 	bool		boot_val;
+	GucBoolCheckHook check_hook;
 	GucBoolAssignHook assign_hook;
 	GucShowHook show_hook;
 	/* variable fields, initialized at runtime: */
 	bool		reset_val;
+	void	   *reset_extra;
 };
 
 struct config_int
@@ -169,10 +187,12 @@ struct config_int
 	int			boot_val;
 	int			min;
 	int			max;
+	GucIntCheckHook check_hook;
 	GucIntAssignHook assign_hook;
 	GucShowHook show_hook;
 	/* variable fields, initialized at runtime: */
 	int			reset_val;
+	void	   *reset_extra;
 };
 
 struct config_real
@@ -183,10 +203,12 @@ struct config_real
 	double		boot_val;
 	double		min;
 	double		max;
+	GucRealCheckHook check_hook;
 	GucRealAssignHook assign_hook;
 	GucShowHook show_hook;
 	/* variable fields, initialized at runtime: */
 	double		reset_val;
+	void	   *reset_extra;
 };
 
 struct config_string
@@ -195,10 +217,12 @@ struct config_string
 	/* constant fields, must be set correctly in initial value: */
 	char	  **variable;
 	const char *boot_val;
+	GucStringCheckHook check_hook;
 	GucStringAssignHook assign_hook;
 	GucShowHook show_hook;
 	/* variable fields, initialized at runtime: */
 	char	   *reset_val;
+	void	   *reset_extra;
 };
 
 struct config_enum
@@ -208,10 +232,12 @@ struct config_enum
 	int		   *variable;
 	int			boot_val;
 	const struct config_enum_entry *options;
+	GucEnumCheckHook check_hook;
 	GucEnumAssignHook assign_hook;
 	GucShowHook show_hook;
 	/* variable fields, initialized at runtime: */
 	int			reset_val;
+	void	   *reset_extra;
 };
 
 /* constant tables corresponding to enums above and in guc.h */

src/include/utils/pg_locale.h

@@ -33,14 +33,14 @@ extern char *localized_abbrev_months[];
 extern char *localized_full_months[];
 
 
-extern const char *locale_messages_assign(const char *value,
-					   bool doit, GucSource source);
-extern const char *locale_monetary_assign(const char *value,
-					   bool doit, GucSource source);
-extern const char *locale_numeric_assign(const char *value,
-					  bool doit, GucSource source);
-extern const char *locale_time_assign(const char *value,
-				   bool doit, GucSource source);
+extern bool check_locale_messages(char **newval, void **extra, GucSource source);
+extern void assign_locale_messages(const char *newval, void *extra);
+extern bool check_locale_monetary(char **newval, void **extra, GucSource source);
+extern void assign_locale_monetary(const char *newval, void *extra);
+extern bool check_locale_numeric(char **newval, void **extra, GucSource source);
+extern void assign_locale_numeric(const char *newval, void *extra);
+extern bool check_locale_time(char **newval, void **extra, GucSource source);
+extern void assign_locale_time(const char *newval, void *extra);
 
 extern bool check_locale(int category, const char *locale);
 extern char *pg_perm_setlocale(int category, const char *locale);

src/include/utils/tzparser.h

@@ -13,6 +13,8 @@
 #ifndef TZPARSER_H
 #define TZPARSER_H
 
+#include "utils/datetime.h"
+
 /*
  * The result of parsing a timezone configuration file is an array of
  * these structs, in order by abbrev.  We export this because datetime.c
@@ -30,6 +32,6 @@ typedef struct tzEntry
 } tzEntry;
 
 
-extern bool load_tzoffsets(const char *filename, bool doit, int elevel);
+extern TimeZoneAbbrevTable *load_tzoffsets(const char *filename);
 
 #endif   /* TZPARSER_H */

src/pl/plperl/plperl.c

@@ -363,7 +363,7 @@ _PG_init(void)
 							 &plperl_use_strict,
 							 false,
 							 PGC_USERSET, 0,
-							 NULL, NULL);
+							 NULL, NULL, NULL);
 
 	/*
 	 * plperl.on_init is marked PGC_SIGHUP to support the idea that it might
@@ -377,7 +377,7 @@ _PG_init(void)
 							   &plperl_on_init,
 							   NULL,
 							   PGC_SIGHUP, 0,
-							   NULL, NULL);
+							   NULL, NULL, NULL);
 
 	/*
 	 * plperl.on_plperl_init is marked PGC_SUSET to avoid issues whereby a
@@ -399,7 +399,7 @@ _PG_init(void)
 							   &plperl_on_plperl_init,
 							   NULL,
 							   PGC_SUSET, 0,
-							   NULL, NULL);
+							   NULL, NULL, NULL);
 
 	DefineCustomStringVariable("plperl.on_plperlu_init",
 							   gettext_noop("Perl initialization code to execute once when plperlu is first used."),
@@ -407,7 +407,7 @@ _PG_init(void)
 							   &plperl_on_plperlu_init,
 							   NULL,
 							   PGC_SUSET, 0,
-							   NULL, NULL);
+							   NULL, NULL, NULL);
 
 	EmitWarningsOnPlaceholders("plperl");
 

src/pl/plpgsql/src/pl_handler.c

@@ -63,7 +63,7 @@ _PG_init(void)
 							 PLPGSQL_RESOLVE_ERROR,
 							 variable_conflict_options,
 							 PGC_SUSET, 0,
-							 NULL, NULL);
+							 NULL, NULL, NULL);
 
 	EmitWarningsOnPlaceholders("plpgsql");
 

src/test/regress/expected/guc.out

@@ -622,7 +622,8 @@ alter function report_guc(text) set search_path = no_such_schema;
 NOTICE:  schema "no_such_schema" does not exist
 -- with error occurring here
 select report_guc('work_mem'), current_setting('work_mem');
-ERROR:  schema "no_such_schema" does not exist
+ERROR:  invalid value for parameter "search_path": "no_such_schema"
+DETAIL:  schema "no_such_schema" does not exist
 alter function report_guc(text) reset search_path set work_mem = '2MB';
 select report_guc('work_mem'), current_setting('work_mem');
  report_guc | current_setting 

src/timezone/pgtz.c

@@ -1444,27 +1444,39 @@ pg_timezone_initialize(void)
 {
 	pg_tz	   *def_tz = NULL;
 
-	/* Do we need to try to figure the session timezone? */
-	if (pg_strcasecmp(GetConfigOption("timezone", false), "UNKNOWN") == 0)
+	/*
+	 * Make sure that session_timezone and log_timezone are set.
+	 * (session_timezone could still be NULL even if a timezone value was set
+	 * in postgresql.conf, if that setting was interval-based rather than
+	 * timezone-based.)
+	 */
+	if (!session_timezone)
 	{
-		/* Select setting */
 		def_tz = select_default_timezone();
 		session_timezone = def_tz;
-		/* Tell GUC about the value. Will redundantly call pg_tzset() */
-		SetConfigOption("timezone", pg_get_timezone_name(def_tz),
-						PGC_POSTMASTER, PGC_S_ARGV);
 	}
-
-	/* What about the log timezone? */
-	if (pg_strcasecmp(GetConfigOption("log_timezone", false), "UNKNOWN") == 0)
+	if (!log_timezone)
 	{
-		/* Select setting, but don't duplicate work */
+		/* Don't duplicate work */
 		if (!def_tz)
 			def_tz = select_default_timezone();
 		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() */
-		SetConfigOption("log_timezone", pg_get_timezone_name(def_tz),
-						PGC_POSTMASTER, PGC_S_ARGV);
+		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() */
+		SetConfigOption("log_timezone", pg_get_timezone_name(log_timezone),
+						PGC_POSTMASTER, PGC_S_ENV_VAR);
 	}
 }