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=dERV+SvC=RfSQSvME0tkSnxs_9KU+gNxSUrQ_+4uri2Q@mail.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)
(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) ("Daniel Verite" <daniel@manitou-mail.org>) |
| Список | pgsql-hackers |
I do not like the choice of "elseif", which exists in PHP & VB. PL/pgSQL has "ELSIF" like perl, that I do not like much, though. Given the syntax and behavioral proximity with cpp, I suggest that "elif" would be a better choice.
I'm personally indifferent to elif vs elsif vs elseif, but I think elseif was the consensus. It's easy enough to change.
Documentation: I do not think that the systematic test-like example in the documentation is relevant, a better example should be shown. The list of what is considered true or false should be told explicitely, not end with "etc" that is everyone to guess.
It was copied from the regression test. I knew there would be follow-up suggestions for what should be shown.
Tests: On principle they should include echos in all non executed branches to reassure that they are indeed not executed.
Will do.
Also, no error cases are tested. They should be. Maybe it is not very easy to test some cases which expect to make psql generate errors, but I feel they are absolutely necessary for such a feature.
Will do.
1: unrecognized value "whatever" for "\if"; assuming "on"
I do not think that the script should continue with such an assumption.
I agree, and this means we can't use ParseVariableBool() as-is. I already broke out argument reading to it's own function, knowing that it'd be the stub for expressions. So I guess we start that now. What subset of true-ish values do you think we should support? If we think that real expressions are possible soon, we could only allow 'true' and 'false' for now, but if we expect that expressions might not make it into v10, then perhaps we should support the same text values that coerce to booleans on the server side.
2: encountered \else after \else ... "# ERROR BEFORE"
Even with ON_ERROR_STOP activated the execution continues.
3: no \endif
Error reported, but it does not stop even with ON_ERROR_STOP.
4: include with bad nesting...
Again, even with ON_ERROR_STOP, the execution continues...
All valid issues. Will add those to the regression as well (with ON_ERROR_STOP disabled, obviously).
Code:
- if (status != PSQL_CMD_ERROR)
+ if ((status != PSQL_CMD_ERROR) && psqlscan_branch_active(scan_state))
Why the added parenthesis?
Will fix.
case ...: case ...:
The project rules are to add explicit /* PASSTHROUGH */ comment.
Will do.
There are spaces at the end of one line in a comment about psqlscan_branch_end_state.
There are multiple "TODO" comments in the file... Is it done or work in progress?
I forgot to remove them. But it would be wildly optimistic of me to think there would be no more work for me after the first patch submission.
Usually in pg comments about a function do not repeat the function name. For very short comment, they are /* inlined */ on one line. Use pg comment style.
В списке pgsql-hackers по дате отправления:
Предыдущее
От: Tom LaneДата:
Сообщение: Re: [HACKERS] IndexBuild Function call fcinfo cannot access memory