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".

11 files changed, 147 insertions, 59 deletions

diff --git a/src/include/commands/variable.h b/src/include/commands/variable.h
index 39bccbd5bf6..1d904ff984b 100644
--- a/src/include/commands/variable.h
+++ b/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 */
diff --git a/src/include/mb/pg_wchar.h b/src/include/mb/pg_wchar.h
index 85a7b2f87dd..8efc6d30464 100644
--- a/src/include/mb/pg_wchar.h
+++ b/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);
diff --git a/src/include/replication/syncrep.h b/src/include/replication/syncrep.h
index 9022b965855..728e2c8f2d2 100644
--- a/src/include/replication/syncrep.h
+++ b/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 */
diff --git a/src/include/tcop/tcopprot.h b/src/include/tcop/tcopprot.h
index bf21cd9d06f..d5192d98558 100644
--- a/src/include/tcop/tcopprot.h
+++ b/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);
diff --git a/src/include/tsearch/ts_cache.h b/src/include/tsearch/ts_cache.h
index 5ae38b2a43f..01550eddfe3 100644
--- a/src/include/tsearch/ts_cache.h
+++ b/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 */
diff --git a/src/include/utils/datetime.h b/src/include/utils/datetime.h
index 11672458f4c..9911d207936 100644
--- a/src/include/utils/datetime.h
+++ b/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);
diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h
index 71a1cdbd782..1f2a9f53c14 100644
--- a/src/include/utils/elog.h
+++ b/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
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index c07f263ea07..452310f2bdf 100644
--- a/src/include/utils/guc.h
+++ b/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;
 };
 
-
-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);
+/*
+ * 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 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 */
diff --git a/src/include/utils/guc_tables.h b/src/include/utils/guc_tables.h
index 073b77d28ce..a1ca012ef9a 100644
--- a/src/include/utils/guc_tables.h
+++ b/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;
@@ -38,6 +38,16 @@ union config_var_value
 };
 
 /*
+ * 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
  */
 enum config_group
@@ -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 */
diff --git a/src/include/utils/pg_locale.h b/src/include/utils/pg_locale.h
index 4c72fd0dc6c..25b9d509150 100644
--- a/src/include/utils/pg_locale.h
+++ b/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);
diff --git a/src/include/utils/tzparser.h b/src/include/utils/tzparser.h
index ba18819ef59..2065078116a 100644
--- a/src/include/utils/tzparser.h
+++ b/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 */