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

On Sat, Feb 11, 2017 at 3:48 PM, Fabien COELHO wrote: > > Just realized that '?' means "unknown transactional status" in %x. That >> might cause confusion if a person had a prompt of %x%R. Is that enough >> reason to pick a different cue? >> > > Argh. > > "\?\.?[tfz]" seems

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

Just realized that '?' means "unknown transactional status" in %x. That might cause confusion if a person had a prompt of %x%R. Is that enough reason to pick a different cue? Argh. "\?\.?[tfz]" seems distinctive enough. Note that %R uses "'=-*^!$( and %x uses *!?, which means that they

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

On Sat, Feb 11, 2017 at 2:43 AM, Fabien COELHO wrote: > > Ok, so that's not just PROMPT_READY, that's every prompt...which might be >> ok. ? is a great optional cue, and you're thinking on 2 levels deep, 2nd >> level always being '.'? >> > > Yep. The idea is to keep it

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

Ok, so that's not just PROMPT_READY, that's every prompt...which might be ok. ? is a great optional cue, and you're thinking on 2 levels deep, 2nd level always being '.'? Yep. The idea is to keep it short, but to still have something to say "there are more levels" in the stack, hence the one

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

> > calvin=> \if true > calvin?t=> SELECT 1 + > calvin?t-> 2; > 3 > calvin?t=> \if true > calvin?t=> \echo hello > hello > calvin?t=> \endif > calvin?t=> \else > calvin?z=> \echo ignored > calvin?t=> \endif > calvin=> > Ok, so that's not just PROMPT_READY, that's

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

Hello, I'm looking forward to the doc update. My 0.02€ about prompting: Given that there is no more barking, then having some prompt indication that the code is inside a conditional branch becomes more important, so ISTM that there should be some plan to add it. Yeah, prompting just got

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

> > Shouldn't there be some documentation changes to reflect the behavior on > errors? A precise paragraph about that would be welcome, IMHO. > Oddly enough, the documentation I wrote hadn't addressed invalid booleans, only the error messages did that. The new behavior certainly warrants a

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

Hello Corey, Changes in this patch: - invalid boolean expression on \if or \elif is treated as if the script had a bad \command, so it either stops the script (ON_ERROR_STOP, script mode), or just gives the ParseVariableBool error and continues. - All interactive "barks" removed except

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

On Thu, Feb 9, 2017 at 4:43 PM, Erik Rijkers wrote: > On 2017-02-09 22:15, Tom Lane wrote: > >> Corey Huinker writes: >> > > The feature now ( at patch v10) lets you break off with Ctrl-C anywhere. > I like it now much more. > > The main thing I still

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

On 2017-02-09 22:15, Tom Lane wrote: Corey Huinker writes: The feature now ( at patch v10) lets you break off with Ctrl-C anywhere. I like it now much more. The main thing I still dislike somewhat about the patch is the verbose output. To be honest I would

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

On Thu, Feb 9, 2017 at 4:15 PM, Tom Lane wrote: > Basically, I think you need to start removing complexity (in the sense of > special cases), not adding more. I think Robert was saying the same > thing, though possibly I shouldn't put words in his mouth. Yeah, I was

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

Corey Huinker writes: > On Thu, Feb 9, 2017 at 3:13 PM, Tom Lane wrote: >> IMO, an erroneous backslash command should have no effect, period. > One way around this is to make the small change: commands with invalid > expressions are ignored in

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

On Thu, Feb 9, 2017 at 3:13 PM, Tom Lane wrote: > Robert Haas writes: > > I (still) think this is a bad design. Even if you've got all the > > messages just right as things stand today, some new feature that comes > > along in the future can change

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

Robert Haas writes: > I (still) think this is a bad design. Even if you've got all the > messages just right as things stand today, some new feature that comes > along in the future can change things so that they're not right any > more, and nobody's going to relish

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

On Mon, Feb 6, 2017 at 5:49 PM, Corey Huinker wrote: > I suppressed the endif-balance checking in cases where we're in an > already-failed situation. > In cases where there was a boolean parsing failure, and ON_ERROR_STOP is on, > the error message no longer speak of a

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

This was my previous understanding of ON_ERROR_STOP. Somewhere in the course of developing this patch I lost that. Glad to have it back. The only changes I made were to invalid booleans on if/elif, and the final branch balancing check won't set status to EXIT_USER unless it's non-interactive

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

> > > It seems that ON_ERROR_STOP is mostly ignored by design when in > interactive mode, probably because it is nicer not to disconnect the user > who is actually typing things on a terminal. > > """ > ON_ERROR_STOP > > By default, command processing continues after an error. When this >

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

Hello Corey, In cases where there was a boolean parsing failure, and ON_ERROR_STOP is on, the error message no longer speak of a future which the session does not have. I could left the ParseVariableBool() message as the only one, but wasn't sure that that was enough of an error message on its

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

On Mon, Feb 6, 2017 at 3:43 PM, Corey Huinker wrote: > I did some more tests. I found a subtlety that I missed before: when >> running under ON_ERROR_STOP, messages are not fully consistent: >> >> sh> cat test.sql >> \set ON_ERROR_STOP on >> \if error >>\echo NO

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

> > I did some more tests. I found a subtlety that I missed before: when > running under ON_ERROR_STOP, messages are not fully consistent: > > sh> cat test.sql > \set ON_ERROR_STOP on > \if error >\echo NO > \endif > \echo NO > > sh> ./psql < test.sql > SET > # ok > unrecognized value

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

Consolidated Fabien's TAP test additions with v7, in case anyone else wants to be reviewing. Patch applies (with "patch"), make check ok, psql tap test ok. I did some more tests. I found a subtlety that I missed before: when running under ON_ERROR_STOP, messages are not fully consistent:

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

On Mon, Feb 6, 2017 at 2:32 PM, Fabien COELHO wrote: > > Do you think the TAP tests would benefit from having the input described >> in a q(...) block rather than a string? >> > > My 0.02€: Not really, so I would not bother. It breaks perl indentation > and logic for a

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

Do you think the TAP tests would benefit from having the input described in a q(...) block rather than a string? My 0.02€: Not really, so I would not bother. It breaks perl indentation and logic for a limited benefit. Maybe add comments if you feel that a test case is unclear. -- Fabien.

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

On Mon, Feb 6, 2017 at 11:21 AM, Corey Huinker wrote: > >> Find attached a small patch to improve tap tests, which also checks that >>> psql really exited by checking that nothing is printed afterwards. >>> >> >> > Do you think the TAP tests would benefit from having the

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

> > > Find attached a small patch to improve tap tests, which also checks that >> psql really exited by checking that nothing is printed afterwards. >> > > Do you think the TAP tests would benefit from having the input described in a q(...) block rather than a string? q(\if false \echo a \elif

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

Find attached a small patch to improve tap tests, which also checks that psql really exited by checking that nothing is printed afterwards. . It is better with the attachement. -- Fabien.diff --git a/src/bin/psql/t/001_if.pl b/src/bin/psql/t/001_if.pl index 68c9b15..a703cab 100644 ---

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

# elif error "\\if false\n\\elif error\n\\endif\n" # ignore commands on error (stdout must be empty) "\\if error\n\\echo NO\n\\else\n\\echo NO\n\\endif\n" Those are already in the regression (around line 2763 of expected/psql.out), are you saying we should have them in TAP as well?

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

> > Hmmm. ISTM that control-c must at least reset the stack, otherwise their > is not easy way to get out. What can be left to another patch is doing a > control-C for contents and then another one for the stack when there is no > content. > And so it shall be. > - put Fabien's tap test in

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

Hello Corey, The check I was suggesting on whether Ctrl+C has been pressed on an empty line seems harder to implement, because get_interactive() just calls readline() or fgets(), which block to return when a whole line is ready. AFAICS psql can't know what was the edit-in-progress when these

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

On Sat, Feb 4, 2017 at 11:53 AM, Corey Huinker wrote: > The check I was suggesting on whether Ctrl+C has been pressed >> on an empty line seems harder to implement, because get_interactive() >> just calls readline() or fgets(), which block to return when a whole >> line

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

> > The check I was suggesting on whether Ctrl+C has been pressed > on an empty line seems harder to implement, because get_interactive() > just calls readline() or fgets(), which block to return when a whole > line is ready. AFAICS psql can't know what was the edit-in-progress > when these

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

> > I noticed that the "barking" is conditional to "success". ISTM that it > should always "bark" in interactive mode, whether success or not. > "success" in those cases means "the expression was a valid boolean", and non-success cases (should) result in an error being printed regardless of

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

Corey Huinker wrote: [about Ctrl-C] > That does seem to be the consensus desired behavior. I'm just not sure > where to handle that. The var "cancel_pressed" shows up in a lot of places. > Advice? Probably you don't need to care about cancel_pressed, and the /if stack could be unwound

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

A few comments about v5. New patch. Patch applies (with patch, I gave up on "git apply"). Make check ok. Psql tap test ok. Highlights: - Interactive barking on branching state changes, commands typed while in inactive state I noticed that the "barking" is conditional to "success". ISTM

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

On Fri, Feb 3, 2017 at 7:42 PM, Jim Nasby wrote: > Since the current consensus is to be very verbose about \if, this is > obviously a non-issue. Maybe worth adding a 'I' case to %R, but no big deal > if that doesn't happen. I think we left the door open to a separate

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

On 2/2/17 4:39 PM, Corey Huinker wrote: On Wed, Feb 1, 2017 at 4:58 PM, Jim Nasby > wrote: I think the issue here is that the original case for this is a user accidentally getting into an \if and then having no clue what's

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

On Fri, Feb 3, 2017 at 4:24 PM, Corey Huinker wrote: > That might be what we end up doing. I'm willing to see how unwieldy it gets > before rolling back to "endif: peace out". All I'm saying is, give peace a chance. :-) > The state logic has stuff to do anyway, so for

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

On Fri, Feb 3, 2017 at 3:49 PM, Robert Haas wrote: > On Fri, Feb 3, 2017 at 11:08 AM, Corey Huinker > wrote: > > I could bulk up the error message on if/elif like such: > > > > if: true, executing commands. > > if: false, ignoring commands until

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

On Fri, Feb 3, 2017 at 11:08 AM, Corey Huinker wrote: > I could bulk up the error message on if/elif like such: > > if: true, executing commands. > if: false, ignoring commands until next \else, \elif, or \endif. > if: error, ignoring all commands until next \endif >

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

On Fri, Feb 3, 2017 at 12:20 PM, Daniel Verite wrote: > If we add the functionality that Ctrl+C also exits from branches, > we could do it like the shell does Ctrl+D for logout, that is it > logs out only if the input buffer is empty, otherwise it does > the other

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

Corey Huinker wrote: > I meant in the specific psql-context, does it do anything other > than (attempt to) terminate sent-but-not-received SQL queries? It cancels the current edit in the query buffer, including the case when it spans multiple lines. If we add the functionality that

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

Corey Huinker writes: > Well played (again). That one ranks up there with "and don't call me > Shirley." I meant in the specific psql-context, does it do anything other > than (attempt to) terminate sent-but-not-received SQL queries? It also flushes the input buffer. I

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

> > Can anyone think of a reason why Ctrl-C would be a bad idea? If not I'll >> start looking into it, as I'm not presently aware of what it is used for. >> > > Not me. > > Wikipedia, which holds all the knowledge in the universe, says: "In many > command-line interface environments, control-C is

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

I could bulk up the error message on if/elif like such: [...] Looks ok to me. Can anyone think of a reason why Ctrl-C would be a bad idea? If not I'll start looking into it, as I'm not presently aware of what it is used for. Not me. Wikipedia, which holds all the knowledge in the

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

On Fri, Feb 3, 2017 at 7:24 AM, Fabien COELHO wrote: > > Hello Erik, > > Still, it would be an improvement to be able to break out of an inactive >> \if-branch with Ctrl-C. >> > > Yes. > > '\endif' is too long to type, /and/ you have to know it. >> > > Yep. \if is shorter

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

2. Inside an \if block \q should be given precedence and cause a direct exit of psql (or at the very least exit the if block(s)), as in regular SQL statements (compare: 'select * from t \q' which will immediately exit psql -- this is good. ) One use case if to be able to write "\if ... \q

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

Hello Erik, Still, it would be an improvement to be able to break out of an inactive \if-branch with Ctrl-C. Yes. '\endif' is too long to type, /and/ you have to know it. Yep. \if is shorter but has been rejected. Ctrl-C should be the way out. 2. Inside an \if block \q should be given

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

On 2017-02-03 08:16, Corey Huinker wrote: 0001.if_endif.v5.diff 1. Well, with this amount of interactive output it is impossible to get stuck without knowing :) This is good. Still, it would be an improvement to be able to break out of an inactive \if-branch with Ctrl-C. (I noticed that

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

On Fri, Feb 3, 2017 at 12:57 AM, Fabien COELHO wrote: > > Hello Corey, > > Glad you like the barking. I'm happy to let the prompt issue rest for now, >> we can always add it later. >> >> If we DID want it, however, I don't think it'll be hard to add a special >> prompt

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

Hello Corey, Glad you like the barking. I'm happy to let the prompt issue rest for now, we can always add it later. If we DID want it, however, I don't think it'll be hard to add a special prompt (Thinking %T or %Y because they both look like branches, heh), Ah!:-) T may stand for Tree, but

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

> > > On reflection, it seems fairly improbable to me that people would use > \if and friends interactively. They're certainly useful for scripting, > but would you really type commands that you know are going to be ignored? > I'm thinking the one use-case is where a person is debugging a

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

Corey Huinker writes: >> As it is, I've added interactive mode psql_error notifications about the >> resulting branching state of any branching commands, and any attempt to >> send non-branching commands or queries while in an inactive branch will >> generate a psql_error

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

> > > All, > As it is, I've added interactive mode psql_error notifications about the > resulting branching state of any branching commands, and any attempt to > send non-branching commands or queries while in an inactive branch will > generate a psql_error saying that the command was ignored.

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

On Wed, Feb 1, 2017 at 4:58 PM, Jim Nasby wrote: > I think the issue here is that the original case for this is a user > accidentally getting into an \if and then having no clue what's going on. > That's similar to what happens when you miss a quote or a semicolon. We >

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

Hello, What do I need to do to get TAP tests running? I misunderstood. You need to configure with "--enable-tap-tests". There is a spurious empty line added at the very end of "mainloop.h": + #endif /* MAINLOOP_H */ Not in my diff, but that's been coming and going in your diff

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

On 2/1/17 12:03 PM, Fabien COELHO wrote: I'm unsure whether it is a good idea, I like terse interfaces, but this is just an opinion. I think the issue here is that the original case for this is a user accidentally getting into an \if and then having no clue what's going on. That's similar to

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

Hello, We could just issue interactive-only warnings when: - A user types a branching condition command which sets the branch inactive - A user types a command or query when the current branch is inactive. The warnings could be specific about state, something like: psql session is now in an

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

> > > You can also run "make check" directly in the "src/bin/psql" directory. Previously, that didn't do anything, but now that I've created a TAP test it does. It doesn't, however, run the psql regress tests. But at least I can use the two commands in combination and not have to run *all* TAP

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

On Wed, Feb 1, 2017 at 8:53 AM, Corey Huinker wrote: > >> Run make check-world (as opposed to just make check ) > > > I'll give that a shot. > That was it. Tests don't run if you don't invoke them. Thanks.

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

> > However I would say yes, it should provide some feedback... This means > probably adding a new prompt substitution "%". In the worst > case, the prompt should reflect the current stack, or at least the top of > the task... > We could just issue interactive-only warnings when: - A user types a

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

Hello Corey, There is a spurious empty line added at the very end of "mainloop.h": + #endif /* MAINLOOP_H */ Not in my diff, but that's been coming and going in your diff reviews. Strange. Maybe this is linked to the warning displayed

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

Good find. I'll have to bulk up the help text. Yes. This raises a question: in interactive mode, should we give some feedback as to the result of an \if or \elif test? (see below) Obviously \if makes more sense for scripting. However I would say yes, it should provide some feedback...

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

On 2017-02-01 14:18, Corey Huinker wrote: to do to get TAP tests running? I must be missing something. Guesswork on my part: Add --enable-tap-tests option on ./configure Run make check-world (as opposed to just make check ) Erik Rijkers -- Sent via pgsql-hackers mailing list

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

> > > Add --enable-tap-tests option on ./configure > This much I had already done. > > Run make check-world (as opposed to just make check ) I'll give that a shot.

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

Run make check-world (as opposed to just make check ) I'll give that a shot. You can also run "make check" directly in the "src/bin/psql" directory. -- Fabien. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription:

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

Corey Huinker writes: > Right. What I meant was, no matter how I changed that test, I could not get > it to fail, which made me think it was not executing at all. What do I need > to do to get TAP tests running? I must be missing something. configure --enable-tap-tests,

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

> > - wrote an intentionally failing TAP test to see if "make check" executes >> it. it does not. need help. >> > > A failing test expects code 3, not 0 as written in "t/001_if.pl". With > > is($retcode,'3','Invalid \if respects ON_ERROR_STOP'); > > instead, the tests succeeds because psql

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

On Wed, Feb 1, 2017 at 6:31 AM, Erik Rijkers wrote: > On 2017-02-01 09:27, Corey Huinker wrote: > >> 0001.if_endif.v4.diff >> > > A few thoughts after a quick try: > > I dislike the ease with which one gets stuck inside an \if block, in > interactive mode. > > (for instance, in

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

On 2017-02-01 09:27, Corey Huinker wrote: 0001.if_endif.v4.diff A few thoughts after a quick try: I dislike the ease with which one gets stuck inside an \if block, in interactive mode. (for instance, in my very first session, I tried '\? \if' to see if there is more info in that

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

Hello Corey, Some comments about v4: Patch applies. "git apply" complained about a space or line somewhere, not sure why. make check ok. - rebased on post-psql hooks master Good. - took nearly every suggestion for change to documentation Indeed. Looks ok to me. - \if ERROR will

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

> > > I think ParseVariableBool is only likely to change to reject a NULL value > rather than silently interpreting it as FALSE, which is the way it is in > HEAD right now. That behavior is a leftover hack, really, and moving the > treatment of unset values upstream seems a lot cleaner. See my

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

Corey Huinker writes: > On Tue, Jan 31, 2017 at 1:04 AM, Fabien COELHO wrote: >> The ParseVariableBool function has been updated, and the new version is >> much cleaner, including all fixes that I suggested in your copy, so you can >> use it in your

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

On Tue, Jan 31, 2017 at 1:04 AM, Fabien COELHO wrote: > > This is code lifted from variable.c's ParseVariableBool(). When the other >>> patch for "psql hooks" is committed (the one that detects when the string >>> wasn't a valid boolean), this code will go away and we'll

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

This is code lifted from variable.c's ParseVariableBool(). When the other patch for "psql hooks" is committed (the one that detects when the string wasn't a valid boolean), this code will go away and we'll just use ParseVariableBool() again. The ParseVariableBool function has been updated,

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

Hello Daniel, [...] So psql is not following that model with ON_ERROR_STOP if it exits with an error when unable to evaluate an "if" expression. I'm not implying that we should necessarily adopt the shell behavior, but as these choices have certainly been made in POSIX for good reasons, we

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

Corey Huinker wrote: > > \if ERROR > > \echo X > > \else > > \echo Y > > \endif > > > > having both X & Y printed and error reported on else and endif. I think > > that an expression error should just put the stuff in ignore state. > > > > Not just false, but ignore the

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

On 1/29/17 2:35 AM, Fabien COELHO wrote: I'm wondering what pg would do on "EXISTS(SELECT 1 FROM customer)" if there are many employees. [...] I believe that the scan stops on the first row it finds, because the EXITS() clause is met. Hmmm... That is not so clear from "EXPLAIN" output: You

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

> > My point is that examples about one thing can be interpreted as example > for other things which is also done in the example, so it is better to do > everything right. > Fair enough. I'll rewrite the examples to use pk lookups. I doubt the query plan for those will change much in the future.

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

Hello, I'm wondering what pg would do on "EXISTS(SELECT 1 FROM customer)" if there are many employees. [...] I believe that the scan stops on the first row it finds, because the EXITS() clause is met. Hmmm... That is not so clear from "EXPLAIN" output: Result (cost=0.03..0.04 rows=1

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

> > > I'm wondering what pg would do on "EXISTS(SELECT 1 FROM customer)" if > there are many employees. EXPLAIN suggests a seq_scan, which seems bad. > With "(SELECT COUNT(*) FROM pgbench_accounts) <> 0" pg seems to generate an > index only scan on a large table, so maybe it is a better way to do

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

Hello Corey, And here it is About the patch v3: ## DOCUMENTATION I'm wondering what pg would do on "EXISTS(SELECT 1 FROM customer)" if there are many employees. EXPLAIN suggests a seq_scan, which seems bad. With "(SELECT COUNT(*) FROM pgbench_accounts) <> 0" pg seems to generate an

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

On Thu, Jan 26, 2017 at 4:06 PM, Corey Huinker wrote: > > > On Thu, Jan 26, 2017 at 3:55 PM, Fabien COELHO > wrote: > >> >> Hello Daniel, >> >> A comment about control flow and variables: in branches that are not >>> taken, variables are expanded

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

On Thu, Jan 26, 2017 at 3:55 PM, Fabien COELHO wrote: > > Hello Daniel, > > A comment about control flow and variables: in branches that are not >> taken, variables are expanded nonetheless, in a way that can be surprising. >> Case in point: >> >> \set var 'ab''cd' >> --

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

Hello Daniel, A comment about control flow and variables: in branches that are not taken, variables are expanded nonetheless, in a way that can be surprising. Case in point: \set var 'ab''cd' -- select :var; \if false select :var ; \else select 1; \endif To avoid that kind of trouble,

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

Corey Huinker wrote: > Revised patch A comment about control flow and variables: in branches that are not taken, variables are expanded nonetheless, in a way that can be surprising. Case in point: \set var 'ab''cd' -- select :var; \if false select :var ; \else select 1; \endif

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

Revised patch, with one caveat: It contains copy/pasted code from variable.c intended to bridge the gap until https://commitfest.postgresql. org/12/799/ (changing ParseVariableBool to detect invalid boolean-ish strings) is merged. We may want to pause full-review of this patch pending resolution

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

"Daniel Verite" writes: > ISTM that it's important that eventually ParseVariableBool() > and \if agree on what evaluates to true and false (and the > more straightforward way to achieve that is by \if calling > directly ParseVariableBool), but that it's not productive

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

On Tue, Jan 24, 2017 at 1:25 PM, Corey Huinker wrote: > This might be asking a lot, but could we make a "strict" mode for > ParseVariableBool() that returns a success boolean, and have the existing > ParseVariableBool() signature call that new function, and issue the >

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

> > ISTM that it's important that eventually ParseVariableBool() > and \if agree on what evaluates to true and false (and the > more straightforward way to achieve that is by \if calling > directly ParseVariableBool), but that it's not productive that we > discuss /if issues relatively to the

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

Corey Huinker wrote: > > > > 1: unrecognized value "whatever" for "\if"; assuming "on" > > > > I do not think that the script should continue with such an assumption. > > > > I agree, and this means we can't use ParseVariableBool() as-is The patch at

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

I would suggest to assume false on everything else, and/or maybe to ignore the whole if/endif section in such cases. +1, it also halves the number of values we have to support later. After giving it some thought, I revise a little bit my opinion: I think that if the value is evaluated to

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

On Tue, Jan 24, 2017 at 1:15 AM, Fabien COELHO wrote: > > 1: unrecognized value "whatever" for "\if"; assuming "on" >>> >>> I do not think that the script should continue with such an assumption. >>> >> >> I agree, and this means we can't use ParseVariableBool() as-is. I

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

> > > As I understood it, the negative feeback was really about sh inspiration > "if/fi", not about elif/elsif/elseif. I do not think that there was an > expressed consensus about the later. > True > The closest case is CPP with its line-oriented #-prefix syntax, and I > still think that we

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

1: unrecognized value "whatever" for "\if"; assuming "on" I do not think that the script should continue with such an assumption. I agree, and this means we can't use ParseVariableBool() as-is. I already broke out argument reading to it's own function, knowing that it'd be the stub for

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

Hello, I'm personally indifferent to elif vs elsif vs elseif, but I think elseif was the consensus. It's easy enough to change. Went with elsif to follow pl/pgsql. In reviewing the original thread it seemed that "elif" was linked to "fi" and that got some negative feedback. As I understood

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

On Mon, Jan 23, 2017 at 4:12 PM, Corey Huinker wrote: > 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

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

> > 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. > I'm personally indifferent to elif vs elsif vs elseif,

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

A few comments: Argh, better with the attachements:-( -- Fabien. if-error-1.sql Description: application/sql if-error-2.sql Description: application/sql if-error-3.sql Description: application/sql if-error-4.sql Description: application/sql -- Sent via pgsql-hackers mailing list

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

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

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

> > > Fabien is pressed for time, so I've been speaking with him out-of-thread > about how I should go about implementing it. > > The v1 patch will be \if , \elseif , \else, \endif, where > will be naively evaluated via ParseVariableBool(). > > \ifs and \endifs must be in the same "file" (each

<    1   2   3   4   5   6   7   8   9   10   >