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

From Corey Huinker
Subject Re: \if, \elseif, \else, \endif (was Re: PSQL commands:\quit_if, \quit_unless)
Date
Msg-id CADkLM=e8pN4euMZp43N8S9obJoee1xsn_vkbusHwv01R5pb93g@mail.gmail.com
Whole thread Raw
In response to Re: \if, \elseif, \else, \endif (was Re: PSQL commands:\quit_if, \quit_unless)  (Corey Huinker <corey.huinker@gmail.com>)
Responses Re: \if, \elseif, \else, \endif (was Re: PSQL commands:\quit_if, \quit_unless)  (Fabien COELHO <coelho@cri.ensmp.fr>)
List 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.huinker@gmail.com> wrote:
On Fri, Mar 24, 2017 at 4:10 PM, Fabien COELHO <coelho@cri.ensmp.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

pgsql-hackers by date:

Previous
From: Thomas Munro
Date:
Subject: Re: delta relations in AFTER triggers
Next
From: David Steele
Date:
Subject: Re: increasing the default WAL segment size