Re: \if, \elseif, \else, \endif (was Re: [HACKERS] PSQL commands:\quit_if, \quit_unless) - Mailing list pgsql-hackers

From Fabien COELHO
Subject Re: \if, \elseif, \else, \endif (was Re: [HACKERS] PSQL commands:\quit_if, \quit_unless)
Date
Msg-id alpine.DEB.2.20.1701232007150.31421@lancre
Whole thread Raw
In response to \if, \elseif, \else, \endif (was Re: [HACKERS] PSQL commands:\quit_if, \quit_unless)  (Corey Huinker <corey.huinker@gmail.com>)
Responses Re: \if, \elseif, \else, \endif (was Re: [HACKERS] PSQL commands:\quit_if, \quit_unless)
Re: \if, \elseif, \else, \endif (was Re: [HACKERS] PSQL commands:\quit_if, \quit_unless)
List 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.



pgsql-hackers by date:

Previous
From: Pavel Stehule
Date:
Subject: [HACKERS] PoC plpgsql - possibility to force custom or generic plan
Next
From: Peter Eisentraut
Date:
Subject: Re: [HACKERS] [COMMITTERS] pgsql: Generate fmgr prototypesautomatically