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

Поиск
Список
Период
Сортировка
От Fabien COELHO
Тема Re: \if, \elseif, \else, \endif (was Re: [HACKERS] PSQL commands:\quit_if, \quit_unless)
Дата
Msg-id alpine.DEB.2.20.1701232007150.31421@lancre
обсуждение исходный текст
Ответ на \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)  (Corey Huinker <corey.huinker@gmail.com>)
Список 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 по дате отправления:

Предыдущее
От: Pavel Stehule
Дата:
Сообщение: [HACKERS] PoC plpgsql - possibility to force custom or generic plan
Следующее
От: Peter Eisentraut
Дата:
Сообщение: Re: [HACKERS] [COMMITTERS] pgsql: Generate fmgr prototypesautomatically