Re: [GENERAL] ON_ERROR_ROLLBACK - Mailing list pgsql-hackers

Bernd Helmle <mailings@oopsware.de> writes:
> --On 29. Dezember 2014 12:55:11 -0500 Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Given the lack of previous complaints, this probably isn't backpatching
>> material, but it sure seems like a bit of attention to consistency
>> would be warranted here.

> Now that i read it i remember a client complaining about this some time
> ago. I forgot about it, but i think there's value in it to backpatch.

Hm.  Last night I wrote the attached draft patch, which I was intending
to apply to HEAD only.  The argument against back-patching is basically
that this might change the interpretation of scripts that had been
accepted silently before.  For example
    \set ECHO_HIDDEN NoExec
will now select "noexec" mode whereas before you silently got "on" mode.
In one light this is certainly a bug fix, but in another it's just
definitional instability.

If we'd gotten a field bug report we might well have chosen to back-patch,
though, and perhaps your client's complaint counts as that.

Opinions anyone?

            regards, tom lane

diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index d29dfa2..bdfb67c 100644
*** a/doc/src/sgml/ref/psql-ref.sgml
--- b/doc/src/sgml/ref/psql-ref.sgml
*************** EOF
*** 173,180 ****
        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>.
        </para>
        </listitem>
      </varlistentry>
--- 173,179 ----
        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> to <literal>on</>.
        </para>
        </listitem>
      </varlistentry>
*************** EOF
*** 333,340 ****
        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.
        </para>
        </listitem>
      </varlistentry>
--- 332,339 ----
        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.
!       This is equivalent to setting the variable <varname>QUIET</varname>
!       to <literal>on</>.
        </para>
        </listitem>
      </varlistentry>
*************** bar
*** 2884,2891 ****
          <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
          <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
--- 2883,2891 ----
          <term><varname>ECHO_HIDDEN</varname></term>
          <listitem>
          <para>
!         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
*************** bar
*** 3046,3061 ****
        </term>
          <listitem>
          <para>
!         When <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
          ignored in interactive sessions, and not when reading script
!         files. When <literal>off</> (the default), a statement in a
          transaction block that generates an error aborts the entire
!         transaction. The on_error_rollback-on 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.
          </para>
          </listitem>
        </varlistentry>
--- 3046,3061 ----
        </term>
          <listitem>
          <para>
!         When set to <literal>on</>, if a statement in a transaction block
          generates an error, the error is ignored and the transaction
!         continues. When set to <literal>interactive</>, such errors are only
          ignored in interactive sessions, and not when reading script
!         files. When unset or set to <literal>off</>, a statement in a
          transaction block that generates an error aborts the entire
!         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 then rolling back to the
!         savepoint if the command fails.
          </para>
          </listitem>
        </varlistentry>
*************** bar
*** 3065,3071 ****
          <listitem>
          <para>
          By default, command processing continues after an error.  When this
!         variable is set, it 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
--- 3065,3072 ----
          <listitem>
          <para>
          By default, command processing continues after an error.  When this
!         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
*************** bar
*** 3107,3114 ****
          <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
          interactive mode.
          </para>
          </listitem>
--- 3108,3115 ----
          <term><varname>QUIET</varname></term>
          <listitem>
          <para>
!         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>
*************** bar
*** 3118,3125 ****
          <term><varname>SINGLELINE</varname></term>
          <listitem>
          <para>
!         This variable is equivalent to the command line option
!         <option>-S</option>.
          </para>
          </listitem>
        </varlistentry>
--- 3119,3126 ----
          <term><varname>SINGLELINE</varname></term>
          <listitem>
          <para>
!         Setting this variable to <literal>on</> is equivalent to the command
!         line option <option>-S</option>.
          </para>
          </listitem>
        </varlistentry>
*************** bar
*** 3128,3135 ****
          <term><varname>SINGLESTEP</varname></term>
          <listitem>
          <para>
!         This variable is equivalent to the command line option
!         <option>-s</option>.
          </para>
          </listitem>
        </varlistentry>
--- 3129,3136 ----
          <term><varname>SINGLESTEP</varname></term>
          <listitem>
          <para>
!         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 c7a17d7..504cc0d 100644
*** a/src/bin/psql/command.c
--- b/src/bin/psql/command.c
*************** exec_command(const char *cmd,
*** 1362,1368 ****
                                                   OT_NORMAL, NULL, false);

          if (opt)
!             pset.timing = ParseVariableBool(opt);
          else
              pset.timing = !pset.timing;
          if (!pset.quiet)
--- 1362,1368 ----
                                                   OT_NORMAL, NULL, false);

          if (opt)
!             pset.timing = ParseVariableBool(opt, "\\timing");
          else
              pset.timing = !pset.timing;
          if (!pset.quiet)
*************** do_pset(const char *param, const char *v
*** 2412,2423 ****
      }

      /* set expanded/vertical mode */
!     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);
          else
              popt->topt.expanded = !popt->topt.expanded;
      }
--- 2412,2425 ----
      }

      /* set expanded/vertical mode */
!     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, param);
          else
              popt->topt.expanded = !popt->topt.expanded;
      }
*************** do_pset(const char *param, const char *v
*** 2426,2432 ****
      else if (strcmp(param, "numericlocale") == 0)
      {
          if (value)
!             popt->topt.numericLocale = ParseVariableBool(value);
          else
              popt->topt.numericLocale = !popt->topt.numericLocale;
      }
--- 2428,2434 ----
      else if (strcmp(param, "numericlocale") == 0)
      {
          if (value)
!             popt->topt.numericLocale = ParseVariableBool(value, param);
          else
              popt->topt.numericLocale = !popt->topt.numericLocale;
      }
*************** do_pset(const char *param, const char *v
*** 2481,2487 ****
      else if (strcmp(param, "t") == 0 || strcmp(param, "tuples_only") == 0)
      {
          if (value)
!             popt->topt.tuples_only = ParseVariableBool(value);
          else
              popt->topt.tuples_only = !popt->topt.tuples_only;
      }
--- 2483,2489 ----
      else if (strcmp(param, "t") == 0 || strcmp(param, "tuples_only") == 0)
      {
          if (value)
!             popt->topt.tuples_only = ParseVariableBool(value, param);
          else
              popt->topt.tuples_only = !popt->topt.tuples_only;
      }
*************** do_pset(const char *param, const char *v
*** 2512,2521 ****
          if (value && pg_strcasecmp(value, "always") == 0)
              popt->topt.pager = 2;
          else if (value)
!             if (ParseVariableBool(value))
                  popt->topt.pager = 1;
              else
                  popt->topt.pager = 0;
          else if (popt->topt.pager == 1)
              popt->topt.pager = 0;
          else
--- 2514,2525 ----
          if (value && pg_strcasecmp(value, "always") == 0)
              popt->topt.pager = 2;
          else if (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
*************** do_pset(const char *param, const char *v
*** 2526,2532 ****
      else if (strcmp(param, "footer") == 0)
      {
          if (value)
!             popt->topt.default_footer = ParseVariableBool(value);
          else
              popt->topt.default_footer = !popt->topt.default_footer;
      }
--- 2530,2536 ----
      else if (strcmp(param, "footer") == 0)
      {
          if (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 ef24a4e..bad70e4 100644
*** a/src/bin/psql/settings.h
--- b/src/bin/psql/settings.h
***************
*** 27,32 ****
--- 27,37 ----
  #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,
*************** typedef enum
*** 51,56 ****
--- 56,69 ----

  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,
*************** typedef struct _psqlSettings
*** 110,115 ****
--- 123,129 ----
      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 11a159a..a23ebce 100644
*** a/src/bin/psql/startup.c
--- b/src/bin/psql/startup.c
*************** showVersion(void)
*** 705,735 ****
  static void
  autocommit_hook(const char *newval)
  {
!     pset.autocommit = ParseVariableBool(newval);
  }

  static void
  on_error_stop_hook(const char *newval)
  {
!     pset.on_error_stop = ParseVariableBool(newval);
  }

  static void
  quiet_hook(const char *newval)
  {
!     pset.quiet = ParseVariableBool(newval);
  }

  static void
  singleline_hook(const char *newval)
  {
!     pset.singleline = ParseVariableBool(newval);
  }

  static void
  singlestep_hook(const char *newval)
  {
!     pset.singlestep = ParseVariableBool(newval);
  }

  static void
--- 705,735 ----
  static void
  autocommit_hook(const char *newval)
  {
!     pset.autocommit = ParseVariableBool(newval, "AUTOCOMMIT");
  }

  static void
  on_error_stop_hook(const char *newval)
  {
!     pset.on_error_stop = ParseVariableBool(newval, "ON_ERROR_STOP");
  }

  static void
  quiet_hook(const char *newval)
  {
!     pset.quiet = ParseVariableBool(newval, "QUIET");
  }

  static void
  singleline_hook(const char *newval)
  {
!     pset.singleline = ParseVariableBool(newval, "SINGLELINE");
  }

  static void
  singlestep_hook(const char *newval)
  {
!     pset.singlestep = ParseVariableBool(newval, "SINGLESTEP");
  }

  static void
*************** echo_hook(const char *newval)
*** 743,756 ****
  {
      if (newval == NULL)
          pset.echo = PSQL_ECHO_NONE;
!     else if (strcmp(newval, "queries") == 0)
          pset.echo = PSQL_ECHO_QUERIES;
!     else if (strcmp(newval, "errors") == 0)
          pset.echo = PSQL_ECHO_ERRORS;
!     else if (strcmp(newval, "all") == 0)
          pset.echo = PSQL_ECHO_ALL;
      else
          pset.echo = PSQL_ECHO_NONE;
  }

  static void
--- 743,762 ----
  {
      if (newval == NULL)
          pset.echo = PSQL_ECHO_NONE;
!     else if (pg_strcasecmp(newval, "queries") == 0)
          pset.echo = PSQL_ECHO_QUERIES;
!     else if (pg_strcasecmp(newval, "errors") == 0)
          pset.echo = PSQL_ECHO_ERRORS;
!     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
*************** echo_hidden_hook(const char *newval)
*** 758,769 ****
  {
      if (newval == NULL)
          pset.echo_hidden = PSQL_ECHO_HIDDEN_OFF;
!     else if (strcmp(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
          pset.echo_hidden = PSQL_ECHO_HIDDEN_ON;
  }

  static void
--- 764,775 ----
  {
      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;
  }

  static void
*************** on_error_rollback_hook(const char *newva
*** 773,782 ****
          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)
          pset.on_error_rollback = PSQL_ERROR_ROLLBACK_OFF;
      else
!         pset.on_error_rollback = PSQL_ERROR_ROLLBACK_ON;
  }

  static void
--- 779,809 ----
          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;
+ }
+
+ 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
!     {
!         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
*************** histcontrol_hook(const char *newval)
*** 784,797 ****
  {
      if (newval == NULL)
          pset.histcontrol = hctl_none;
!     else if (strcmp(newval, "ignorespace") == 0)
          pset.histcontrol = hctl_ignorespace;
!     else if (strcmp(newval, "ignoredups") == 0)
          pset.histcontrol = hctl_ignoredups;
!     else if (strcmp(newval, "ignoreboth") == 0)
          pset.histcontrol = hctl_ignoreboth;
      else
          pset.histcontrol = hctl_none;
  }

  static void
--- 811,830 ----
  {
      if (newval == NULL)
          pset.histcontrol = hctl_none;
!     else if (pg_strcasecmp(newval, "ignorespace") == 0)
          pset.histcontrol = hctl_ignorespace;
!     else if (pg_strcasecmp(newval, "ignoredups") == 0)
          pset.histcontrol = hctl_ignoredups;
!     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
*************** verbosity_hook(const char *newval)
*** 817,830 ****
  {
      if (newval == NULL)
          pset.verbosity = PQERRORS_DEFAULT;
!     else if (strcmp(newval, "default") == 0)
          pset.verbosity = PQERRORS_DEFAULT;
!     else if (strcmp(newval, "terse") == 0)
          pset.verbosity = PQERRORS_TERSE;
!     else if (strcmp(newval, "verbose") == 0)
          pset.verbosity = PQERRORS_VERBOSE;
      else
          pset.verbosity = PQERRORS_DEFAULT;

      if (pset.db)
          PQsetErrorVerbosity(pset.db, pset.verbosity);
--- 850,867 ----
  {
      if (newval == NULL)
          pset.verbosity = PQERRORS_DEFAULT;
!     else if (pg_strcasecmp(newval, "default") == 0)
          pset.verbosity = PQERRORS_DEFAULT;
!     else if (pg_strcasecmp(newval, "terse") == 0)
          pset.verbosity = PQERRORS_TERSE;
!     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);
*************** EstablishVariableSpace(void)
*** 845,850 ****
--- 882,888 ----
      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 82c926d..ba78f7e 100644
*** a/src/bin/psql/tab-complete.c
--- b/src/bin/psql/tab-complete.c
*************** complete_from_files(const char *text, in
*** 4452,4458 ****

  /*
   * 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.
   */
  static char *
  pg_strdup_keyword_case(const char *s, const char *ref)
--- 4452,4458 ----

  /*
   * Make a pg_strdup copy of s and convert the case according to
!  * 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)
*************** pg_strdup_keyword_case(const char *s, co
*** 4460,4497 ****
      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))
!         )
          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;
  }
--- 4460,4481 ----
      char       *ret,
                 *p;
      unsigned char first = ref[0];

      ret = pg_strdup(s);

!     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 1f871d7..29c50f9 100644
*** a/src/bin/psql/variables.c
--- b/src/bin/psql/variables.c
*************** GetVariable(VariableSpace space, const c
*** 79,89 ****
  }

  /*
!  * Try to interpret value as boolean value.  Valid values are: true,
!  * false, yes, no, on, off, 1, 0; as well as unique prefixes thereof.
   */
  bool
! ParseVariableBool(const char *value)
  {
      size_t        len;

--- 79,94 ----
  }

  /*
!  * 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, const char *name)
  {
      size_t        len;

*************** ParseVariableBool(const char *value)
*** 112,118 ****
      else
      {
          /* NULL is treated as false, so a non-matching value is 'true' */
!         psql_error("unrecognized Boolean value; assuming \"on\"\n");
          return true;
      }
  }
--- 117,125 ----
      else
      {
          /* NULL is treated as false, so a non-matching value is 'true' */
!         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 5a2b98e..f94f779 100644
*** a/src/bin/psql/variables.h
--- b/src/bin/psql/variables.h
*************** typedef struct _variable *VariableSpace;
*** 35,41 ****
  VariableSpace CreateVariableSpace(void);
  const char *GetVariable(VariableSpace space, const char *name);

! bool        ParseVariableBool(const char *val);
  int ParseVariableNum(const char *val,
                   int defaultval,
                   int faultval,
--- 35,41 ----
  VariableSpace CreateVariableSpace(void);
  const char *GetVariable(VariableSpace space, const char *name);

! bool        ParseVariableBool(const char *value, const char *name);
  int ParseVariableNum(const char *val,
                   int defaultval,
                   int faultval,