Refactor GUC set_config_option function:

The main reason for refactoring was that set_config_option() was too
overloaded function and its behavior did not consistent. Old version of
set_config_function hides some messages. For example if you type:

tcp_port = 5432.1

then old implementation ignore this error without any message to log
file in the signal context (configuration reload). Main problem was that
semantic analysis of postgresql.conf is not perform in the
ProcessConfigFile function, but in the set_config_options *after*
context check. This skipped check for variables with PG_POSTMASTER
context. There was request from Joachim Wieland to add more messages
about ignored changes in the config file as well.

Zdenek Kotala

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

@@ -4,7 +4,7 @@
  *
  * Copyright (c) 2000-2006, PostgreSQL Global Development Group
  *
- * $PostgreSQL: pgsql/src/backend/utils/misc/guc-file.l,v 1.38 2006/07/27 08:30:41 petere Exp $
+ * $PostgreSQL: pgsql/src/backend/utils/misc/guc-file.l,v 1.39 2006/08/11 20:08:28 momjian Exp $
  */
 
 %{
@@ -50,7 +50,8 @@ int GUC_yylex(void);
 static bool ParseConfigFile(const char *config_file, const char *calling_file,
 							int depth, GucContext context, int elevel,
 							struct name_value_pair **head_p,
-							struct name_value_pair **tail_p);
+							struct name_value_pair **tail_p,
+							int *varcount);
 static void free_name_value_list(struct name_value_pair * list);
 static char *GUC_scanstr(const char *s);
 
@@ -114,8 +115,10 @@ STRING          \'([^'\\\n]|\\.|\'\')*\'
 void
 ProcessConfigFile(GucContext context)
 {
-	int			elevel;
+	int			elevel, i;
 	struct name_value_pair *item, *head, *tail;
+	bool	   *apply_list = NULL;
+	int			varcount = 0;
 
 	Assert(context == PGC_POSTMASTER || context == PGC_SIGHUP);
 
@@ -134,25 +137,56 @@ ProcessConfigFile(GucContext context)
 
 	if (!ParseConfigFile(ConfigFileName, NULL,
 						 0, context, elevel,
-						 &head, &tail))
+						 &head, &tail, &varcount))
 		goto cleanup_list;
 
+	/* Can we allocate memory here, what about leaving here prematurely? */
+	apply_list = (bool *) palloc(sizeof(bool) * varcount);
+
 	/* Check if all options are valid */
-	for (item = head; item; item = item->next)
+	for (item = head, i = 0; item; item = item->next, i++)
 	{
-		if (!set_config_option(item->name, item->value, context,
-							   PGC_S_FILE, false, false))
+		bool isEqual, isContextOk;
+
+		if (!verify_config_option(item->name, item->value, context,
+						  PGC_S_FILE, &isEqual, &isContextOk))
+		{
+			ereport(elevel,
+				(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
+				errmsg("configuration file is invalid")));
 			goto cleanup_list;
+		}
+
+		if( isContextOk == false )
+		{
+			apply_list[i] = false;
+			if( context == PGC_SIGHUP )
+			{
+				if ( isEqual == false )
+					ereport(elevel,
+						(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
+						errmsg("parameter \"%s\" cannot be changed after server start; configuration file change ignored",
+						item->name)));
+			}
+			else
+				/* if it is boot phase, context must be valid for all
+				 * configuration item. */
+				goto cleanup_list;
+		}
+		else
+			apply_list[i] = true;
 	}
 
 	/* If we got here all the options checked out okay, so apply them. */
-	for (item = head; item; item = item->next)
-	{
-		set_config_option(item->name, item->value, context,
-						  PGC_S_FILE, false, true);
-	}
+	for (item = head, i = 0; item; item = item->next, i++)
+		if (apply_list[i])
+			set_config_option(item->name, item->value, context,
+							  PGC_S_FILE, false, true);
+
 
- cleanup_list:
+cleanup_list:
+	if (apply_list)
+		pfree(apply_list);
 	free_name_value_list(head);
 }
 
@@ -189,13 +223,14 @@ static bool
 ParseConfigFile(const char *config_file, const char *calling_file,
 				int depth, GucContext context, int elevel,
 				struct name_value_pair **head_p,
-				struct name_value_pair **tail_p)
+				struct name_value_pair **tail_p,
+				int *varcount)
 {
-	bool		OK = true;
-	char		abs_path[MAXPGPATH];
-	FILE	   *fp;
+	bool			OK = true;
+	char			abs_path[MAXPGPATH];
+	FILE		   *fp;
 	YY_BUFFER_STATE lex_buffer;
-	int			token;
+	int				token;
 
 	/*
 	 * Reject too-deep include nesting depth.  This is just a safety check
@@ -289,7 +324,7 @@ ParseConfigFile(const char *config_file, const char *calling_file,
 
 			if (!ParseConfigFile(opt_value, config_file,
 								 depth + 1, context, elevel,
-								 head_p, tail_p))
+								 head_p, tail_p, varcount))
 			{
 				pfree(opt_name);
 				pfree(opt_value);
@@ -333,6 +368,7 @@ ParseConfigFile(const char *config_file, const char *calling_file,
 			else
 				(*tail_p)->next = item;
 			*tail_p = item;
+			(*varcount)++;
 		}
 
 		/* break out of loop if read EOF, else loop for next line */

src/backend/utils/misc/guc.c

@@ -10,7 +10,7 @@
  * Written by Peter Eisentraut <peter_e@gmx.net>.
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.333 2006/07/29 03:02:56 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.334 2006/08/11 20:08:28 momjian Exp $
  *
  *--------------------------------------------------------------------
  */
@@ -3690,96 +3690,258 @@ call_string_assign_hook(GucStringAssignHook assign_hook,
 	return result;
 }
 
-
 /*
- * Sets option `name' to given value. The value should be a string
- * which is going to be parsed and converted to the appropriate data
- * type.  The context and source parameters indicate in which context this
- * function is being called so it can apply the access restrictions
- * properly.
- *
- * If value is NULL, set the option to its default value. If the
- * parameter changeVal is false then don't really set the option but do all
- * the checks to see if it would work.
- *
- * If there is an error (non-existing option, invalid value) then an
- * ereport(ERROR) is thrown *unless* this is called in a context where we
- * don't want to ereport (currently, startup or SIGHUP config file reread).
- * In that case we write a suitable error message via ereport(DEBUG) and
- * return false. This is working around the deficiencies in the ereport
- * mechanism, so don't blame me.  In all other cases, the function
- * returns true, including cases where the input is valid but we chose
- * not to apply it because of context or source-priority considerations.
- *
- * See also SetConfigOption for an external interface.
+ * Try to parse value. Determine what is type and call related
+ * parsing function or if newval is equal to NULL, reset value 
+ * to default or bootval. If the value parsed okay return true,
+ * else false.
  */
-bool
-set_config_option(const char *name, const char *value,
-				  GucContext context, GucSource source,
-				  bool isLocal, bool changeVal)
+static bool
+parse_value(int elevel, const struct config_generic *record, 
+		const char *value, GucSource *source, bool changeVal, 
+		union config_var_value *retval)
 {
-	struct config_generic *record;
-	int			elevel;
-	bool		makeDefault;
 
-	if (context == PGC_SIGHUP || source == PGC_S_DEFAULT)
+	Assert( !(changeVal && retval==NULL) );
+	/*
+	 * Evaluate value and set variable.
+	 */
+	switch (record->vartype)
 	{
-		/*
-		 * To avoid cluttering the log, only the postmaster bleats loudly
-		 * about problems with the config file.
-		 */
-		elevel = IsUnderPostmaster ? DEBUG2 : LOG;
-	}
-	else if (source == PGC_S_DATABASE || source == PGC_S_USER)
-		elevel = INFO;
-	else
-		elevel = ERROR;
+		case PGC_BOOL:
+			{
+				struct config_bool *conf = (struct config_bool *) record;
+				bool		newval;
 
-	record = find_option(name, elevel);
-	if (record == NULL)
-	{
-		ereport(elevel,
-				(errcode(ERRCODE_UNDEFINED_OBJECT),
-			   errmsg("unrecognized configuration parameter \"%s\"", name)));
-		return false;
+				if (value)
+				{
+					if (!parse_bool(value, &newval))
+					{
+						ereport(elevel,
+								(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+						  errmsg("parameter \"%s\" requires a Boolean value",
+								 record->name)));
+						return false;
+					}
+				}
+				else
+				{
+					newval = conf->reset_val;
+					*source = conf->gen.reset_source;
+				}
+
+				if (conf->assign_hook)
+					if (!(*conf->assign_hook) (newval, changeVal, *source))
+					{
+						ereport(elevel,
+								(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+							 errmsg("invalid value for parameter \"%s\": %d",
+									record->name, (int) newval)));
+						return false;
+					}
+				if( retval != NULL )
+					retval->boolval = newval;
+				break;
+			}
+
+		case PGC_INT:
+			{
+				struct config_int *conf = (struct config_int *) record;
+				int			newval;
+
+				if (value)
+				{
+					if (!parse_int(value, &newval, conf->gen.flags))
+					{
+						ereport(elevel,
+								(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+						 errmsg("parameter \"%s\" requires an integer value",
+								record->name)));
+						return false;
+					}
+					if (newval < conf->min || newval > conf->max)
+					{
+						ereport(elevel,
+								(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+								 errmsg("%d is outside the valid range for parameter \"%s\" (%d .. %d)",
+										newval, record->name, conf->min, conf->max)));
+						return false;
+					}
+				}
+				else
+				{
+					newval = conf->reset_val;
+					*source = conf->gen.reset_source;
+				}
+
+				if (conf->assign_hook)
+					if (!(*conf->assign_hook) (newval, changeVal, *source))
+					{
+						ereport(elevel,
+								(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+							 errmsg("invalid value for parameter \"%s\": %d",
+									record->name, newval)));
+						return false;
+					}
+				if( retval != NULL )
+					retval->intval = newval;
+				break;
+			}
+
+		case PGC_REAL:
+			{
+				struct config_real *conf = (struct config_real *) record;
+				double		newval;
+
+				if (value)
+				{
+					if (!parse_real(value, &newval))
+					{
+						ereport(elevel,
+								(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+						  errmsg("parameter \"%s\" requires a numeric value",
+								 record->name)));
+						return false;
+					}
+					if (newval < conf->min || newval > conf->max)
+					{
+						ereport(elevel,
+								(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+								 errmsg("%g is outside the valid range for parameter \"%s\" (%g .. %g)",
+										newval, record->name, conf->min, conf->max)));
+						return false;
+					}
+				}
+				else
+				{
+					newval = conf->reset_val;
+					*source = conf->gen.reset_source;
+				}
+
+				if (conf->assign_hook)
+					if (!(*conf->assign_hook) (newval, changeVal, *source))
+					{
+						ereport(elevel,
+								(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+							 errmsg("invalid value for parameter \"%s\": %g",
+									record->name, newval)));
+						return false;
+					}
+				if( retval != NULL )
+					retval->realval = newval;
+				break;
+			}
+
+		case PGC_STRING:
+			{
+				struct config_string *conf = (struct config_string *) record;
+				char	   *newval;
+
+				if (value)
+				{
+					newval = guc_strdup(elevel, value);
+					if (newval == NULL)
+						return false;
+					/*
+					 * The only sort of "parsing" check we need to do is
+					 * apply truncation if GUC_IS_NAME.
+					 */
+					if (conf->gen.flags & GUC_IS_NAME)
+						truncate_identifier(newval, strlen(newval), true);
+				}
+				else if (conf->reset_val)
+				{
+					/*
+					 * We could possibly avoid strdup here, but easier to make
+					 * this case work the same as the normal assignment case.
+					 */
+					newval = guc_strdup(elevel, conf->reset_val);
+					if (newval == NULL)
+						return false;
+					*source = conf->gen.reset_source;
+				}
+				else
+				{
+					/* Nothing to reset to, as yet; so do nothing */
+					break;
+				}
+
+				if (conf->assign_hook)
+				{
+					const char *hookresult;
+
+					/*
+					 * If the hook ereports, we have to make sure we free
+					 * newval, else it will be a permanent memory leak.
+					 */
+					hookresult = call_string_assign_hook(conf->assign_hook,
+														 newval,
+														 changeVal,
+														 *source);
+					if (hookresult == NULL)
+					{
+						free(newval);
+						ereport(elevel,
+								(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+						 errmsg("invalid value for parameter \"%s\": \"%s\"",
+								record->name, value ? value : "")));
+						return false;
+					}
+					else if (hookresult != newval)
+					{
+						free(newval);
+
+						/*
+						 * Having to cast away const here is annoying, but the
+						 * alternative is to declare assign_hooks as returning
+						 * char*, which would mean they'd have to cast away
+						 * const, or as both taking and returning char*, which
+						 * doesn't seem attractive either --- we don't want
+						 * them to scribble on the passed str.
+						 */
+						newval = (char *) hookresult;
+					}
+				}
+
+				if ( !changeVal )
+					free(newval);
+				if( retval != NULL )
+					retval->stringval= newval;
+				break;
+			}
 	}
+	return true;
+}
 
-	/*
-	 * Check if the option can be set at this time. See guc.h for the precise
-	 * rules. Note that we don't want to throw errors if we're in the SIGHUP
-	 * context. In that case we just ignore the attempt and return true.
-	 */
+/*
+ * Check if the option can be set at this time. See guc.h for the precise
+ * rules. 
+ */
+static bool
+checkContext(int elevel, struct config_generic *record, GucContext context)
+{
 	switch (record->context)
 	{
 		case PGC_INTERNAL:
-			if (context == PGC_SIGHUP)
-				return true;
 			if (context != PGC_INTERNAL)
 			{
 				ereport(elevel,
 						(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
 						 errmsg("parameter \"%s\" cannot be changed",
-								name)));
+								record->name)));
 				return false;
 			}
 			break;
 		case PGC_POSTMASTER:
 			if (context == PGC_SIGHUP)
-			{
-				if (changeVal && !is_newvalue_equal(record, value))
-					ereport(elevel,
-							(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
-							 errmsg("parameter \"%s\" cannot be changed after server start; configuration file change ignored",
-									name)));
+				return false;
 
-				return true;
-			}
 			if (context != PGC_POSTMASTER)
 			{
 				ereport(elevel,
 						(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
 						 errmsg("parameter \"%s\" cannot be changed after server start",
-								name)));
+								record->name)));
 				return false;
 			}
 			break;
@@ -3789,7 +3951,7 @@ set_config_option(const char *name, const char *value,
 				ereport(elevel,
 						(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
 						 errmsg("parameter \"%s\" cannot be changed now",
-								name)));
+								record->name)));
 				return false;
 			}
 
@@ -3812,14 +3974,16 @@ set_config_option(const char *name, const char *value,
 				 * backend start.
 				 */
 				if (IsUnderPostmaster)
-					return true;
+				{
+					return false;
+				}
 			}
 			else if (context != PGC_BACKEND && context != PGC_POSTMASTER)
 			{
 				ereport(elevel,
 						(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
 						 errmsg("parameter \"%s\" cannot be set after connection start",
-								name)));
+								record->name)));
 				return false;
 			}
 			break;
@@ -3829,7 +3993,7 @@ set_config_option(const char *name, const char *value,
 				ereport(elevel,
 						(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
 						 errmsg("permission denied to set parameter \"%s\"",
-								name)));
+								record->name)));
 				return false;
 			}
 			break;
@@ -3837,6 +4001,115 @@ set_config_option(const char *name, const char *value,
 			/* always okay */
 			break;
 	}
+	return true;
+}
+
+/*
+ * Get error level for different sources and context.
+ */
+static int
+get_elevel(GucContext context, GucSource source)
+{
+	int elevel;
+	if (context == PGC_SIGHUP || source == PGC_S_DEFAULT)
+	{
+		/*
+		 * To avoid cluttering the log, only the postmaster bleats loudly
+		 * about problems with the config file.
+		 */
+		elevel = IsUnderPostmaster ? DEBUG2 : LOG;
+	}
+	else if (source == PGC_S_DATABASE || source == PGC_S_USER)
+		elevel = INFO;
+	else
+		elevel = ERROR;
+
+	return elevel;
+}
+
+/*
+ * Verify if option exists and value is valid.
+ * It is primary used for validation of items in configuration file.
+ */
+bool
+verify_config_option(const char *name, const char *value,
+				GucContext context, GucSource source,
+				bool *isNewEqual, bool *isContextOK)
+{
+	union config_var_value newval;
+	int					   elevel;
+	struct config_generic *record;
+
+	elevel = get_elevel(context, source);
+
+	record = find_option(name, elevel);
+	if (record == NULL)
+	{
+		ereport(elevel,
+				(errcode(ERRCODE_UNDEFINED_OBJECT),
+			   errmsg("unrecognized configuration parameter \"%s\"", name)));
+		return false;
+	}
+
+	if( parse_value(elevel, record, value, &source, false,  &newval) )
+	{
+		if( isNewEqual != NULL)
+			*isNewEqual = is_newvalue_equal(record, value);
+		if( isContextOK != NULL)
+			*isContextOK = checkContext(elevel, record, context);
+	}
+	else 
+		return false;
+
+    return true;
+}
+
+
+/*
+ * Sets option `name' to given value. The value should be a string
+ * which is going to be parsed and converted to the appropriate data
+ * type.  The context and source parameters indicate in which context this
+ * function is being called so it can apply the access restrictions
+ * properly.
+ *
+ * If value is NULL, set the option to its default value. If the
+ * parameter changeVal is false then don't really set the option but do all
+ * the checks to see if it would work.
+ *
+ * If there is an error (non-existing option, invalid value) then an
+ * ereport(ERROR) is thrown *unless* this is called in a context where we
+ * don't want to ereport (currently, startup or SIGHUP config file reread).
+ * In that case we write a suitable error message via ereport(DEBUG) and
+ * return false. This is working around the deficiencies in the ereport
+ * mechanism, so don't blame me.  In all other cases, the function
+ * returns true, including cases where the input is valid but we chose
+ * not to apply it because of context or source-priority considerations.
+ *
+ * See also SetConfigOption for an external interface.
+ */
+bool
+set_config_option(const char *name, const char *value,
+				  GucContext context, GucSource source,
+				  bool isLocal, bool changeVal)
+{
+	struct config_generic *record;
+	int			elevel;
+	bool		makeDefault;
+
+	elevel = get_elevel(context, source);
+
+	record = find_option(name, elevel);
+	if (record == NULL)
+	{
+		ereport(elevel,
+				(errcode(ERRCODE_UNDEFINED_OBJECT),
+			   errmsg("unrecognized configuration parameter \"%s\"", name)));
+		return false;
+	}
+
+	/* Check if change is allowed in the running context. */
+	if( !checkContext(elevel, record, context) )
+		return false;
 
 	/*
 	 * Should we set reset/stacked values?	(If so, the behavior is not
@@ -3871,33 +4144,9 @@ set_config_option(const char *name, const char *value,
 			{
 				struct config_bool *conf = (struct config_bool *) record;
 				bool		newval;
-
-				if (value)
-				{
-					if (!parse_bool(value, &newval))
-					{
-						ereport(elevel,
-								(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-						  errmsg("parameter \"%s\" requires a Boolean value",
-								 name)));
-						return false;
-					}
-				}
-				else
-				{
-					newval = conf->reset_val;
-					source = conf->gen.reset_source;
-				}
-
-				if (conf->assign_hook)
-					if (!(*conf->assign_hook) (newval, changeVal, source))
-					{
-						ereport(elevel,
-								(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-							 errmsg("invalid value for parameter \"%s\": %d",
-									name, (int) newval)));
-						return false;
-					}
+				
+				if( !parse_value(elevel, record, value, &source, changeVal, (union config_var_value*) &newval) )
+					return false; 
 
 				if (changeVal || makeDefault)
 				{
@@ -3948,40 +4197,8 @@ set_config_option(const char *name, const char *value,
 				struct config_int *conf = (struct config_int *) record;
 				int			newval;
 
-				if (value)
-				{
-					if (!parse_int(value, &newval, conf->gen.flags))
-					{
-						ereport(elevel,
-								(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-						 errmsg("parameter \"%s\" requires an integer value",
-								name)));
-						return false;
-					}
-					if (newval < conf->min || newval > conf->max)
-					{
-						ereport(elevel,
-								(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-								 errmsg("%d is outside the valid range for parameter \"%s\" (%d .. %d)",
-										newval, name, conf->min, conf->max)));
-						return false;
-					}
-				}
-				else
-				{
-					newval = conf->reset_val;
-					source = conf->gen.reset_source;
-				}
-
-				if (conf->assign_hook)
-					if (!(*conf->assign_hook) (newval, changeVal, source))
-					{
-						ereport(elevel,
-								(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-							 errmsg("invalid value for parameter \"%s\": %d",
-									name, newval)));
-						return false;
-					}
+ 				if( !parse_value(elevel, record, value, &source, changeVal, (union config_var_value*) &newval) )
+ 					return false;
 
 				if (changeVal || makeDefault)
 				{
@@ -4032,40 +4249,8 @@ set_config_option(const char *name, const char *value,
 				struct config_real *conf = (struct config_real *) record;
 				double		newval;
 
-				if (value)
-				{
-					if (!parse_real(value, &newval))
-					{
-						ereport(elevel,
-								(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-						  errmsg("parameter \"%s\" requires a numeric value",
-								 name)));
-						return false;
-					}
-					if (newval < conf->min || newval > conf->max)
-					{
-						ereport(elevel,
-								(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-								 errmsg("%g is outside the valid range for parameter \"%s\" (%g .. %g)",
-										newval, name, conf->min, conf->max)));
-						return false;
-					}
-				}
-				else
-				{
-					newval = conf->reset_val;
-					source = conf->gen.reset_source;
-				}
-
-				if (conf->assign_hook)
-					if (!(*conf->assign_hook) (newval, changeVal, source))
-					{
-						ereport(elevel,
-								(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-							 errmsg("invalid value for parameter \"%s\": %g",
-									name, newval)));
-						return false;
-					}
+				if( !parse_value(elevel, record, value, &source, changeVal, (union config_var_value*) &newval) )
+					return false; 
 
 				if (changeVal || makeDefault)
 				{
@@ -4116,71 +4301,8 @@ set_config_option(const char *name, const char *value,
 				struct config_string *conf = (struct config_string *) record;
 				char	   *newval;
 
-				if (value)
-				{
-					newval = guc_strdup(elevel, value);
-					if (newval == NULL)
-						return false;
-					/*
-					 * The only sort of "parsing" check we need to do is
-					 * apply truncation if GUC_IS_NAME.
-					 */
-					if (conf->gen.flags & GUC_IS_NAME)
-						truncate_identifier(newval, strlen(newval), true);
-				}
-				else if (conf->reset_val)
-				{
-					/*
-					 * We could possibly avoid strdup here, but easier to make
-					 * this case work the same as the normal assignment case.
-					 */
-					newval = guc_strdup(elevel, conf->reset_val);
-					if (newval == NULL)
-						return false;
-					source = conf->gen.reset_source;
-				}
-				else
-				{
-					/* Nothing to reset to, as yet; so do nothing */
-					break;
-				}
-
-				if (conf->assign_hook)
-				{
-					const char *hookresult;
-
-					/*
-					 * If the hook ereports, we have to make sure we free
-					 * newval, else it will be a permanent memory leak.
-					 */
-					hookresult = call_string_assign_hook(conf->assign_hook,
-														 newval,
-														 changeVal,
-														 source);
-					if (hookresult == NULL)
-					{
-						free(newval);
-						ereport(elevel,
-								(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-						 errmsg("invalid value for parameter \"%s\": \"%s\"",
-								name, value ? value : "")));
-						return false;
-					}
-					else if (hookresult != newval)
-					{
-						free(newval);
-
-						/*
-						 * Having to cast away const here is annoying, but the
-						 * alternative is to declare assign_hooks as returning
-						 * char*, which would mean they'd have to cast away
-						 * const, or as both taking and returning char*, which
-						 * doesn't seem attractive either --- we don't want
-						 * them to scribble on the passed str.
-						 */
-						newval = (char *) hookresult;
-					}
-				}
+				if( !parse_value(elevel, record, value, &source, changeVal, (union config_var_value*) &newval) )
+					return false; 
 
 				if (changeVal || makeDefault)
 				{
@@ -4228,7 +4350,8 @@ set_config_option(const char *name, const char *value,
 					}
 				}
 				else
-					free(newval);
+					if( newval != NULL )
+						free(newval);
 				break;
 			}
 	}
@@ -5314,6 +5437,9 @@ _ShowOption(struct config_generic * record, bool use_units)
 static bool
 is_newvalue_equal(struct config_generic *record, const char *newvalue)
 {
+	if( !newvalue )
+		return false;
+  
 	switch (record->vartype)
 	{
 		case PGC_BOOL:

src/include/utils/guc.h

@@ -7,7 +7,7 @@
  * Copyright (c) 2000-2006, PostgreSQL Global Development Group
  * Written by Peter Eisentraut <peter_e@gmx.net>.
  *
- * $PostgreSQL: pgsql/src/include/utils/guc.h,v 1.71 2006/07/29 03:02:56 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/utils/guc.h,v 1.72 2006/08/11 20:08:28 momjian Exp $
  *--------------------------------------------------------------------
  */
 #ifndef GUC_H
@@ -193,6 +193,9 @@ extern void ParseLongOption(const char *string, char **name, char **value);
 extern bool set_config_option(const char *name, const char *value,
 				  GucContext context, GucSource source,
 				  bool isLocal, bool changeVal);
+extern bool verify_config_option(const char *name, const char *value,
+				GucContext context, GucSource source,
+				bool *isNewEqual, bool *isContextOK);
 extern char *GetConfigOptionByName(const char *name, const char **varname);
 extern void GetConfigOptionByNum(int varnum, const char **values, bool *noshow);
 extern int	GetNumConfigOptions(void);