Re: [HACKERS] \if, \elseif, \else, \endif (was Re: PSQL commands:\quit_if, \quit_unless) - Mailing list pgsql-hackers
| From | Corey Huinker |
|---|---|
| Subject | Re: [HACKERS] \if, \elseif, \else, \endif (was Re: PSQL commands:\quit_if, \quit_unless) |
| Date | |
| Msg-id | CADkLM=fHrBW4Z+ixPD35Ad+4oKG1KX0UK7xDY+0G9gdNnDQS8A@mail.gmail.com Whole thread Raw |
| In response to | Re: [HACKERS] \if, \elseif, \else, \endif (was Re: PSQL commands: \quit_if, \quit_unless) (Tom Lane <tgl@sss.pgh.pa.us>) |
| Responses |
Re: [HACKERS] \if, \elseif, \else, \endif (was Re: PSQL commands:\quit_if, \quit_unless)
Re: [HACKERS] \if, \elseif, \else, \endif (was Re: PSQL commands:\quit_if, \quit_unless) |
| List | pgsql-hackers |
Attached is the latest work. Not everything is done yet. I post it because the next step is likely to be "tedious" as Tom put it, and if there's a way out of it, I want to avoid it.
What is done:
- all changes here built off the v22 patch
- any function which had scan_state and cond_stack passed in now only has scan_state, and cond_stack is extracted from the cb_passthrough pointer.
- ConditonalStack is now only explictly passed to get_prompt ... which doesn't have scan state
- Conditional commands no longer reset scan state, nor do they clear the query buffer
- boolean expressions consume all options, but only evaluate variables and backticks in situations where those would be active
- invalid boolean arguments are treated as false
- contextually wrong \else, \endif, \elif are still errors
What is not done:
- TAP tests are not converted to regular regression test(s)
- skipped slash commands still consume the rest of the line
That last part is big, to quote Tom:
* More generally, I do not think that the approach of having exec_commandsimply fall out immediately when in a false branch is going to work,because it ignores the fact that different backslash commands havedifferent argument parsing rules. Some will eat the rest of the line andsome won't. I'm afraid that it might be necessary to remove that codeblock and add a test to every single backslash command that decideswhether to actually perform its action after it's consumed its arguments.That would be tedious :-(. But as it stands, backslash commands will getparsed differently (ie with potentially-different ending points) dependingon whether they're in a live branch or not, and that seems just way tooerror-prone to be allowed to stand.
If that's what needs to be done, does it make sense to first commit a pre-patch that encapsulates each command family ( \c and \connect are a family, all \d* commands are one family) into its own static function? It would make the follow-up patch to if-endif cleaner and easier to review.
On Thu, Mar 16, 2017 at 5:14 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Corey Huinker <corey.huinker@gmail.com> writes:
> Ok, I've got some time now and I'm starting to dig into this. I'd like to
> restate what I *think* my feedback is, in case I missed or misunderstood
> something.
> ...
> 3. Change command scans to scan the whole boolean expression, not just
> OT_NORMAL.
> There's a couple ways to go about this. My gut reaction is to create a new
> scan type OT_BOOL_EXPR, which for the time being is the same as
> OT_WHOLE_LINE, but could one day be something different.
OT_WHOLE_LINE is not what you want because that results in verbatim
copying, without variable expansion or anything. My vote would be to
repeatedly do OT_NORMAL until you get a NULL, thereby consuming as
many regular arguments as the backslash command has. (After which,
if it wasn't exactly one argument, complain, for the moment. But this
leaves the door open for something like "\if :foo = :bar".) Note that
this implies that "\if some-expression \someothercommand" will be allowed,
but I think that's fine, as I see no reason to allow backslashes in
whatever if-expression syntax we invent later. OT_WHOLE_LINE is a bit of
a bastard child and I'd just as soon not define it as being the lexing
behavior of any new commands.
> 5. Allow contextually-correct invalid boolean expressions to map to false.
> Out-of-context \endif, \else, and \elif commands remain as errors to be
> ignored, invalid expressions in an \if or legallyl-placed \elif are just
> treated as false.
WFM.
regards, tom lane
Attachment
pgsql-hackers by date: