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.

diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index d8bd2e964af063c50a1679b86ee27fa2d7f59ff8..f65b9d4767b27fe87e7bc4e7557797a37bc608ca 100644 (file)
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -161,8 +161,7 @@ EOF
       Echo the actual queries generated by <command>\d</command> and other backslash
       commands. You can use this to study <application>psql</application>'s
       internal operations. This is equivalent to
-      setting the variable <varname>ECHO_HIDDEN</varname> from within
-      <application>psql</application>.
+      setting the variable <varname>ECHO_HIDDEN</varname> to <literal>on</>.
       </para>
       </listitem>
     </varlistentry>
@@ -321,8 +320,8 @@ EOF
       quietly. By default, it prints welcome messages and various
       informational output. If this option is used, none of this
       happens. This is useful with the <option>-c</option> option.
-      Within <application>psql</application> you can also set the
-      <varname>QUIET</varname> variable to achieve the same effect.
+      This is equivalent to setting the variable <varname>QUIET</varname>
+      to <literal>on</>.
       </para>
       </listitem>
     </varlistentry>
@@ -2824,8 +2823,9 @@ bar
         <term><varname>ECHO_HIDDEN</varname></term>
         <listitem>
         <para>
-        When this variable is set and a backslash command queries the
-        database, the query is first shown. This way you can study the
+        When this variable is set to <literal>on</> and a backslash command
+        queries the database, the query is first shown.
+        This feature helps you to study
         <productname>PostgreSQL</productname> internals and provide
         similar functionality in your own programs. (To select this behavior
         on program start-up, use the switch <option>-E</option>.)  If you set
@@ -2985,16 +2985,16 @@ bar
       </term>
         <listitem>
         <para>
-        When <literal>on</>, if a statement in a transaction block
+        When set to <literal>on</>, if a statement in a transaction block
         generates an error, the error is ignored and the transaction
-        continues. When <literal>interactive</>, such errors are only
+        continues. When set to <literal>interactive</>, such errors are only
         ignored in interactive sessions, and not when reading script
-        files. When <literal>off</> (the default), a statement in a
+        files. When unset or set to <literal>off</>, a statement in a
         transaction block that generates an error aborts the entire
-        transaction. The on_error_rollback-on mode works by issuing an
+        transaction. The error rollback mode works by issuing an
         implicit <command>SAVEPOINT</> for you, just before each command
-        that is in a transaction block, and rolls back to the savepoint
-        on error.
+        that is in a transaction block, and then rolling back to the
+        savepoint if the command fails.
         </para>
         </listitem>
       </varlistentry>
@@ -3004,7 +3004,8 @@ bar
         <listitem>
         <para>
         By default, command processing continues after an error.  When this
-        variable is set, it will instead stop immediately.  In interactive mode,
+        variable is set to <literal>on</>, processing will instead stop
+        immediately.  In interactive mode,
         <application>psql</application> will return to the command prompt;
         otherwise, <application>psql</application> will exit, returning
         error code 3 to distinguish this case from fatal error
@@ -3046,8 +3047,8 @@ bar
         <term><varname>QUIET</varname></term>
         <listitem>
         <para>
-        This variable is equivalent to the command line option
-        <option>-q</option>. It is probably not too useful in
+        Setting this variable to <literal>on</> is equivalent to the command
+        line option <option>-q</option>. It is probably not too useful in
         interactive mode.
         </para>
         </listitem>
@@ -3057,8 +3058,8 @@ bar
         <term><varname>SINGLELINE</varname></term>
         <listitem>
         <para>
-        This variable is equivalent to the command line option
-        <option>-S</option>.
+        Setting this variable to <literal>on</> is equivalent to the command
+        line option <option>-S</option>.
         </para>
         </listitem>
       </varlistentry>
@@ -3067,8 +3068,8 @@ bar
         <term><varname>SINGLESTEP</varname></term>
         <listitem>
         <para>
-        This variable is equivalent to the command line option
-        <option>-s</option>.
+        Setting this variable to <literal>on</> is equivalent to the command
+        line option <option>-s</option>.
         </para>
         </listitem>
       </varlistentry>

diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 39bd3617b942f0d9acc011ecdefd11d80354839e..8f8c7859b72c06817a87e845269527a252aee7bf 100644 (file)
--- 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 0a60e6817b2fa9cfbbf05ab9971269e029fc7db8..5e3e92a8e98d7ee8037750e6e7d07def712e7e50 100644 (file)
--- 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,
@@ -48,6 +53,14 @@ typedef enum
        PSQL_ERROR_ROLLBACK_ON
 } PSQL_ERROR_ROLLBACK;
 
+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,
@@ -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 45653a15a806a72676e97b24b0655237fe15b5a3..2ab5690e547c9d7d68d3d2e3983412b7ef09457e 100644 (file)
--- 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 6126f7407aee87d1601c1972a0951ae7e34ce3d4..9841c1ab745b845292afef8b4580945283b68d11 100644 (file)
--- 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 1f871d7fa0127b208f25a7b69dd196a31d39e65f..29c50f969c06edff57d70ac650049768f5c76637 100644 (file)
--- 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 5a2b98e122a34bf6ab2c5a5f0a6f7a21e94f2415..f94f7796247313396196c53535cef67f15256608 100644 (file)
--- 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,