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...


  -       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.


Sent via pgsql-hackers mailing list (
To make changes to your subscription:

Reply via email to