Re: \if, \elseif, \else, \endif (was Re: [HACKERS] PSQL commands:\quit_if, \quit_unless)

I'm wondering what pg would do on "EXISTS(SELECT 1 FROM customer)" if there are many employees. EXPLAIN suggests a seq_scan, which seems bad. With "(SELECT COUNT(*) FROM pgbench_accounts) <> 0" pg seems to generate an index only scan on a large table, so maybe it is a better way to do it. It seems strange that there is no better way to do that in a relational tool, the relational model being an extension of set theory... and there is no easy way (?) to check whether a set is empty.

"""If an EOF is reached on the main file or an <command>\include</command>-ed file before all <command>\if</command>-<command>\endif</command> are matched, then psql will raise an error."""

In sentence above: "before all" -> "before all local"? "then" -> ""?

"other options booleans" -> "other booleans of options"? or "options' booleans" maybe, but that is for people?

"unabigous" -> "unambiguous"

I think that the three paragraph explanation about non evaluation could be factor into one, maybe something like: """Lines within false branches are not evaluated in any way: queries are not sent to the server, non conditional commands are not evaluated but bluntly ignored, nested if expressions in such branches are also not evaluated but are tallied to check for nesting."""

I would suggest to merge elif/else constraints by describing what is expected rather than what is not expected. """An \if command may contain any number of \elif clauses and may end with one \else clause""".

## CODE

In "read_boolean_expression":

 + if (value)

"if (value != NULL)" is usually used, I think.

 + if (value == NULL)
 +   return false; /* not set -> assume "off" */

This is dead code, because value has been checked to be non NULL a few lines above.

 + (pg_strncasecmp(value, "on", (len > 2 ? len : 2)) == 0)
 + (pg_strncasecmp(value, "off", (len > 2 ? len : 2)) == 0)

Hmmm, not easy to parse. Maybe it deserves a comment?
"check at least two chars to distinguish on & off"

",action" -> ", action" (space after commas).

The "result" is not set on errors, but maybe it should be set to false anyway and explicitely, instead of relying on some prior initialization?
Or document that the result is not set on errors.

if command:

  if (is active) {
    success = ...
    if (success) {
      ...
    }
  }
  if (success) {
    ...
  }

The second test on success may not rely on the value set above. That looks very strange. ISTM that the state should be pushed whether parsing succeeded or not. Moreover, it results in:

  \if ERROR
     \echo X
  \else
     \echo Y
  \endif

having both X & Y printed and error reported on else and endif. I think that an expression error should just put the stuff in ignore state.

On "else" when in state ignored, ISTM that it should remain in state ignore, not switch to else-false.

Comment about "IFSTATE_FALSE" talks about the state being true, maybe a copy-paste error?

In comments: "... variables the branch" -> "variables if the branch"

The "if_endifs_balanced" variable is not very useful. Maybe just the test and error reporting in the right place:

 if (... && !psqlscan_branch_empty(scan_state))
   psql_error("found EOF before closing \\endif(s)\n");

History saving: I'm wondering whether all read line should be put into history, whether executed or not.

Is it possible to make some of the added functions static? If so, do it.

I checked that it does stop on errors with -v ON_ERROR_STOP=1. However I would be more at ease if this was tested somewhere.