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

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.

In response to

Responses

Browse pgsql-hackers by date

  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