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

Поиск
Список
Период
Сортировка
От Corey Huinker
Тема Re: [HACKERS] \if, \elseif, \else, \endif (was Re: PSQL commands:\quit_if, \quit_unless)
Дата
Msg-id CADkLM=dwGETiOV=3J=7JGM=acoMRgHz2CnrETek-wdYYE6AuJQ@mail.gmail.com
обсуждение исходный текст
Ответ на Re: [HACKERS] \if, \elseif, \else, \endif (was Re: PSQL commands:\quit_if, \quit_unless)  (Fabien COELHO <coelho@cri.ensmp.fr>)
Ответы Re: [HACKERS] \if, \elseif, \else, \endif (was Re: PSQL commands:\quit_if, \quit_unless)  (Corey Huinker <corey.huinker@gmail.com>)
Список pgsql-hackers
Дерево обсуждения
On Sun, Feb 26, 2017 at 2:47 AM, Fabien COELHO <coelho@cri.ensmp.fr> wrote:

Hello Corey,

About v18: Patch applies, make check ok, psql tap tests ok.


ISTM that contrary to the documentation "\elif something" is not evaluated in all cases, and the resulting code is harder to understand with a nested switch and condition structure:

  switch
  default
    read
    if
       switch

I wish (this is a personal taste) it could avoid switch-nesting on the very same value. It should also conform to the documentation.

I wasn't too happy with it, but I figured it would spark discussion. I succeeded.
 

If there is no compelling reason for the switch-nesting, I would suggest to move the read_boolean_expression before the swich, to deal with error immediately there, and then to have just one switch.

I thought about doing it that way. However, in the case of:

\set x invalid
\if true
\else
\elif :x
\endif

The error has already "happened" at line 4, char 5, and it doesn't matter what expression follows, you will get an error.  But because read_boolean_expression() can emit errors, you would see the error saying "invalid" isn't a valid boolean expression, and then see another error saying that the \elif was out of place. If we suppress read_boolean_expression()'s error reporting, then we have to re-create that error message ourselves, or be fine with the error message on invalid \elifs being inconsistent with invalid \ifs.  

Similar to your suggestion below, we could encapsulate the first switch into a function valid_elif_context(ConditionalStack), which might make the code cleaner, but would make it harder to see that all swtich-cases are covered between the two. That might be a tradeoff we want to make.
 

Alternatively if the structure must really be kept, then deal with errors in a first switch, read value *after* switch and deal with other errors there, then start a second switch, and adjust the documentation accordingly?

  switch
    errors
  read
  if
    errors
  // no error
  switch


How would the documentation have to change?

Also, the %R documentation has single line marker '^' before not executed '@', but it is the reverse in the code.

Noted and fixed in the next patch, good catch.
 

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Tom Lane
Дата:
Сообщение: Re: [HACKERS] I propose killing PL/Tcl's "modules" infrastructure
Следующее
От: Andrew Dunstan
Дата:
Сообщение: Re: [HACKERS] btree_gin and btree_gist for enums