psql-var-hooks-v2.patch

diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index a9a2fdb..61b2304 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -254,7 +254,7 @@ exec_command(const char *cmd,
 		if (opt1 != NULL && strncmp(opt1, prefix, sizeof(prefix) - 1) == 0)
 		{
 			reuse_previous =
-				ParseVariableBool(opt1 + sizeof(prefix) - 1, prefix) ?
+				ParseVariableBool(opt1 + sizeof(prefix) - 1, prefix, NULL) ?
 				TRI_YES : TRI_NO;
 
 			free(opt1);
@@ -1548,7 +1548,7 @@ exec_command(const char *cmd,
 												 OT_NORMAL, NULL, false);
 
 		if (opt)
-			pset.timing = ParseVariableBool(opt, "\\timing");
+			pset.timing = ParseVariableBool(opt, "\\timing", NULL);
 		else
 			pset.timing = !pset.timing;
 		if (!pset.quiet)
@@ -2660,7 +2660,7 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet)
 		if (value && pg_strcasecmp(value, "auto") == 0)
 			popt->topt.expanded = 2;
 		else if (value)
-			popt->topt.expanded = ParseVariableBool(value, param);
+			popt->topt.expanded = ParseVariableBool(value, param, NULL);
 		else
 			popt->topt.expanded = !popt->topt.expanded;
 	}
@@ -2669,7 +2669,7 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet)
 	else if (strcmp(param, "numericlocale") == 0)
 	{
 		if (value)
-			popt->topt.numericLocale = ParseVariableBool(value, param);
+			popt->topt.numericLocale = ParseVariableBool(value, param, NULL);
 		else
 			popt->topt.numericLocale = !popt->topt.numericLocale;
 	}
@@ -2724,7 +2724,7 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet)
 	else if (strcmp(param, "t") == 0 || strcmp(param, "tuples_only") == 0)
 	{
 		if (value)
-			popt->topt.tuples_only = ParseVariableBool(value, param);
+			popt->topt.tuples_only = ParseVariableBool(value, param, NULL);
 		else
 			popt->topt.tuples_only = !popt->topt.tuples_only;
 	}
@@ -2756,7 +2756,7 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet)
 			popt->topt.pager = 2;
 		else if (value)
 		{
-			if (ParseVariableBool(value, param))
+			if (ParseVariableBool(value, param, NULL))
 				popt->topt.pager = 1;
 			else
 				popt->topt.pager = 0;
@@ -2778,7 +2778,7 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet)
 	else if (strcmp(param, "footer") == 0)
 	{
 		if (value)
-			popt->topt.default_footer = ParseVariableBool(value, param);
+			popt->topt.default_footer = ParseVariableBool(value, param, NULL);
 		else
 			popt->topt.default_footer = !popt->topt.default_footer;
 	}
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
index 7ce05fb..9dcfc0a 100644
--- a/src/bin/psql/startup.c
+++ b/src/bin/psql/startup.c
@@ -786,43 +786,59 @@ showVersion(void)
  * This isn't an amazingly good place for them, but neither is anywhere else.
  */
 
-static void
+/*
+ * Hook to set an internal flag from a user-supplied string value.
+ * If the syntax is correct, affect *flag and return true.
+ * Otherwise, keep *flag untouched and return false.
+ */
+static bool
+generic_boolean_hook(const char *newval, const char* varname, bool *flag)
+{
+	bool isvalid;
+	bool val = ParseVariableBool(newval, varname, &isvalid);
+	if (isvalid)
+		*flag = val;
+	return isvalid;
+}
+
+static bool
 autocommit_hook(const char *newval)
 {
-	pset.autocommit = ParseVariableBool(newval, "AUTOCOMMIT");
+	return generic_boolean_hook(newval, "AUTOCOMMIT", &pset.autocommit);
 }
 
-static void
+static bool
 on_error_stop_hook(const char *newval)
 {
-	pset.on_error_stop = ParseVariableBool(newval, "ON_ERROR_STOP");
+	return generic_boolean_hook(newval, "ON_ERROR_STOP", &pset.on_error_stop);
 }
 
-static void
+static bool
 quiet_hook(const char *newval)
 {
-	pset.quiet = ParseVariableBool(newval, "QUIET");
+	return generic_boolean_hook(newval, "QUIET", &pset.quiet);
 }
 
-static void
+static bool
 singleline_hook(const char *newval)
 {
-	pset.singleline = ParseVariableBool(newval, "SINGLELINE");
+	return generic_boolean_hook(newval, "SINGLELINE", &pset.singleline);
 }
 
-static void
+static bool
 singlestep_hook(const char *newval)
 {
-	pset.singlestep = ParseVariableBool(newval, "SINGLESTEP");
+	return generic_boolean_hook(newval, "SINGLESTEP", &pset.singlestep);
 }
 
-static void
+static bool
 fetch_count_hook(const char *newval)
 {
 	pset.fetch_count = ParseVariableNum(newval, -1, -1, false);
+	return true;
 }
 
-static void
+static bool
 echo_hook(const char *newval)
 {
 	if (newval == NULL)
@@ -837,39 +853,52 @@ echo_hook(const char *newval)
 		pset.echo = PSQL_ECHO_NONE;
 	else
 	{
-		psql_error("unrecognized value \"%s\" for \"%s\"; assuming \"%s\"\n",
-				   newval, "ECHO", "none");
-		pset.echo = PSQL_ECHO_NONE;
+		psql_error("unrecognized value \"%s\" for \"%s\"\n",
+				   newval, "ECHO");
+		return false;
 	}
+	return true;
 }
 
-static void
+static bool
 echo_hidden_hook(const char *newval)
 {
 	if (newval == NULL)
 		pset.echo_hidden = PSQL_ECHO_HIDDEN_OFF;
 	else if (pg_strcasecmp(newval, "noexec") == 0)
 		pset.echo_hidden = PSQL_ECHO_HIDDEN_NOEXEC;
-	else if (ParseVariableBool(newval, "ECHO_HIDDEN"))
-		pset.echo_hidden = PSQL_ECHO_HIDDEN_ON;
-	else	/* ParseVariableBool printed msg if needed */
-		pset.echo_hidden = PSQL_ECHO_HIDDEN_OFF;
+	else
+	{
+		bool isvalid;
+		bool val = ParseVariableBool(newval, "ECHO_HIDDEN", &isvalid);
+		if (!isvalid)
+			return false; /* ParseVariableBool printed msg */
+		pset.echo_hidden = val ? PSQL_ECHO_HIDDEN_ON : PSQL_ECHO_HIDDEN_OFF;
+	}
+	return true;
 }
 
-static void
+static bool
 on_error_rollback_hook(const char *newval)
 {
 	if (newval == NULL)
 		pset.on_error_rollback = PSQL_ERROR_ROLLBACK_OFF;
 	else if (pg_strcasecmp(newval, "interactive") == 0)
 		pset.on_error_rollback = PSQL_ERROR_ROLLBACK_INTERACTIVE;
-	else if (ParseVariableBool(newval, "ON_ERROR_ROLLBACK"))
-		pset.on_error_rollback = PSQL_ERROR_ROLLBACK_ON;
-	else	/* ParseVariableBool printed msg if needed */
-		pset.on_error_rollback = PSQL_ERROR_ROLLBACK_OFF;
+	else
+	{
+		bool isvalid;
+		bool val = ParseVariableBool(newval, "ON_ERROR_ROLLBACK", &isvalid);
+		if (isvalid)
+			pset.on_error_rollback = val ? PSQL_ERROR_ROLLBACK_ON : PSQL_ERROR_ROLLBACK_OFF;
+		else
+			/* ParseVariableBool printed msg if needed */
+			return false;
+	}
+	return true;
 }
 
-static void
+static bool
 comp_keyword_case_hook(const char *newval)
 {
 	if (newval == NULL)
@@ -884,13 +913,14 @@ comp_keyword_case_hook(const char *newval)
 		pset.comp_case = PSQL_COMP_CASE_LOWER;
 	else
 	{
-		psql_error("unrecognized value \"%s\" for \"%s\"; assuming \"%s\"\n",
-				   newval, "COMP_KEYWORD_CASE", "preserve-upper");
-		pset.comp_case = PSQL_COMP_CASE_PRESERVE_UPPER;
+		psql_error("unrecognized value \"%s\" for \"%s\"\n",
+				   newval, "COMP_KEYWORD_CASE");
+		return false;
 	}
+	return true;
 }
 
-static void
+static bool
 histcontrol_hook(const char *newval)
 {
 	if (newval == NULL)
@@ -905,31 +935,35 @@ histcontrol_hook(const char *newval)
 		pset.histcontrol = hctl_none;
 	else
 	{
-		psql_error("unrecognized value \"%s\" for \"%s\"; assuming \"%s\"\n",
-				   newval, "HISTCONTROL", "none");
-		pset.histcontrol = hctl_none;
+		psql_error("unrecognized value \"%s\" for \"%s\"\n",
+				   newval, "HISTCONTROL");
+		return false;
 	}
+	return true;
 }
 
-static void
+static bool
 prompt1_hook(const char *newval)
 {
 	pset.prompt1 = newval ? newval : "";
+	return true;
 }
 
-static void
+static bool
 prompt2_hook(const char *newval)
 {
 	pset.prompt2 = newval ? newval : "";
+	return true;
 }
 
-static void
+static bool
 prompt3_hook(const char *newval)
 {
 	pset.prompt3 = newval ? newval : "";
+	return true;
 }
 
-static void
+static bool
 verbosity_hook(const char *newval)
 {
 	if (newval == NULL)
@@ -942,16 +976,17 @@ verbosity_hook(const char *newval)
 		pset.verbosity = PQERRORS_VERBOSE;
 	else
 	{
-		psql_error("unrecognized value \"%s\" for \"%s\"; assuming \"%s\"\n",
-				   newval, "VERBOSITY", "default");
-		pset.verbosity = PQERRORS_DEFAULT;
+		psql_error("unrecognized value \"%s\" for \"%s\"\n",
+				   newval, "VERBOSITY");
+		return false;
 	}
 
 	if (pset.db)
 		PQsetErrorVerbosity(pset.db, pset.verbosity);
+	return true;
 }
 
-static void
+static bool
 show_context_hook(const char *newval)
 {
 	if (newval == NULL)
@@ -964,13 +999,14 @@ show_context_hook(const char *newval)
 		pset.show_context = PQSHOW_CONTEXT_ALWAYS;
 	else
 	{
-		psql_error("unrecognized value \"%s\" for \"%s\"; assuming \"%s\"\n",
-				   newval, "SHOW_CONTEXT", "errors");
-		pset.show_context = PQSHOW_CONTEXT_ERRORS;
+		psql_error("unrecognized value \"%s\" for \"%s\"\n",
+				   newval, "SHOW_CONTEXT");
+		return false;
 	}
 
 	if (pset.db)
 		PQsetErrorContextVisibility(pset.db, pset.show_context);
+	return true;
 }
 
 
diff --git a/src/bin/psql/variables.c b/src/bin/psql/variables.c
index f43f418..24ca750 100644
--- a/src/bin/psql/variables.c
+++ b/src/bin/psql/variables.c
@@ -86,12 +86,16 @@ GetVariable(VariableSpace space, const char *name)
  *
  * "name" is the name of the variable we're assigning to, to use in error
  * report if any.  Pass name == NULL to suppress the error report.
+ *
+ * "*valid" reports whether "value" was syntactically valid, unless valid == NULL
  */
 bool
-ParseVariableBool(const char *value, const char *name)
+ParseVariableBool(const char *value, const char *name, bool *valid)
 {
 	size_t		len;
 
+	if (valid)
+		*valid = true;
 	if (value == NULL)
 		return false;			/* not set -> assume "off" */
 
@@ -116,10 +120,16 @@ ParseVariableBool(const char *value, const char *name)
 		return false;
 	else
 	{
-		/* NULL is treated as false, so a non-matching value is 'true' */
+		/*
+		 * NULL is treated as false, so a non-matching value is 'true'.
+		 * A caller that cares about syntactic conformance should
+		 * check *valid to know whether the value was recognized.
+		 */
 		if (name)
-			psql_error("unrecognized value \"%s\" for \"%s\"; assuming \"%s\"\n",
-					   value, name, "on");
+			psql_error("unrecognized value \"%s\" for \"%s\": boolean expected\n",
+					   value, name);
+		if (valid)
+			*valid = false;
 		return true;
 	}
 }
@@ -205,13 +215,19 @@ SetVariable(VariableSpace space, const char *name, const char *value)
 	{
 		if (strcmp(current->name, name) == 0)
 		{
-			/* found entry, so update */
-			if (current->value)
-				free(current->value);
-			current->value = pg_strdup(value);
+			/* found entry, so update, unless a hook returns false */
+			bool confirmed = true;
 			if (current->assign_hook)
-				(*current->assign_hook) (current->value);
-			return true;
+			{
+				confirmed = (*current->assign_hook) (value);
+			}
+			if (confirmed)
+			{
+				if (current->value)
+					free(current->value);
+				current->value = pg_strdup(value);
+			}
+			return confirmed;
 		}
 	}
 
@@ -248,7 +264,7 @@ SetVariableAssignHook(VariableSpace space, const char *name, VariableAssignHook
 		{
 			/* found entry, so update */
 			current->assign_hook = hook;
-			(*hook) (current->value);
+			(void)(*hook) (current->value); /* ignore return value */
 			return true;
 		}
 	}
@@ -260,7 +276,7 @@ SetVariableAssignHook(VariableSpace space, const char *name, VariableAssignHook
 	current->assign_hook = hook;
 	current->next = NULL;
 	previous->next = current;
-	(*hook) (NULL);
+	(void)(*hook) (NULL);		/* ignore return value */
 	return true;
 }
 
diff --git a/src/bin/psql/variables.h b/src/bin/psql/variables.h
index d7a05a1..9836fc5 100644
--- a/src/bin/psql/variables.h
+++ b/src/bin/psql/variables.h
@@ -20,7 +20,7 @@
  * Note: if value == NULL then the variable is logically unset, but we are
  * keeping the struct around so as not to forget about its hook function.
  */
-typedef void (*VariableAssignHook) (const char *newval);
+typedef bool (*VariableAssignHook) (const char *newval);
 
 struct _variable
 {
@@ -35,7 +35,7 @@ typedef struct _variable *VariableSpace;
 VariableSpace CreateVariableSpace(void);
 const char *GetVariable(VariableSpace space, const char *name);
 
-bool		ParseVariableBool(const char *value, const char *name);
+bool		ParseVariableBool(const char *value, const char *name, bool *valid);
 int ParseVariableNum(const char *val,
 				 int defaultval,
 				 int faultval,