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

1 files changed, 30 insertions, 4 deletions

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 */