Re: [HACKERS] \if, \elseif, \else, \endif (was Re: PSQL commands: \quit_if, \quit_unless) - Mailing list pgsql-hackers

Corey Huinker <corey.huinker@gmail.com> writes:
> [ 0001.if_endif.v21.diff ]

I had thought that this patch was pretty close to committable, but
the more I poke at it the less I think so.  The technology it uses
for skipping unexecuted script sections has got too many bugs.

* Daniel Verite previously pointed out the desirability of disabling
variable expansion while skipping script.  That doesn't seem to be here,
though there's an apparently-vestigial comment in psql/common.c claiming
that it is.  IIRC, I objected to putting knowledge of ConditionalStack
into the shared psqlscan.l lexer, and I still think that would be a bad
idea; but we need some way to get the lexer to shut that off.  Probably
the best way is to add a passthrough "void *" argument that would let the
get_variable callback function mechanize the rule about not expanding
in a false branch.

* Whether or not you think it's important not to expand skipped variables,
I think that it's critical that skipped backtick expressions not be
executed.  The specific use-case that I'm concerned about is backtick
evals in \if expressions, which are going to be all over the place as
long as we haven't got any native expression eval capability, and will
doubtless remain important even when/if we do.  So in a case like

    \if something
    \elif `expr :var1 + :var2 = :var3`
    \endif

I think it's essential that expr not be called if the first if-condition
succeeded.  (That first condition might be checking whether the vars
contain valid integers, for example.)  The current patch gets this totally
wrong --- not only does it perform the backtick, but \elif complains if
the result isn't a valid bool.  I do not think that a skipped \if or \elif
should evaluate its argument at all.

* The documentation says that an \if or \elif expression extends to the
end of the line, but actually the code is just eating one OT_NORMAL
argument.  That means it's OK to do this:

regression=# \if 1 \echo foo \echo bar \endif
foo
bar
regression=# 

which doesn't seem insane, except that the inverse case is insane:

regression=# \if 0 \echo foo \echo bar \endif
regression@# 

(notice we're not out of the conditional).  Even if we change it to
eat the whole line as argument, this inconsistency will remain:

regression=# \if 1
regression=# \echo foo \endif
foo
regression=# 

(notice we're out of the conditional)

regression=# \if 0
regression@# \echo foo \endif
command ignored, use \endif or Ctrl-C to exit current branch.
regression@# 

(notice we're not out of the conditional)

This inconsistency seems to have to do with the code in HandleSlashCmds
that discards arguments until EOL after a failed backslash command.
You've modified that to also discard arguments after a non-executed
command, and I think that's broken.

* More generally, I do not think that the approach of having exec_command
simply fall out immediately when in a false branch is going to work,
because it ignores the fact that different backslash commands have
different argument parsing rules.  Some will eat the rest of the line and
some won't.  I'm afraid that it might be necessary to remove that code
block and add a test to every single backslash command that decides
whether to actually perform its action after it's consumed its arguments.
That would be tedious :-(.  But as it stands, backslash commands will get
parsed differently (ie with potentially-different ending points) depending
on whether they're in a live branch or not, and that seems just way too
error-prone to be allowed to stand.

* I think it's completely wrong to do either resetPQExpBuffer(query_buf)
or psql_scan_reset(scan_state) when deciding a branch is not to be
executed.  Compare these results:

regression=# select (1 +
regression(# \if 1
regression-# \echo foo
foo
regression-# \endif
regression-# 2);
 ?column? 
----------
        3
(1 row)

regression=# select (1 +
regression(# \if 0
regression-# \echo foo
command ignored, use \endif or Ctrl-C to exit current branch.
regression@# \endif
regression=# 2);
ERROR:  syntax error at or near "2"
LINE 1: 2);
        ^
regression=# 

If the first \if doesn't throw away an incomplete query buffer (which it
should not), then surely the second should not either.  Somebody who
actually wants to toss the query buffer can put \r into the appropriate
branch of their \if; it's not for us to force that on them.

* Also, the documentation for psql_scan_reset is pretty clear that it's to
be called when and only when the query buffer is reset, which makes your
calls in the bodies of the conditional commands wrong.  As an example:

regression=# select (1 +
regression(# 2;
regression(# 

(notice we've not sent the known-incomplete command to the server) vs

regression(# \r
Query buffer reset (cleared).
regression=# select (1 +
regression(# \if 1
regression-# \endif
regression-# 2;
ERROR:  syntax error at or near ";"
LINE 2: 2;
         ^
regression=# 

That happens because the \if code gratuituously resets the lexer,
as we can see from the unexpected change in the prompt.

* I'm not on board with having a bad expression result in failing
the \if or \elif altogether.  It was stated several times upthread
that that should be processed as though the result were "false",
and I agree with that.  As it stands, it's completely impossible to
write script code that can cope with possibly-failing expressions,
or even to reason very clearly about what will happen: you can't
know whether a following \else will be honored, for example.
We might as well replace the recommendation to use ON_ERROR_STOP with
a forced abort() for an invalid expression value, because trying to
continue a script with this behavior is entirely useless.


I did some work on the patch before reaching these conclusions,
mostly improving the documentation, getting rid of some unnecessary
#include's, etc.  I've attached that work as far as it went.

            regards, tom lane

diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 2a9c412..7743fb0 100644
*** a/doc/src/sgml/ref/psql-ref.sgml
--- b/doc/src/sgml/ref/psql-ref.sgml
*************** hello 10
*** 2064,2069 ****
--- 2064,2165 ----


        <varlistentry>
+         <term><literal>\if</literal> <replaceable class="parameter">expression</replaceable></term>
+         <term><literal>\elif</literal> <replaceable class="parameter">expression</replaceable></term>
+         <term><literal>\else</literal></term>
+         <term><literal>\endif</literal></term>
+         <listitem>
+         <para>
+         This group of commands implements nestable conditional blocks.
+         A conditional block must begin with an <command>\if</command> and end
+         with an <command>\endif</command>.  In between there may be any number
+         of <command>\elif</command> clauses, which may optionally be followed
+         by a single <command>\else</command> clause.  Ordinary queries and
+         other types of backslash commands may (and usually do) appear between
+         the commands forming a conditional block.
+         </para>
+         <para>
+         The <command>\if</command> and <command>\elif</command> commands read
+         the rest of the line and evaluate it as a boolean expression.  If the
+         expression is <literal>true</literal> then processing continues
+         normally; otherwise, lines are skipped until a
+         matching <command>\elif</command>, <command>\else</command>,
+         or <command>\endif</command> is reached.  Once
+         an <command>\if</command> or <command>\elif</command> has succeeded,
+         later matching <command>\elif</command> commands are not evaluated but
+         are treated as false.  Lines following an <command>\else</command> are
+         processed only if no earlier matching <command>\if</command>
+         or <command>\elif</command> succeeded.
+         </para>
+         <para>
+         Lines being skipped are parsed normally to identify queries and
+         backslash commands, but queries are not sent to the server, and
+         backslash commands other than conditionals
+         (<command>\if</command>, <command>\elif</command>,
+         <command>\else</command>, <command>\endif</command>) are
+         ignored.  Conditional commands are checked only for valid nesting.
+         </para>
+         <para>
+         The <replaceable class="parameter">expression</replaceable> argument
+         of <command>\if</command> or <command>\elif</command>
+         is subject to variable interpolation and backquote expansion, just
+         like any other backslash command argument.  After that it is evaluated
+         like the value of an on/off option variable.  So a valid value
+         is any unambiguous case-insensitive match for one of:
+         <literal>true</literal>, <literal>false</literal>, <literal>1</literal>,
+         <literal>0</literal>, <literal>on</literal>, <literal>off</literal>,
+         <literal>yes</literal>, <literal>no</literal>.  For example,
+         <literal>t</literal>, <literal>T</literal>, and <literal>tR</literal>
+         will all be considered to be <literal>true</literal>.
+         </para>
+         <para>
+         Expressions that do not properly evaluate to true or false will
+         generate an error and cause the <command>\if</command> or
+         <command>\elif</command> command to fail.  Because that behavior may
+         change branching context in undesirable ways (executing code which
+         was intended to be skipped, causing <command>\elif</command>,
+         <command>\else</command>, and <command>\endif</command> commands to
+         pair with the wrong <command>\if</command>, etc), it is
+         recommended that scripts that use conditionals also set
+         <varname>ON_ERROR_STOP</varname>.
+         </para>
+         <para>
+         All the backslash commands of a given conditional block must appear in
+         the same source file. If EOF is reached on the main input file or an
+         <command>\include</command>-ed file before all local
+         <command>\if</command>-blocks have been closed,
+         then <application>psql</> will raise an error.
+         </para>
+         <para>
+          Here is an example:
+         </para>
+ <programlisting>
+ -- set ON_ERROR_STOP in case the variables are not valid boolean expressions
+ \set ON_ERROR_STOP on
+ -- check for the existence of two separate records in the database and store
+ -- the results in separate psql variables
+ SELECT
+     EXISTS(SELECT 1 FROM customer WHERE customer_id = 123) as is_customer,
+     EXISTS(SELECT 1 FROM employee WHERE employee_id = 456) as is_employee
+ \gset
+ \if :is_customer
+     SELECT * FROM customer WHERE customer_id = 123;
+ \elif :is_employee
+     \echo 'is not a customer but is an employee'
+     SELECT * FROM employee WHERE employee_id = 456;
+ \else
+     \if yes
+         \echo 'not a customer or employee'
+     \else
+         \echo 'this will never print'
+     \endif
+ \endif
+ </programlisting>
+         </listitem>
+       </varlistentry>
+
+
+       <varlistentry>
          <term><literal>\l[+]</literal> or <literal>\list[+] [ <link linkend="APP-PSQL-patterns"><replaceable
class="parameter">pattern</replaceable></link>]</literal></term> 
          <listitem>
          <para>
*************** testdb=> <userinput>INSERT INTO my_ta
*** 3715,3721 ****
          <listitem>
          <para>
          In prompt 1 normally <literal>=</literal>,
!         but <literal>^</literal> if in single-line mode,
          or <literal>!</literal> if the session is disconnected from the
          database (which can happen if <command>\connect</command> fails).
          In prompt 2 <literal>%R</literal> is replaced by a character that
--- 3811,3818 ----
          <listitem>
          <para>
          In prompt 1 normally <literal>=</literal>,
!         but <literal>@</literal> if the session is in a false conditional
!         block, or <literal>^</literal> if in single-line mode,
          or <literal>!</literal> if the session is disconnected from the
          database (which can happen if <command>\connect</command> fails).
          In prompt 2 <literal>%R</literal> is replaced by a character that
diff --git a/src/bin/psql/.gitignore b/src/bin/psql/.gitignore
index c2862b1..5239013 100644
*** a/src/bin/psql/.gitignore
--- b/src/bin/psql/.gitignore
***************
*** 3,5 ****
--- 3,7 ----
  /sql_help.c

  /psql
+
+ /tmp_check/
diff --git a/src/bin/psql/Makefile b/src/bin/psql/Makefile
index f8e31ea..90ee85d 100644
*** a/src/bin/psql/Makefile
--- b/src/bin/psql/Makefile
*************** REFDOCDIR= $(top_srcdir)/doc/src/sgml/re
*** 21,30 ****
  override CPPFLAGS := -I. -I$(srcdir) -I$(libpq_srcdir) $(CPPFLAGS)
  LDFLAGS += -L$(top_builddir)/src/fe_utils -lpgfeutils -lpq

! OBJS=    command.o common.o help.o input.o stringutils.o mainloop.o copy.o \
      startup.o prompt.o variables.o large_obj.o describe.o \
      crosstabview.o tab-complete.o \
!     sql_help.o psqlscanslash.o \
      $(WIN32RES)


--- 21,30 ----
  override CPPFLAGS := -I. -I$(srcdir) -I$(libpq_srcdir) $(CPPFLAGS)
  LDFLAGS += -L$(top_builddir)/src/fe_utils -lpgfeutils -lpq

! OBJS=    command.o common.o conditional.o copy.o help.o input.o mainloop.o \
      startup.o prompt.o variables.o large_obj.o describe.o \
      crosstabview.o tab-complete.o \
!     sql_help.o stringutils.o psqlscanslash.o \
      $(WIN32RES)


*************** uninstall:
*** 57,64 ****
--- 57,71 ----

  clean distclean:
      rm -f psql$(X) $(OBJS) lex.backup
+     rm -rf tmp_check

  # files removed here are supposed to be in the distribution tarball,
  # so do not clean them in the clean/distclean rules
  maintainer-clean: distclean
      rm -f sql_help.h sql_help.c psqlscanslash.c
+
+ check:
+     $(prove_check)
+
+ installcheck:
+     $(prove_installcheck)
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 4f4a0aa..359da08 100644
*** a/src/bin/psql/command.c
--- b/src/bin/psql/command.c
*************** typedef enum EditableObjectType
*** 59,64 ****
--- 59,65 ----
  /* functions for use in this file */
  static backslashResult exec_command(const char *cmd,
               PsqlScanState scan_state,
+              ConditionalStack cstack,
               PQExpBuffer query_buf);
  static bool do_edit(const char *filename_arg, PQExpBuffer query_buf,
          int lineno, bool *edited);
*************** static void checkWin32Codepage(void);
*** 106,111 ****
--- 107,113 ----

  backslashResult
  HandleSlashCmds(PsqlScanState scan_state,
+                 ConditionalStack cstack,
                  PQExpBuffer query_buf)
  {
      backslashResult status = PSQL_CMD_SKIP_LINE;
*************** HandleSlashCmds(PsqlScanState scan_state
*** 118,124 ****
      cmd = psql_scan_slash_command(scan_state);

      /* And try to execute it */
!     status = exec_command(cmd, scan_state, query_buf);

      if (status == PSQL_CMD_UNKNOWN)
      {
--- 120,126 ----
      cmd = psql_scan_slash_command(scan_state);

      /* And try to execute it */
!     status = exec_command(cmd, scan_state, cstack, query_buf);

      if (status == PSQL_CMD_UNKNOWN)
      {
*************** HandleSlashCmds(PsqlScanState scan_state
*** 129,135 ****
          status = PSQL_CMD_ERROR;
      }

!     if (status != PSQL_CMD_ERROR)
      {
          /* eat any remaining arguments after a valid command */
          /* note we suppress evaluation of backticks here */
--- 131,137 ----
          status = PSQL_CMD_ERROR;
      }

!     if (status != PSQL_CMD_ERROR && conditional_active(cstack))
      {
          /* eat any remaining arguments after a valid command */
          /* note we suppress evaluation of backticks here */
*************** read_connect_arg(PsqlScanState scan_stat
*** 191,196 ****
--- 193,222 ----
      return result;
  }

+ /*
+  * Read and interpret argument as a boolean expression.
+  * Return true if a boolean value was successfully parsed.
+  */
+ static bool
+ read_boolean_expression(PsqlScanState scan_state, char *action,
+                         bool *result)
+ {
+     char    *value = psql_scan_slash_option(scan_state,
+                                             OT_NORMAL, NULL, false);
+     bool    success = ParseVariableBool(value, action, result);
+     free(value);
+     return success;
+ }
+
+ /*
+  * Return true if the command given is a branching command.
+  */
+ static bool
+ is_branching_command(const char *cmd)
+ {
+     return (strcmp(cmd, "if") == 0 || strcmp(cmd, "elif") == 0
+             || strcmp(cmd, "else") == 0 || strcmp(cmd, "endif") == 0);
+ }

  /*
   * Subroutine to actually try to execute a backslash command.
*************** read_connect_arg(PsqlScanState scan_stat
*** 198,209 ****
--- 224,248 ----
  static backslashResult
  exec_command(const char *cmd,
               PsqlScanState scan_state,
+              ConditionalStack cstack,
               PQExpBuffer query_buf)
  {
      bool        success = true; /* indicate here if the command ran ok or
                                   * failed */
      backslashResult status = PSQL_CMD_SKIP_LINE;

+     if (!conditional_active(cstack) && !is_branching_command(cmd))
+     {
+         if (pset.cur_cmd_interactive)
+             psql_error("command ignored, use \\endif or Ctrl-C to exit "
+                         "current branch.\n");
+
+         /* Continue with an empty buffer as if the command were never read */
+         resetPQExpBuffer(query_buf);
+         psql_scan_reset(scan_state);
+         return status;
+     }
+
      /*
       * \a -- toggle field alignment This makes little sense but we keep it
       * around.
*************** exec_command(const char *cmd,
*** 1008,1013 ****
--- 1047,1202 ----
          }
      }

+     /*
+      * \if <expr> is the beginning of an \if..\endif block. <expr> must be a
+      * valid boolean expression, or the command will be ignored. If this \if
+      * is itself a part of a branch that is false/ignored, the expression
+      * will be checked for validity but cannot override the outer block.
+      */
+     else if (strcmp(cmd, "if") == 0)
+     {
+         bool if_true = false;
+         ifState new_if_state;
+         success = read_boolean_expression(scan_state, "\\if <expr>",
+                                             &if_true);
+
+         switch (conditional_stack_peek(cstack))
+         {
+             case IFSTATE_IGNORED:
+             case IFSTATE_FALSE:
+             case IFSTATE_ELSE_FALSE:
+                 /* new if-block, expression result is ignored */
+                 new_if_state = IFSTATE_IGNORED;
+                 break;
+             default:
+                 /* new if-block, expression result matters */
+                 new_if_state = (if_true) ? IFSTATE_TRUE : IFSTATE_FALSE;
+                 break;
+         }
+
+         /* only start if a new if-block if the expression was valid */
+         if (success)
+             conditional_stack_push(cstack, new_if_state);
+
+         psql_scan_reset(scan_state);
+     }
+
+     /*
+      * \elif <expr> is part of an \if..\endif block. <expr> must be a valid
+      * boolean expression, or the command will be ignored.
+      */
+     else if (strcmp(cmd, "elif") == 0)
+     {
+         bool elif_true = false;
+         switch (conditional_stack_peek(cstack))
+         {
+             case IFSTATE_IGNORED:
+                 /*
+                  * inactive branch, only test for valid expression.
+                  * either if-endif already had a true block,
+                  * or whole parent block is false.
+                  */
+                 success = read_boolean_expression(scan_state, "\\elif <expr>",
+                                                     &elif_true);
+                 break;
+             case IFSTATE_TRUE:
+                 /*
+                  * just finished true section of this if-endif, test for valid
+                  * expression, but then ignore the rest until \endif
+                  */
+                 success = read_boolean_expression(scan_state, "\\elif <expr>",
+                                                     &elif_true);
+                 if (success)
+                     conditional_stack_poke(cstack, IFSTATE_IGNORED);
+                 break;
+             case IFSTATE_FALSE:
+                 /*
+                  * have not yet found a true block in this if-endif,
+                  * this might be the first.
+                  */
+                 success = read_boolean_expression(scan_state, "\\elif <expr>",
+                                                     &elif_true);
+                 if (success && elif_true)
+                     conditional_stack_poke(cstack, IFSTATE_TRUE);
+                 break;
+             case IFSTATE_NONE:
+                 /* no if to elif from */
+                 psql_error("\\elif: no matching \\if\n");
+                 success = false;
+                 break;
+             case IFSTATE_ELSE_TRUE:
+             case IFSTATE_ELSE_FALSE:
+                 psql_error("\\elif: cannot occur after \\else\n");
+                 success = false;
+                 break;
+             default:
+                 break;
+         }
+         psql_scan_reset(scan_state);
+     }
+
+     /*
+      * \else is part of an \if..\endif block
+      * the statements within an \else branch will only be executed if
+      * all previous \if and \endif expressions evaluated to false
+      * and the block was not itself being ignored.
+      */
+     else if (strcmp(cmd, "else") == 0)
+     {
+         switch (conditional_stack_peek(cstack))
+         {
+             case IFSTATE_FALSE:
+                 /* just finished false section of an active branch */
+                 conditional_stack_poke(cstack, IFSTATE_ELSE_TRUE);
+                 break;
+             case IFSTATE_TRUE:
+             case IFSTATE_IGNORED:
+                 /*
+                  * either just finished true section of an active branch,
+                  * or whole branch was inactive. either way, be on the
+                  * lookout for any invalid \endif or \else commands
+                  */
+                 conditional_stack_poke(cstack, IFSTATE_ELSE_FALSE);
+                 break;
+             case IFSTATE_NONE:
+                 /* no if to else from */
+                 psql_error("\\else: no matching \\if\n");
+                 success = false;
+                 break;
+             case IFSTATE_ELSE_TRUE:
+             case IFSTATE_ELSE_FALSE:
+                 psql_error("\\else: cannot occur after \\else\n");
+                 success = false;
+                 break;
+             default:
+                 break;
+         }
+         psql_scan_reset(scan_state);
+     }
+
+     /*
+      * \endif - closing statment of an \if...\endif block
+      */
+     else if (strcmp(cmd, "endif") == 0)
+     {
+         /*
+          * get rid of this ifstate element and look at the previous
+          * one, if any
+          */
+         switch (conditional_stack_peek(cstack))
+         {
+             case IFSTATE_NONE:
+                 psql_error("\\endif: no matching \\if\n");
+                 success = false;
+                 break;
+             default:
+                 success = conditional_stack_pop(cstack);
+                 Assert(success);
+                 break;
+         }
+         psql_scan_reset(scan_state);
+     }
+
      /* \l is list databases */
      else if (strcmp(cmd, "l") == 0 || strcmp(cmd, "list") == 0 ||
               strcmp(cmd, "l+") == 0 || strcmp(cmd, "list+") == 0)
diff --git a/src/bin/psql/command.h b/src/bin/psql/command.h
index d0c3264..a396f29 100644
*** a/src/bin/psql/command.h
--- b/src/bin/psql/command.h
***************
*** 10,15 ****
--- 10,16 ----

  #include "fe_utils/print.h"
  #include "fe_utils/psqlscan.h"
+ #include "conditional.h"


  typedef enum _backslashResult
*************** typedef enum _backslashResult
*** 25,30 ****
--- 26,32 ----


  extern backslashResult HandleSlashCmds(PsqlScanState scan_state,
+                 ConditionalStack cstack,
                  PQExpBuffer query_buf);

  extern int    process_file(char *filename, bool use_relative_path);
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index 1aa56ab..83ac284 100644
*** a/src/bin/psql/common.c
--- b/src/bin/psql/common.c
*************** setQFout(const char *fname)
*** 119,124 ****
--- 119,129 ----
   * If "escape" is true, return the value suitably quoted and escaped,
   * as an identifier or string literal depending on "as_ident".
   * (Failure in escaping should lead to returning NULL.)
+  *
+  * Variables are not expanded if the current branch is inactive
+  * (part of an \if..\endif section which is false). \elif branches
+  * will need temporarily mark the branch active in order to
+  * properly evaluate conditionals.
   */
  char *
  psql_get_variable(const char *varname, bool escape, bool as_ident)
diff --git a/src/bin/psql/conditional.c b/src/bin/psql/conditional.c
index ...8643ff1 .
*** a/src/bin/psql/conditional.c
--- b/src/bin/psql/conditional.c
***************
*** 0 ****
--- 1,103 ----
+ /*
+  * psql - the PostgreSQL interactive terminal
+  *
+  * Copyright (c) 2000-2017, PostgreSQL Global Development Group
+  *
+  * src/bin/psql/conditional.c
+  */
+ #include "postgres_fe.h"
+
+ #include "conditional.h"
+
+ /*
+  * create stack
+  */
+ ConditionalStack
+ conditional_stack_create(void)
+ {
+     ConditionalStack cstack = pg_malloc(sizeof(ConditionalStackData));
+     cstack->head = NULL;
+     return cstack;
+ }
+
+ /*
+  * destroy stack
+  */
+ void
+ conditional_stack_destroy(ConditionalStack cstack)
+ {
+     while (conditional_stack_pop(cstack))
+         continue;
+     free(cstack);
+ }
+
+ /*
+  * Create a new conditional branch.
+  */
+ void
+ conditional_stack_push(ConditionalStack cstack,    ifState new_state)
+ {
+     IfStackElem *p = (IfStackElem*) pg_malloc(sizeof(IfStackElem));
+     p->if_state = new_state;
+     p->next = cstack->head;
+     cstack->head = p;
+ }
+
+ /*
+  * Destroy the topmost conditional branch.
+  * Returns false if there was no branch to end.
+  */
+ bool
+ conditional_stack_pop(ConditionalStack cstack)
+ {
+     IfStackElem *p = cstack->head;
+     if (!p)
+         return false;
+     cstack->head = cstack->head->next;
+     free(p);
+     return true;
+ }
+
+ /*
+  * Fetch the current state of the top of the stack
+  */
+ ifState
+ conditional_stack_peek(ConditionalStack cstack)
+ {
+     if (conditional_stack_empty(cstack))
+         return IFSTATE_NONE;
+     return cstack->head->if_state;
+ }
+
+ /*
+  * Change the state of the topmost branch.
+  * Returns false if there was no branch state to set.
+  */
+ bool
+ conditional_stack_poke(ConditionalStack cstack, ifState new_state)
+ {
+     if (conditional_stack_empty(cstack))
+         return false;
+     cstack->head->if_state = new_state;
+     return true;
+ }
+
+ /*
+  * True if there are no active \if-blocks
+  */
+ bool
+ conditional_stack_empty(ConditionalStack cstack)
+ {
+     return cstack->head == NULL;
+ }
+
+ /*
+  * True if the current conditional block is active, or if there is no
+  * open \if (ie, we should execute commands normally)
+  */
+ bool
+ conditional_active(ConditionalStack cstack)
+ {
+     ifState s = conditional_stack_peek(cstack);
+     return s == IFSTATE_NONE || s == IFSTATE_TRUE || s == IFSTATE_ELSE_TRUE;
+ }
diff --git a/src/bin/psql/conditional.h b/src/bin/psql/conditional.h
index ...96dbd7f .
*** a/src/bin/psql/conditional.h
--- b/src/bin/psql/conditional.h
***************
*** 0 ****
--- 1,62 ----
+ /*
+  * psql - the PostgreSQL interactive terminal
+  *
+  * Copyright (c) 2000-2017, PostgreSQL Global Development Group
+  *
+  * src/bin/psql/conditional.h
+  */
+ #ifndef CONDITIONAL_H
+ #define CONDITIONAL_H
+
+ /*
+  * Possible states of a single level of \if block.
+  */
+ typedef enum ifState
+ {
+     IFSTATE_NONE = 0,    /* Not currently in an \if block */
+     IFSTATE_TRUE,        /* currently in an \if or \elif which is true
+                          * and all parent branches (if any) are true */
+     IFSTATE_FALSE,        /* currently in an \if or \elif which is false
+                          * but no true branch has yet been seen,
+                          * and all parent branches (if any) are true */
+     IFSTATE_IGNORED,    /* currently in an \elif which follows a true \if
+                          * or the whole \if is a child of a false parent */
+     IFSTATE_ELSE_TRUE,    /* currently in an \else which is true
+                          * and all parent branches (if any) are true */
+     IFSTATE_ELSE_FALSE    /* currently in an \else which is false or ignored */
+ } ifState;
+
+ /*
+  * The state of nested \ifs is stored in a stack.
+  */
+ typedef struct IfStackElem
+ {
+     ifState        if_state;
+     struct IfStackElem *next;
+ } IfStackElem;
+
+ typedef struct ConditionalStackData
+ {
+     IfStackElem    *head;
+ } ConditionalStackData;
+
+ typedef struct ConditionalStackData *ConditionalStack;
+
+
+ extern ConditionalStack conditional_stack_create(void);
+
+ extern void conditional_stack_destroy(ConditionalStack cstack);
+
+ extern bool conditional_stack_empty(ConditionalStack cstack);
+
+ extern void conditional_stack_push(ConditionalStack cstack, ifState new_state);
+
+ extern bool conditional_stack_pop(ConditionalStack cstack);
+
+ extern ifState conditional_stack_peek(ConditionalStack cstack);
+
+ extern bool conditional_stack_poke(ConditionalStack cstack, ifState new_state);
+
+ extern bool conditional_active(ConditionalStack cstack);
+
+ #endif   /* CONDITIONAL_H */
diff --git a/src/bin/psql/copy.c b/src/bin/psql/copy.c
index 481031a..2005b9a 100644
*** a/src/bin/psql/copy.c
--- b/src/bin/psql/copy.c
*************** handleCopyIn(PGconn *conn, FILE *copystr
*** 552,558 ****
          /* interactive input probably silly, but give one prompt anyway */
          if (showprompt)
          {
!             const char *prompt = get_prompt(PROMPT_COPY);

              fputs(prompt, stdout);
              fflush(stdout);
--- 552,558 ----
          /* interactive input probably silly, but give one prompt anyway */
          if (showprompt)
          {
!             const char *prompt = get_prompt(PROMPT_COPY, NULL);

              fputs(prompt, stdout);
              fflush(stdout);
*************** handleCopyIn(PGconn *conn, FILE *copystr
*** 590,596 ****

              if (showprompt)
              {
!                 const char *prompt = get_prompt(PROMPT_COPY);

                  fputs(prompt, stdout);
                  fflush(stdout);
--- 590,596 ----

              if (showprompt)
              {
!                 const char *prompt = get_prompt(PROMPT_COPY, NULL);

                  fputs(prompt, stdout);
                  fflush(stdout);
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index ba14df0..79afafb 100644
*** a/src/bin/psql/help.c
--- b/src/bin/psql/help.c
*************** slashUsage(unsigned short int pager)
*** 210,215 ****
--- 210,222 ----
      fprintf(output, _("  \\qecho [STRING]        write string to query output stream (see \\o)\n"));
      fprintf(output, "\n");

+     fprintf(output, _("Conditionals\n"));
+     fprintf(output, _("  \\if <expr>             begin a conditional block\n"));
+     fprintf(output, _("  \\elif <expr>           else if in the current conditional block\n"));
+     fprintf(output, _("  \\else                  else in the current conditional block\n"));
+     fprintf(output, _("  \\endif                 end current conditional block\n"));
+     fprintf(output, "\n");
+
      fprintf(output, _("Informational\n"));
      fprintf(output, _("  (options: S = show system objects, + = additional detail)\n"));
      fprintf(output, _("  \\d[S+]                 list tables, views, and sequences\n"));
diff --git a/src/bin/psql/mainloop.c b/src/bin/psql/mainloop.c
index 6e358e2..47d6c50 100644
*** a/src/bin/psql/mainloop.c
--- b/src/bin/psql/mainloop.c
*************** const PsqlScanCallbacks psqlscan_callbac
*** 25,30 ****
--- 25,47 ----


  /*
+  * execute query if branch is active.
+  * warn interactive users about ignored queries.
+  */
+ static bool
+ send_query(const char *query, ConditionalStack cstack)
+ {
+     /* execute query if branch is active */
+     if (conditional_active(cstack))
+         return SendQuery(query);
+
+     if (pset.cur_cmd_interactive)
+         psql_error("query ignored; use \\endif or Ctrl-C to exit current \\if branch\n");
+
+     return true;
+ }
+
+ /*
   * Main processing loop for reading lines of input
   *    and sending them to the backend.
   *
*************** int
*** 35,40 ****
--- 52,58 ----
  MainLoop(FILE *source)
  {
      PsqlScanState scan_state;    /* lexer working state */
+     ConditionalStack cond_stack;    /* \if status stack */
      volatile PQExpBuffer query_buf;        /* buffer for query being accumulated */
      volatile PQExpBuffer previous_buf;    /* if there isn't anything in the new
                                           * buffer yet, use this one for \e,
*************** MainLoop(FILE *source)
*** 69,74 ****
--- 86,92 ----

      /* Create working state */
      scan_state = psql_scan_create(&psqlscan_callbacks);
+     cond_stack = conditional_stack_create();

      query_buf = createPQExpBuffer();
      previous_buf = createPQExpBuffer();
*************** MainLoop(FILE *source)
*** 122,128 ****
--- 140,157 ----
              cancel_pressed = false;

              if (pset.cur_cmd_interactive)
+             {
                  putc('\n', stdout);
+                 /*
+                  * if interactive user is in a branch, then Ctrl-C will exit
+                  * from the inner-most branch
+                  */
+                 if (!conditional_stack_empty(cond_stack))
+                 {
+                     psql_error("\\if: escaped\n");
+                     conditional_stack_pop(cond_stack);
+                 }
+             }
              else
              {
                  successResult = EXIT_USER;
*************** MainLoop(FILE *source)
*** 140,146 ****
              /* May need to reset prompt, eg after \r command */
              if (query_buf->len == 0)
                  prompt_status = PROMPT_READY;
!             line = gets_interactive(get_prompt(prompt_status), query_buf);
          }
          else
          {
--- 169,176 ----
              /* May need to reset prompt, eg after \r command */
              if (query_buf->len == 0)
                  prompt_status = PROMPT_READY;
!             line = gets_interactive(get_prompt(prompt_status, cond_stack),
!                                     query_buf);
          }
          else
          {
*************** MainLoop(FILE *source)
*** 297,303 ****
                  }

                  /* execute query */
!                 success = SendQuery(query_buf->data);
                  slashCmdStatus = success ? PSQL_CMD_SEND : PSQL_CMD_ERROR;
                  pset.stmt_lineno = 1;

--- 327,333 ----
                  }

                  /* execute query */
!                 success = send_query(query_buf->data, cond_stack);
                  slashCmdStatus = success ? PSQL_CMD_SEND : PSQL_CMD_ERROR;
                  pset.stmt_lineno = 1;

*************** MainLoop(FILE *source)
*** 343,348 ****
--- 373,379 ----

                  /* execute backslash command */
                  slashCmdStatus = HandleSlashCmds(scan_state,
+                                                  cond_stack,
                                                   query_buf->len > 0 ?
                                                   query_buf : previous_buf);

*************** MainLoop(FILE *source)
*** 358,364 ****

                  if (slashCmdStatus == PSQL_CMD_SEND)
                  {
!                     success = SendQuery(query_buf->data);

                      /* transfer query to previous_buf by pointer-swapping */
                      {
--- 389,395 ----

                  if (slashCmdStatus == PSQL_CMD_SEND)
                  {
!                     success = send_query(query_buf->data, cond_stack);

                      /* transfer query to previous_buf by pointer-swapping */
                      {
*************** MainLoop(FILE *source)
*** 430,436 ****
              pg_send_history(history_buf);

          /* execute query */
!         success = SendQuery(query_buf->data);

          if (!success && die_on_error)
              successResult = EXIT_USER;
--- 461,467 ----
              pg_send_history(history_buf);

          /* execute query */
!         success = send_query(query_buf->data, cond_stack);

          if (!success && die_on_error)
              successResult = EXIT_USER;
*************** MainLoop(FILE *source)
*** 439,444 ****
--- 470,488 ----
      }

      /*
+      * Check for unbalanced \if-\endifs unless user explicitly quit, or the
+      * script is erroring out
+      */
+     if (slashCmdStatus != PSQL_CMD_TERMINATE &&
+         successResult != EXIT_USER &&
+         !conditional_stack_empty(cond_stack))
+     {
+         psql_error("reached EOF without finding closing \\endif(s)\n");
+         if (die_on_error && !pset.cur_cmd_interactive)
+             successResult = EXIT_USER;
+     }
+
+     /*
       * Let's just make real sure the SIGINT handler won't try to use
       * sigint_interrupt_jmp after we exit this routine.  If there is an outer
       * MainLoop instance, it will reset sigint_interrupt_jmp to point to
*************** MainLoop(FILE *source)
*** 452,457 ****
--- 496,502 ----
      destroyPQExpBuffer(history_buf);

      psql_scan_destroy(scan_state);
+     conditional_stack_destroy(cond_stack);

      pset.cur_cmd_source = prev_cmd_source;
      pset.cur_cmd_interactive = prev_cmd_interactive;
diff --git a/src/bin/psql/prompt.c b/src/bin/psql/prompt.c
index f7930c4..e502ff3 100644
*** a/src/bin/psql/prompt.c
--- b/src/bin/psql/prompt.c
***************
*** 66,72 ****
   */

  char *
! get_prompt(promptStatus_t status)
  {
  #define MAX_PROMPT_SIZE 256
      static char destination[MAX_PROMPT_SIZE + 1];
--- 66,72 ----
   */

  char *
! get_prompt(promptStatus_t status, ConditionalStack cstack)
  {
  #define MAX_PROMPT_SIZE 256
      static char destination[MAX_PROMPT_SIZE + 1];
*************** get_prompt(promptStatus_t status)
*** 188,194 ****
                      switch (status)
                      {
                          case PROMPT_READY:
!                             if (!pset.db)
                                  buf[0] = '!';
                              else if (!pset.singleline)
                                  buf[0] = '=';
--- 188,196 ----
                      switch (status)
                      {
                          case PROMPT_READY:
!                             if (cstack != NULL && !conditional_active(cstack))
!                                 buf[0] = '@';
!                             else if (!pset.db)
                                  buf[0] = '!';
                              else if (!pset.singleline)
                                  buf[0] = '=';
diff --git a/src/bin/psql/prompt.h b/src/bin/psql/prompt.h
index 977e754..b3d2d98 100644
*** a/src/bin/psql/prompt.h
--- b/src/bin/psql/prompt.h
***************
*** 10,16 ****

  /* enum promptStatus_t is now defined by psqlscan.h */
  #include "fe_utils/psqlscan.h"

! char       *get_prompt(promptStatus_t status);

  #endif   /* PROMPT_H */
--- 10,17 ----

  /* enum promptStatus_t is now defined by psqlscan.h */
  #include "fe_utils/psqlscan.h"
+ #include "conditional.h"

! char       *get_prompt(promptStatus_t status, ConditionalStack cstack);

  #endif   /* PROMPT_H */
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
index 694f0ef..353d6c8 100644
*** a/src/bin/psql/startup.c
--- b/src/bin/psql/startup.c
*************** main(int argc, char *argv[])
*** 331,349 ****
              else if (cell->action == ACT_SINGLE_SLASH)
              {
                  PsqlScanState scan_state;

                  if (pset.echo == PSQL_ECHO_ALL)
                      puts(cell->val);

                  scan_state = psql_scan_create(&psqlscan_callbacks);
                  psql_scan_setup(scan_state,
                                  cell->val, strlen(cell->val),
                                  pset.encoding, standard_strings());

!                 successResult = HandleSlashCmds(scan_state, NULL) != PSQL_CMD_ERROR
                      ? EXIT_SUCCESS : EXIT_FAILURE;

                  psql_scan_destroy(scan_state);
              }
              else if (cell->action == ACT_FILE)
              {
--- 331,354 ----
              else if (cell->action == ACT_SINGLE_SLASH)
              {
                  PsqlScanState scan_state;
+                 ConditionalStack cond_stack;

                  if (pset.echo == PSQL_ECHO_ALL)
                      puts(cell->val);

                  scan_state = psql_scan_create(&psqlscan_callbacks);
+                 cond_stack = conditional_stack_create();
                  psql_scan_setup(scan_state,
                                  cell->val, strlen(cell->val),
                                  pset.encoding, standard_strings());

!                 successResult = HandleSlashCmds(scan_state,
!                                                 cond_stack,
!                                                 NULL) != PSQL_CMD_ERROR
                      ? EXIT_SUCCESS : EXIT_FAILURE;

                  psql_scan_destroy(scan_state);
+                 conditional_stack_destroy(cond_stack);
              }
              else if (cell->action == ACT_FILE)
              {
diff --git a/src/bin/psql/t/001_if.pl b/src/bin/psql/t/001_if.pl
index ...8180dab .
*** a/src/bin/psql/t/001_if.pl
--- b/src/bin/psql/t/001_if.pl
***************
*** 0 ****
--- 1,42 ----
+ use strict;
+ use warnings;
+
+ use Config;
+ use PostgresNode;
+ use TestLib;
+ use Test::More tests => 21;
+
+ #
+ # test that invalid \if respects ON_ERROR_STOP
+ #
+ my $node = get_new_node('master');
+ $node->init;
+ $node->start;
+
+ my $tests = [
+     # syntax errors
+     [ "\\if invalid\n\\echo NO\n\\endif\n\\echo NO\n", '', 'boolean expected', '\if syntax error' ],
+     [ "\\if false\n\\elif invalid\n\\echo NO\n\\endif\n\\echo NO\n", '', 'boolean expected', '\elif syntax error' ],
+     # unmatched checks
+     [ "\\if true\n", '', 'reached EOF without finding closing .endif', 'unmatched \if' ],
+     [ "\\elif true\n\\echo NO\n", '', '.elif: no matching .if', 'unmatched \elif' ],
+     [ "\\else\n\\echo NO\n", '', '.else: no matching .if', 'unmatched \else' ],
+     [ "\\endif\n\\echo NO\n", '', '.endif: no matching .if', 'unmatched \endif' ],
+     # error stop messages
+     [ "\\set ON_ERROR_STOP on\n\\if error\n\\echo NO\n\\endif\n\\echo NO\n", '',
+         'unrecognized value "error" for ".if <expr>": boolean expected', 'if error'],
+ ];
+
+ # 3 checks per tests
+ for my $test (@$tests) {
+   my ($script, $stdout_expect, $stderr_re, $name) = @$test;
+   my ($stdout, $stderr);
+   my $retcode = $node->psql('postgres', $script,
+         stdout => \$stdout, stderr => \$stderr,
+         on_error_stop => 1);
+   is($retcode,'3',"$name test ON_ERROR_STOP");
+   is($stdout, $stdout_expect, "$name test STDOUT");
+   like($stderr, qr/$stderr_re/, "$name test STDERR");
+ }
+
+ $node->teardown_node;
diff --git a/src/test/regress/expected/psql.out b/src/test/regress/expected/psql.out
index eb7f197..76a6d18 100644
*** a/src/test/regress/expected/psql.out
--- b/src/test/regress/expected/psql.out
*************** deallocate q;
*** 2735,2740 ****
--- 2735,2843 ----
  \pset format aligned
  \pset expanded off
  \pset border 1
+ -- test a large nested if using a variety of true-equivalents
+ \if true
+     \if 1
+         \if yes
+             \if on
+                 \echo 'all true'
+ all true
+             \else
+                 \echo 'should not print #1-1'
+             \endif
+         \else
+             \echo 'should not print #1-2'
+         \endif
+     \else
+         \echo 'should not print #1-3'
+     \endif
+ \else
+     \echo 'should not print #1-4'
+ \endif
+ -- test a variety of false-equivalents in an if/elif/else structure
+ \if false
+     \echo 'should not print #2-1'
+ \elif 0
+     \echo 'should not print #2-2'
+ \elif no
+     \echo 'should not print #2-3'
+ \elif off
+     \echo 'should not print #2-4'
+ \else
+     \echo 'all false'
+ all false
+ \endif
+ -- test simple true-then-else
+ \if true
+     \echo 'first thing true'
+ first thing true
+ \else
+     \echo 'should not print #3-1'
+ \endif
+ -- test simple false-true-else
+ \if false
+     \echo 'should not print #4-1'
+ \elif true
+     \echo 'second thing true'
+ second thing true
+ \else
+     \echo 'should not print #5-1'
+ \endif
+ -- invalid boolean expressions mean the \if is ignored
+ \if invalid_boolean_expression
+ unrecognized value "invalid_boolean_expression" for "\if <expr>": boolean expected
+     \echo 'will print anyway #6-1'
+ will print anyway #6-1
+ \else
+ \else: no matching \if
+     \echo 'will print anyway #6-2'
+ will print anyway #6-2
+ \endif
+ \endif: no matching \if
+ -- test un-matched endif
+ \endif
+ \endif: no matching \if
+ -- test un-matched else
+ \else
+ \else: no matching \if
+ -- test un-matched elif
+ \elif
+ \elif: no matching \if
+ -- test double-else error
+ \if true
+ \else
+ \else
+ \else: cannot occur after \else
+ \endif
+ -- test elif out-of-order
+ \if false
+ \else
+ \elif
+ \elif: cannot occur after \else
+ \endif
+ -- test if-endif matching in a false branch
+ \if false
+     \if false
+         \echo 'should not print #7-1'
+     \else
+         \echo 'should not print #7-2'
+     \endif
+     \echo 'should not print #7-3'
+ \else
+     \echo 'should print #7-4'
+ should print #7-4
+ \endif
+ -- show that variables still expand even in false blocks
+ \set var 'ab''cd'
+ -- select :var;
+ \if false
+   select :var;
+ -- this will be skipped because of an unterminated string
+ \endif
+ -- fix the unterminated string
+ ';
+ -- now the if block can be properly ended
+ \endif
  -- SHOW_CONTEXT
  \set SHOW_CONTEXT never
  do $$
diff --git a/src/test/regress/sql/psql.sql b/src/test/regress/sql/psql.sql
index 8f8e17a..6d1a6b5 100644
*** a/src/test/regress/sql/psql.sql
--- b/src/test/regress/sql/psql.sql
*************** deallocate q;
*** 382,387 ****
--- 382,487 ----
  \pset expanded off
  \pset border 1

+ -- test a large nested if using a variety of true-equivalents
+ \if true
+     \if 1
+         \if yes
+             \if on
+                 \echo 'all true'
+             \else
+                 \echo 'should not print #1-1'
+             \endif
+         \else
+             \echo 'should not print #1-2'
+         \endif
+     \else
+         \echo 'should not print #1-3'
+     \endif
+ \else
+     \echo 'should not print #1-4'
+ \endif
+
+ -- test a variety of false-equivalents in an if/elif/else structure
+ \if false
+     \echo 'should not print #2-1'
+ \elif 0
+     \echo 'should not print #2-2'
+ \elif no
+     \echo 'should not print #2-3'
+ \elif off
+     \echo 'should not print #2-4'
+ \else
+     \echo 'all false'
+ \endif
+
+ -- test simple true-then-else
+ \if true
+     \echo 'first thing true'
+ \else
+     \echo 'should not print #3-1'
+ \endif
+
+ -- test simple false-true-else
+ \if false
+     \echo 'should not print #4-1'
+ \elif true
+     \echo 'second thing true'
+ \else
+     \echo 'should not print #5-1'
+ \endif
+
+ -- invalid boolean expressions mean the \if is ignored
+ \if invalid_boolean_expression
+     \echo 'will print anyway #6-1'
+ \else
+     \echo 'will print anyway #6-2'
+ \endif
+
+ -- test un-matched endif
+ \endif
+
+ -- test un-matched else
+ \else
+
+ -- test un-matched elif
+ \elif
+
+ -- test double-else error
+ \if true
+ \else
+ \else
+ \endif
+
+ -- test elif out-of-order
+ \if false
+ \else
+ \elif
+ \endif
+
+ -- test if-endif matching in a false branch
+ \if false
+     \if false
+         \echo 'should not print #7-1'
+     \else
+         \echo 'should not print #7-2'
+     \endif
+     \echo 'should not print #7-3'
+ \else
+     \echo 'should print #7-4'
+ \endif
+
+ -- show that variables still expand even in false blocks
+ \set var 'ab''cd'
+ -- select :var;
+ \if false
+   select :var;
+ -- this will be skipped because of an unterminated string
+ \endif
+ -- fix the unterminated string
+ ';
+ -- now the if block can be properly ended
+ \endif
+
  -- SHOW_CONTEXT

  \set SHOW_CONTEXT never

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers