On Mon, Feb 6, 2017 at 5:49 PM, Corey Huinker <corey.huinker@gmail.com> wrote:
> 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.
I (still) think this is a bad design. Even if you've got all the
messages just right as things stand today, some new feature that comes
along in the future can change things so that they're not right any
more, and nobody's going to relish maintaining this. This sort of
thing seems fine to me:
new \if is invalid
But then further breaking it down by things like whether
ON_ERROR_STOP=1 is set, or breaking down the \endif output depending
on the surrounding context, seems terrifyingly complex to me.
Mind you, I'm not planning to commit this patch anyway, so feel free
to ignore me, but if I were planning to commit it, I would not commit
it with that level of chattiness.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company