From: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> |
---|---|
To: | Corey Huinker <corey(dot)huinker(at)gmail(dot)com> |
Cc: | PostgreSQL <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: \if, \elseif, \else, \endif (was Re: PSQL commands: \quit_if, \quit_unless) |
Date: | 2017-01-23 20:37:14 |
Message-ID: | alpine.DEB.2.20.1701232007150.31421@lancre |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
> And here's the patch. I've changed the subject line and will be submitting
> a new entry to the commitfest.
A few comments:
Patch applies, make check is ok, but currently really too partial.
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.
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.
Tests: On principle they should include echos in all non executed branches
to reassure that they are indeed not executed.
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.
1: unrecognized value "whatever" for "\if"; assuming "on"
I do not think that the script should continue with such an assumption.
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...
Code:
- if (status != PSQL_CMD_ERROR)
+ if ((status != PSQL_CMD_ERROR) && psqlscan_branch_active(scan_state))
Why the added parenthesis?
case ...: case ...:
The project rules are to add explicit /* PASSTHROUGH */ comment.
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?
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.
--
Fabien.
From | Date | Subject | |
---|---|---|---|
Next Message | Peter Eisentraut | 2017-01-23 20:38:06 | Re: [COMMITTERS] pgsql: Generate fmgr prototypes automatically |
Previous Message | Pavel Stehule | 2017-01-23 20:10:49 | PoC plpgsql - possibility to force custom or generic plan |