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

Поиск
Список
Период
Сортировка
От Corey Huinker
Тема Re: \if, \elseif, \else, \endif (was Re: [HACKERS] PSQL commands:\quit_if, \quit_unless)
Дата
Msg-id CADkLM=eVD+SXB0LD6J1pn+NiWn4sLqR7RUpMpd8FPnvyToyWjw@mail.gmail.com
обсуждение исходный текст
Ответ на Re: \if, \elseif, \else, \endif (was Re: [HACKERS] PSQL commands:\quit_if, \quit_unless)  (Corey Huinker <corey.huinker@gmail.com>)
Ответы Re: \if, \elseif, \else, \endif (was Re: [HACKERS] PSQL commands:\quit_if, \quit_unless)  (Fabien COELHO <coelho@cri.ensmp.fr>)
Re: \if, \elseif, \else, \endif (was Re: [HACKERS] PSQL commands:\quit_if, \quit_unless)  (Robert Haas <robertmhaas@gmail.com>)
Список pgsql-hackers


On Mon, Feb 6, 2017 at 3:43 PM, Corey Huinker <corey.huinker@gmail.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.
Вложения

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Alvaro Herrera
Дата:
Сообщение: Re: [HACKERS] multivariate statistics (v19)
Следующее
От: Michael Paquier
Дата:
Сообщение: Re: [HACKERS] IF [NOT] EXISTS for replication slots