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

On Mon, Feb 6, 2017 at 3:43 PM, Corey Huinker <corey(dot)huinker(at)gmail(dot)com>
wrote:

> I did some more tests. I found a subtlety that I missed before: when
>> running under ON_ERROR_STOP, messages are not fully consistent:
>>
>> sh> cat test.sql
>> \set ON_ERROR_STOP on
>> \if error
>> \echo NO
>> \endif
>> \echo NO
>>
>> sh> ./psql < test.sql
>> SET
>> # ok
>> unrecognized value "error" for "\if <expr>": boolean expected
>> # ok
>
>
> That's straight from ParseVariableBool, and we can keep that or suppress
> it. Whatever we do, we should do it with the notion that more complex
> expressions will eventually be allowed, but they'll still have to resolve
> to something that's a text boolean.
>
>
>>
>> new \if is invalid, ignoring commands until next \endif
>> # hmmm... but it does not, it is really stopping immediately...
>
> found EOF before closing \endif(s)
>> # no, it has just stopped before EOF because of the error...
>>
>
> Yeah, chattiness caught up to us here. Both of these messages can be
> suppressed, I think.
>
>
>>
>> Also I'm not quite sure why psql decided that it is in interactive mode
>> above, its stdin is a file, but why not.
>>
>> The issue is made more explicit with -f:
>>
>> sh> ./psql -f test.sql
>> SET
>> psql:test.sql:2: unrecognized value "error" for "\if <expr>": boolean
>> expected
>> psql:test.sql:2: new \if is invalid, ignoring commands until next \endif
>> psql:test.sql:2: found EOF before closing \endif(s)
>>
>> It stopped on line 2, which is expected, but it was not on EOF.
>>
>> I think that the message when stopping should be ", stopping as required
>> by ON_ERROR_STOP" or something like that instead of ", ignoring...", and
>> the EOF message should not be printed at all in this case.
>
>
> I agree, and will look into making that happen. Thanks for the test case.
>
>

I suppressed the endif-balance checking in cases where we're in an
already-failed situation.
In cases where there was a boolean parsing failure, and ON_ERROR_STOP is
on, the error message no longer speak of a future which the session does
not have. I could left the ParseVariableBool() message as the only one, but
wasn't sure that that was enough of an error message on its own.
Added the test case to the existing tap tests. Incidentally, the tap tests
aren't presently fooled into thinking they're interactive.

$ cat test2.sql
\if error
\echo NO
\endif
\echo NOPE
$ psql test < test2.sql -v ON_ERROR_STOP=0
unrecognized value "error" for "\if <expr>": boolean expected
new \if is invalid, ignoring commands until next \endif
NOPE
$ psql test < test2.sql -v ON_ERROR_STOP=1
unrecognized value "error" for "\if <expr>": boolean expected
new \if is invalid.
$ psql test -f test2.sql -v ON_ERROR_STOP=0
psql:test2.sql:1: unrecognized value "error" for "\if <expr>": boolean
expected
psql:test2.sql:1: new \if is invalid, ignoring commands until next \endif
NOPE
$ psql test -f test2.sql -v ON_ERROR_STOP=1
psql:test2.sql:1: unrecognized value "error" for "\if <expr>": boolean
expected
psql:test2.sql:1: new \if is invalid.

Revised cumulative patch attached for those playing along at home.