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

From: Corey Huinker <corey(dot)huinker(at)gmail(dot)com>
To: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Daniel Verite <daniel(at)manitou-mail(dot)org>, "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>, Greg Stark <stark(at)mit(dot)edu>, Erik Rijkers <er(at)xs4all(dot)nl>, Robert Haas <robertmhaas(at)gmail(dot)com>, Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com>, PostgreSQL <pgsql-hackers(at)postgresql(dot)org>, pgsql-hackers-owner(at)postgresql(dot)org
Subject: Re: \if, \elseif, \else, \endif (was Re: PSQL commands: \quit_if, \quit_unless)
Date: 2017-03-24 22:53:38
Message-ID: CADkLM=e8pN4euMZp43N8S9obJoee1xsn_vkbusHwv01R5pb93g@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

v25

- PQExpBuffer on gather_boolean_expression()
- convenience/clarity functions: ignore_slash_option(),
ignore_2_slash_options(), ignore_slash_line()
- remove inaccurate test of variable expansion in a false block
- added kitchen-sink test of parsing slash commands in a false block
- removed spurious file that shouldn't have been in v24
- removed any potential free(NULL) calls *that I introduced*, others remain
from master branch

NOT done:
- grouping all branching commands into one function - can be done in a
later patch for clarity
- combining _ef / _ev or _sf / _sv - can be done in a later patch for
clarity

On Fri, Mar 24, 2017 at 4:33 PM, Corey Huinker <corey(dot)huinker(at)gmail(dot)com>
wrote:

> On Fri, Mar 24, 2017 at 4:10 PM, Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
> wrote:
>
>>
>> Hello Corey,
>>
>> I wished for the same thing, happy to use one if it is made known to me.
>>> I pulled that pattern from somewhere else in the code, and given that the
>>> max number of args for a command is around 4, I'm not too worried about
>>> scaling.
>>>
>>
>> If there are expressions one day like pgbench, the number of arguments
>> becomes arbitrary. Have you looked at PQExpBuffer?
>
>
> I will look into it.
>
>
>>
>>>> There seems to be pattern repetition for _ev _ef and _sf _sv. Would it
>>>> make sense to create a function instead of keeping the initial
>>>> copy-paste?
>>>>
>>>
>>> Yes, and a few things like that, but I wanted this patch to keep as much
>>> code as-is as possible.
>>>
>>
>> If you put the generic function at the same place, one would be more or
>> less kept and the other would be just removed?
>
>
>> "git diff --patience -w" gives a rather convenient output for looking at
>> the patch.
>
>
> Good to know about that option.
>
> As for a function for digested ignored slash options, it seems like I can
> disregard the true/false value of the "semicolon" parameter. Is that
> correct?
>
>
>> I would suggest to put together all if-related backslash command, so that
>>>> the stack management is all in one function instead of 4.
>>>>
>>>
>>> I recognize the urge to group them together, but would there be any
>>> actual
>>> code sharing between them? Wouldn't I be either re-checking the string
>>> "cmd" again, or otherwise setting an enum that I immediately re-check
>>> inside the all_branching_commands() function?
>>>
>>
>> I do not see that as a significant issue, especially compared to the
>> benefit of having the automaton transition management in a single place.
>
>
> I'm still struggling to see how this would add any clarity to the code
> beyond what I can achieve by clustering the exec_command_(if/elif/else/endif)
> near one another.
>
>
>
>

Attachment Content-Type Size
0001.if_endif.v25.patch application/octet-stream 98.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David Steele 2017-03-24 23:13:21 Re: increasing the default WAL segment size
Previous Message Thomas Munro 2017-03-24 21:39:10 Re: delta relations in AFTER triggers