Improve consistency of parsing of psql's magic variables.

For simple boolean variables such as ON_ERROR_STOP, psql has for a long
time recognized variant spellings of "on" and "off" (such as "1"/"0"),
and it also made a point of warning you if you'd misspelled the setting.
But these conveniences did not exist for other keyword-valued variables.
In particular, though ECHO_HIDDEN and ON_ERROR_ROLLBACK include "on" and
"off" as possible values, none of the alternative spellings for those were
recognized; and to make matters worse the code would just silently assume
"on" was meant for any unrecognized spelling.  Several people have reported
getting bitten by this, so let's fix it.  In detail, this patch:

* Allows all spellings recognized by ParseVariableBool() for ECHO_HIDDEN
and ON_ERROR_ROLLBACK.

* Reports a warning for unrecognized values for COMP_KEYWORD_CASE, ECHO,
ECHO_HIDDEN, HISTCONTROL, ON_ERROR_ROLLBACK, and VERBOSITY.

* Recognizes all values for all these variables case-insensitively;
previously there was a mishmash of case-sensitive and case-insensitive
behaviors.

Back-patch to all supported branches.  There is a small risk of breaking
existing scripts that were accidentally failing to malfunction; but the
consensus is that the chance of detecting real problems and preventing
future mistakes outweighs this.

6 files changed, 103 insertions, 56 deletions

diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 39bd3617b94..8f8c7859b72 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1359,7 +1359,7 @@ exec_command(const char *cmd,
 												 OT_NORMAL, NULL, false);
 
 		if (opt)
-			pset.timing = ParseVariableBool(opt);
+			pset.timing = ParseVariableBool(opt, "\\timing");
 		else
 			pset.timing = !pset.timing;
 		if (!pset.quiet)
@@ -2323,12 +2323,14 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet)
 	}
 
 	/* set expanded/vertical mode */
-	else if (strcmp(param, "x") == 0 || strcmp(param, "expanded") == 0 || strcmp(param, "vertical") == 0)
+	else if (strcmp(param, "x") == 0 ||
+			 strcmp(param, "expanded") == 0 ||
+			 strcmp(param, "vertical") == 0)
 	{
 		if (value && pg_strcasecmp(value, "auto") == 0)
 			popt->topt.expanded = 2;
 		else if (value)
-			popt->topt.expanded = ParseVariableBool(value);
+			popt->topt.expanded = ParseVariableBool(value, param);
 		else
 			popt->topt.expanded = !popt->topt.expanded;
 	}
@@ -2337,7 +2339,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);
+			popt->topt.numericLocale = ParseVariableBool(value, param);
 		else
 			popt->topt.numericLocale = !popt->topt.numericLocale;
 	}
@@ -2392,7 +2394,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);
+			popt->topt.tuples_only = ParseVariableBool(value, param);
 		else
 			popt->topt.tuples_only = !popt->topt.tuples_only;
 	}
@@ -2423,10 +2425,12 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet)
 		if (value && pg_strcasecmp(value, "always") == 0)
 			popt->topt.pager = 2;
 		else if (value)
-			if (ParseVariableBool(value))
+		{
+			if (ParseVariableBool(value, param))
 				popt->topt.pager = 1;
 			else
 				popt->topt.pager = 0;
+		}
 		else if (popt->topt.pager == 1)
 			popt->topt.pager = 0;
 		else
@@ -2437,7 +2441,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);
+			popt->topt.default_footer = ParseVariableBool(value, param);
 		else
 			popt->topt.default_footer = !popt->topt.default_footer;
 	}
diff --git a/src/bin/psql/settings.h b/src/bin/psql/settings.h
index 0a60e6817b2..5e3e92a8e98 100644
--- a/src/bin/psql/settings.h
+++ b/src/bin/psql/settings.h
@@ -27,6 +27,11 @@
 #define DEFAULT_PROMPT2 "%/%R%# "
 #define DEFAULT_PROMPT3 ">> "
 
+/*
+ * Note: these enums should generally be chosen so that zero corresponds
+ * to the default behavior.
+ */
+
 typedef enum
 {
 	PSQL_ECHO_NONE,
@@ -50,6 +55,14 @@ typedef enum
 
 typedef enum
 {
+	PSQL_COMP_CASE_PRESERVE_UPPER,
+	PSQL_COMP_CASE_PRESERVE_LOWER,
+	PSQL_COMP_CASE_UPPER,
+	PSQL_COMP_CASE_LOWER
+} PSQL_COMP_CASE;
+
+typedef enum
+{
 	hctl_none = 0,
 	hctl_ignorespace = 1,
 	hctl_ignoredups = 2,
@@ -108,6 +121,7 @@ typedef struct _psqlSettings
 	PSQL_ECHO	echo;
 	PSQL_ECHO_HIDDEN echo_hidden;
 	PSQL_ERROR_ROLLBACK on_error_rollback;
+	PSQL_COMP_CASE comp_case;
 	HistControl histcontrol;
 	const char *prompt1;
 	const char *prompt2;
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
index 45653a15a80..2ab5690e547 100644
--- a/src/bin/psql/startup.c
+++ b/src/bin/psql/startup.c
@@ -680,31 +680,31 @@ showVersion(void)
 static void
 autocommit_hook(const char *newval)
 {
-	pset.autocommit = ParseVariableBool(newval);
+	pset.autocommit = ParseVariableBool(newval, "AUTOCOMMIT");
 }
 
 static void
 on_error_stop_hook(const char *newval)
 {
-	pset.on_error_stop = ParseVariableBool(newval);
+	pset.on_error_stop = ParseVariableBool(newval, "ON_ERROR_STOP");
 }
 
 static void
 quiet_hook(const char *newval)
 {
-	pset.quiet = ParseVariableBool(newval);
+	pset.quiet = ParseVariableBool(newval, "QUIET");
 }
 
 static void
 singleline_hook(const char *newval)
 {
-	pset.singleline = ParseVariableBool(newval);
+	pset.singleline = ParseVariableBool(newval, "SINGLELINE");
 }
 
 static void
 singlestep_hook(const char *newval)
 {
-	pset.singlestep = ParseVariableBool(newval);
+	pset.singlestep = ParseVariableBool(newval, "SINGLESTEP");
 }
 
 static void
@@ -718,12 +718,18 @@ echo_hook(const char *newval)
 {
 	if (newval == NULL)
 		pset.echo = PSQL_ECHO_NONE;
-	else if (strcmp(newval, "queries") == 0)
+	else if (pg_strcasecmp(newval, "queries") == 0)
 		pset.echo = PSQL_ECHO_QUERIES;
-	else if (strcmp(newval, "all") == 0)
+	else if (pg_strcasecmp(newval, "all") == 0)
 		pset.echo = PSQL_ECHO_ALL;
+	else if (pg_strcasecmp(newval, "none") == 0)
+		pset.echo = PSQL_ECHO_NONE;
 	else
+	{
+		psql_error("unrecognized value \"%s\" for \"%s\"; assuming \"%s\"\n",
+				   newval, "ECHO", "none");
 		pset.echo = PSQL_ECHO_NONE;
+	}
 }
 
 static void
@@ -731,12 +737,12 @@ echo_hidden_hook(const char *newval)
 {
 	if (newval == NULL)
 		pset.echo_hidden = PSQL_ECHO_HIDDEN_OFF;
-	else if (strcmp(newval, "noexec") == 0)
+	else if (pg_strcasecmp(newval, "noexec") == 0)
 		pset.echo_hidden = PSQL_ECHO_HIDDEN_NOEXEC;
-	else if (pg_strcasecmp(newval, "off") == 0)
-		pset.echo_hidden = PSQL_ECHO_HIDDEN_OFF;
-	else
+	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;
 }
 
 static void
@@ -746,10 +752,31 @@ on_error_rollback_hook(const char *newval)
 		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 (pg_strcasecmp(newval, "off") == 0)
+	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;
+}
+
+static void
+comp_keyword_case_hook(const char *newval)
+{
+	if (newval == NULL)
+		pset.comp_case = PSQL_COMP_CASE_PRESERVE_UPPER;
+	else if (pg_strcasecmp(newval, "preserve-upper") == 0)
+		pset.comp_case = PSQL_COMP_CASE_PRESERVE_UPPER;
+	else if (pg_strcasecmp(newval, "preserve-lower") == 0)
+		pset.comp_case = PSQL_COMP_CASE_PRESERVE_LOWER;
+	else if (pg_strcasecmp(newval, "upper") == 0)
+		pset.comp_case = PSQL_COMP_CASE_UPPER;
+	else if (pg_strcasecmp(newval, "lower") == 0)
+		pset.comp_case = PSQL_COMP_CASE_LOWER;
 	else
-		pset.on_error_rollback = PSQL_ERROR_ROLLBACK_ON;
+	{
+		psql_error("unrecognized value \"%s\" for \"%s\"; assuming \"%s\"\n",
+				   newval, "COMP_KEYWORD_CASE", "preserve-upper");
+		pset.comp_case = PSQL_COMP_CASE_PRESERVE_UPPER;
+	}
 }
 
 static void
@@ -757,14 +784,20 @@ histcontrol_hook(const char *newval)
 {
 	if (newval == NULL)
 		pset.histcontrol = hctl_none;
-	else if (strcmp(newval, "ignorespace") == 0)
+	else if (pg_strcasecmp(newval, "ignorespace") == 0)
 		pset.histcontrol = hctl_ignorespace;
-	else if (strcmp(newval, "ignoredups") == 0)
+	else if (pg_strcasecmp(newval, "ignoredups") == 0)
 		pset.histcontrol = hctl_ignoredups;
-	else if (strcmp(newval, "ignoreboth") == 0)
+	else if (pg_strcasecmp(newval, "ignoreboth") == 0)
 		pset.histcontrol = hctl_ignoreboth;
+	else if (pg_strcasecmp(newval, "none") == 0)
+		pset.histcontrol = hctl_none;
 	else
+	{
+		psql_error("unrecognized value \"%s\" for \"%s\"; assuming \"%s\"\n",
+				   newval, "HISTCONTROL", "none");
 		pset.histcontrol = hctl_none;
+	}
 }
 
 static void
@@ -790,14 +823,18 @@ verbosity_hook(const char *newval)
 {
 	if (newval == NULL)
 		pset.verbosity = PQERRORS_DEFAULT;
-	else if (strcmp(newval, "default") == 0)
+	else if (pg_strcasecmp(newval, "default") == 0)
 		pset.verbosity = PQERRORS_DEFAULT;
-	else if (strcmp(newval, "terse") == 0)
+	else if (pg_strcasecmp(newval, "terse") == 0)
 		pset.verbosity = PQERRORS_TERSE;
-	else if (strcmp(newval, "verbose") == 0)
+	else if (pg_strcasecmp(newval, "verbose") == 0)
 		pset.verbosity = PQERRORS_VERBOSE;
 	else
+	{
+		psql_error("unrecognized value \"%s\" for \"%s\"; assuming \"%s\"\n",
+				   newval, "VERBOSITY", "default");
 		pset.verbosity = PQERRORS_DEFAULT;
+	}
 
 	if (pset.db)
 		PQsetErrorVerbosity(pset.db, pset.verbosity);
@@ -818,6 +855,7 @@ EstablishVariableSpace(void)
 	SetVariableAssignHook(pset.vars, "ECHO", echo_hook);
 	SetVariableAssignHook(pset.vars, "ECHO_HIDDEN", echo_hidden_hook);
 	SetVariableAssignHook(pset.vars, "ON_ERROR_ROLLBACK", on_error_rollback_hook);
+	SetVariableAssignHook(pset.vars, "COMP_KEYWORD_CASE", comp_keyword_case_hook);
 	SetVariableAssignHook(pset.vars, "HISTCONTROL", histcontrol_hook);
 	SetVariableAssignHook(pset.vars, "PROMPT1", prompt1_hook);
 	SetVariableAssignHook(pset.vars, "PROMPT2", prompt2_hook);
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 6126f7407ae..9841c1ab745 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -4046,7 +4046,7 @@ complete_from_files(const char *text, int state)
 
 /*
  * Make a pg_strdup copy of s and convert the case according to
- * COMP_KEYWORD_CASE variable, using ref as the text that was already entered.
+ * COMP_KEYWORD_CASE setting, using ref as the text that was already entered.
  */
 static char *
 pg_strdup_keyword_case(const char *s, const char *ref)
@@ -4054,38 +4054,22 @@ pg_strdup_keyword_case(const char *s, const char *ref)
 	char	   *ret,
 			   *p;
 	unsigned char first = ref[0];
-	int			tocase;
-	const char *varval;
-
-	varval = GetVariable(pset.vars, "COMP_KEYWORD_CASE");
-	if (!varval)
-		tocase = 0;
-	else if (strcmp(varval, "lower") == 0)
-		tocase = -2;
-	else if (strcmp(varval, "preserve-lower") == 0)
-		tocase = -1;
-	else if (strcmp(varval, "preserve-upper") == 0)
-		tocase = +1;
-	else if (strcmp(varval, "upper") == 0)
-		tocase = +2;
-	else
-		tocase = 0;
-
-	/* default */
-	if (tocase == 0)
-		tocase = +1;
 
 	ret = pg_strdup(s);
 
-	if (tocase == -2
-		|| ((tocase == -1 || tocase == +1) && islower(first))
-		|| (tocase == -1 && !isalpha(first))
-		)
+	if (pset.comp_case == PSQL_COMP_CASE_LOWER ||
+		((pset.comp_case == PSQL_COMP_CASE_PRESERVE_LOWER ||
+		  pset.comp_case == PSQL_COMP_CASE_PRESERVE_UPPER) && islower(first)) ||
+		(pset.comp_case == PSQL_COMP_CASE_PRESERVE_LOWER && !isalpha(first)))
+	{
 		for (p = ret; *p; p++)
 			*p = pg_tolower((unsigned char) *p);
+	}
 	else
+	{
 		for (p = ret; *p; p++)
 			*p = pg_toupper((unsigned char) *p);
+	}
 
 	return ret;
 }
diff --git a/src/bin/psql/variables.c b/src/bin/psql/variables.c
index 1f871d7fa01..29c50f969c0 100644
--- a/src/bin/psql/variables.c
+++ b/src/bin/psql/variables.c
@@ -79,11 +79,16 @@ GetVariable(VariableSpace space, const char *name)
 }
 
 /*
- * Try to interpret value as boolean value.  Valid values are: true,
- * false, yes, no, on, off, 1, 0; as well as unique prefixes thereof.
+ * Try to interpret "value" as boolean value.
+ *
+ * Valid values are: true, false, yes, no, on, off, 1, 0; as well as unique
+ * prefixes thereof.
+ *
+ * "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.
  */
 bool
-ParseVariableBool(const char *value)
+ParseVariableBool(const char *value, const char *name)
 {
 	size_t		len;
 
@@ -112,7 +117,9 @@ ParseVariableBool(const char *value)
 	else
 	{
 		/* NULL is treated as false, so a non-matching value is 'true' */
-		psql_error("unrecognized Boolean value; assuming \"on\"\n");
+		if (name)
+			psql_error("unrecognized value \"%s\" for \"%s\"; assuming \"%s\"\n",
+					   value, name, "on");
 		return true;
 	}
 }
diff --git a/src/bin/psql/variables.h b/src/bin/psql/variables.h
index 5a2b98e122a..f94f7796247 100644
--- a/src/bin/psql/variables.h
+++ b/src/bin/psql/variables.h
@@ -35,7 +35,7 @@ typedef struct _variable *VariableSpace;
 VariableSpace CreateVariableSpace(void);
 const char *GetVariable(VariableSpace space, const char *name);
 
-bool		ParseVariableBool(const char *val);
+bool		ParseVariableBool(const char *value, const char *name);
 int ParseVariableNum(const char *val,
 				 int defaultval,
 				 int faultval,