\if, \elseif, \else, \endif (was Re: 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 <expr>, \elseif <expr>, \else, \endif, where
<expr> will be naively evaluated via ParseVariableBool().\ifs and \endifs must be in the same "file" (each MainLoop will start a
new if-stack). This is partly for sanity (you can see the pairings unless
the programmer is off in \gset meta-land), partly for ease of design (data
structures live in MainLoop), but mostly because it would an absolute
requirement if we ever got around to doing \while.I hope to have something ready for the next commitfest.
As for the fate of \quit_if, I can see it both ways. On the one hand,
it's super-simple, already written, and handy.On the other hand, it's easily replaced by
\if <expr>
\q
\endifSo I'll leave that as a separate reviewable patch.
As for loops, I don't think anyone was pushing for implementing \while
now, only to have a decision about what it would look like and how it would
work. There's a whole lot of recording infrastructure (the input could be a
stream) needed to make it happen. Moreover, I think \gexec scratched a
lot of the itches that would have been solved via a psql looping structure.
And here's the patch. I've changed the subject line and will be submitting
a new entry to the commitfest.
Attachments:
0001.if_endif.difftext/plain; charset=US-ASCII; name=0001.if_endif.diffDownload+456-24
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 I do not like much, though. Given the syntax
and behavioral proximity with cpp, I suggest that "elif" would be a better
choice.
Documentation: I do not think that the systematic test-like example in the
documentation is relevant, a better example should be shown. The list of
what is considered true or false should be told explicitely, not end with
"etc" that is everyone to guess.
Tests: On principle they should include echos in all non executed branches
to reassure that they are indeed not executed.
Also, no error cases are tested. They should be. Maybe it is not very easy
to test some cases which expect to make psql generate errors, but I feel
they are absolutely necessary for such a feature.
1: unrecognized value "whatever" for "\if"; assuming "on"
I do not think that the script should continue with such an assumption.
2: encountered \else after \else ... "# ERROR BEFORE"
Even with ON_ERROR_STOP activated the execution continues.
3: no \endif
Error reported, but it does not stop even with ON_ERROR_STOP.
4: include with bad nesting...
Again, even with ON_ERROR_STOP, the execution continues...
Code:
- if (status != PSQL_CMD_ERROR)
+ if ((status != PSQL_CMD_ERROR) && psqlscan_branch_active(scan_state))Why the added parenthesis?
case ...: case ...:
The project rules are to add explicit /* PASSTHROUGH */ comment.
There are spaces at the end of one line in a comment about
psqlscan_branch_end_state.
There are multiple "TODO" comments in the file... Is it done or work in
progress?
Usually in pg comments about a function do not repeat the function name.
For very short comment, they are /* inlined */ on one line. Use pg comment
style.
--
Fabien.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
A few comments:
Argh, better with the attachements:-(
--
Fabien.
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, but I think elseif
was the consensus. It's easy enough to change.
Documentation: I do not think that the systematic test-like example in the
documentation is relevant, a better example should be shown. The list of
what is considered true or false should be told explicitely, not end with
"etc" that is everyone to guess.
It was copied from the regression test. I knew there would be follow-up
suggestions for what should be shown.
Tests: On principle they should include echos in all non executed branches
to reassure that they are indeed not executed.
Will do.
Also, no error cases are tested. They should be. Maybe it is not very easy
to test some cases which expect to make psql generate errors, but I feel
they are absolutely necessary for such a feature.
Will do.
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 expressions. So I guess we start that now. What subset of true-ish
values do you think we should support? If we think that real expressions
are possible soon, we could only allow 'true' and 'false' for now, but if
we expect that expressions might not make it into v10, then perhaps we
should support the same text values that coerce to booleans on the server
side.
2: encountered \else after \else ... "# ERROR BEFORE"
Even with ON_ERROR_STOP activated the execution continues.
3: no \endif
Error reported, but it does not stop even with ON_ERROR_STOP.
4: include with bad nesting...
Again, even with ON_ERROR_STOP, the execution continues...
All valid issues. Will add those to the regression as well (with
ON_ERROR_STOP disabled, obviously).
Code:
- if (status != PSQL_CMD_ERROR) + if ((status != PSQL_CMD_ERROR) && psqlscan_branch_active(scan_st ate))Why the added parenthesis?
Will fix.
case ...: case ...:
The project rules are to add explicit /* PASSTHROUGH */ comment.
Will do.
There are spaces at the end of one line in a comment about
psqlscan_branch_end_state.There are multiple "TODO" comments in the file... Is it done or work in
progress?
I forgot to remove them. But it would be wildly optimistic of me to think
there would be no more work for me after the first patch submission.
Usually in pg comments about a function do not repeat the function name.
For very short comment, they are /* inlined */ on one line. Use pg comment
style.
In that case, I was copying the style found in other functions psqlscan.l -
I'm happy to remove it.
On Mon, Jan 23, 2017 at 4:12 PM, Corey Huinker <corey.huinker@gmail.com>
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 be a better
choice.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.
Documentation: I do not think that the systematic test-like example in
the documentation is relevant, a better example should be shown. The list
of what is considered true or false should be told explicitely, not end
with "etc" that is everyone to guess.It was copied from the regression test. I knew there would be follow-up
suggestions for what should be shown.
Work on this is pending discussion of what true/false values we should
allow, and if values outside of those is an error.
case ...: case ...:
The project rules are to add explicit /* PASSTHROUGH */ comment.
Will do.
I went looking for other examples of explicit /* PASSTHROUGH */ comments
and could find none. Could you explain what it is you want me to fix with
the switch statements?
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 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.
Else-if variants from different languages include:
VB: If ElseIf Else End If (with optional Then)
PHP: if {} elseif {} else {}
Tcl: if {} elseif {} else {}
PL/pgSQL: IF ... THEN ... ELSIF ... ELSE ... END IF;
Perl: if {} elsif {} else {}
Ruby: if elsif else end
CPP: #if #elif #else #endif
Python: if : elif: else:
bash: if [] then ... elif ... else ... fi
The closest case is CPP with its line-oriented #-prefix syntax, and I
still think that we should do it like that.
I went looking for other examples of explicit /* PASSTHROUGH */ comments
and could find none. Could you explain what it is you want me to fix with
the switch statements?
Sorry, I got it wrong. The comment (which is FALLTHROUGH or FALL THROUGH,
not PASSTHROUGH) is required if there is specific code in a case and the
case is expected to continue with executing the next case code as well.
This is quite rare, and it is not the case for your code, which just has
some comments in one case.
--
Fabien
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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 expressions. So I guess we start that now. What
subset of true-ish values do you think we should support? If we think
that real expressions are possible soon, we could only allow 'true' and
'false' for now, but if we expect that expressions might not make it
into v10, then perhaps we should support the same text values that
coerce to booleans on the server side.
Hmmm. I would text value that coerce to true? I would also accept non-zero
integers (eg SELECT 1::BOOL; -- t).
I would suggest to assume false on everything else, and/or maybe to ignore
the whole if/endif section in such cases.
All valid issues. Will add those to the regression as well (with
ON_ERROR_STOP disabled, obviously).
ISTM that with TAP test you can check for error returns, so maybe this can
be done.
--
Fabien.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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 should do it like that.
Given that the pl/pgsql syntax has a space between "end" and "if", I have
to agree. It's easy enough to change back if others can make a convincing
argument for something else.
On Tue, Jan 24, 2017 at 1:15 AM, Fabien COELHO <coelho@cri.ensmp.fr> 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 already
broke out argument reading to it's own function, knowing that it'd be the
stub for expressions. So I guess we start that now. What subset of true-ish
values do you think we should support? If we think that real expressions
are possible soon, we could only allow 'true' and 'false' for now, but if
we expect that expressions might not make it into v10, then perhaps we
should support the same text values that coerce to booleans on the server
side.Hmmm. I would text value that coerce to true? I would also accept non-zero
integers (eg SELECT 1::BOOL; -- t).
The docs (doc/src/sgml/datatype.sgml) list TRUE 't' 'true' 'y' 'yes' 'on'
'1' vs FALSE 'f' 'false' 'n' 'no' 'off' '0'
However, src/test/regress/expected/boolean.out shows that there's some
flexiblity there with spaces, even before you inspect parse_bool_with_len()
in src/backend/utils/adt/bool.c.
If we could bring src/backend/utils/adt/bool.c across the server/client
wall, I would just use parse_bool_with_len as-is.
As it is, given that whatever I do is temporary until real expressions come
into place, that wouldn't be a terrible amount of code copying, and we'd
still have a proto-expression that probably isn't going to conflict with
whatever expression syntax we do chose. Having said that, if anyone can see
ANY reason that some subset of the existing bool-friendly strings would
cause a problem with the expression syntax we do hope to use, speak now and
we can restrict the accepted values.
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.
ISTM that with TAP test you can check for error returns, so maybe this can
be done.
I'll have to educate myself on TAP tests.
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 TRUE or FALSE, then fine. If it
is anything else, then an error is raised (error message shown), which
should also stop the script on "ON_ERROR_STOP", and if not the script
continues with assuming the value was FALSE.
--
Fabien.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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 https://commitfest.postgresql.org/12/799/
in the ongoing CF already changes ParseVariableBool()
to not assume that unrecognizable values should be set to
"on".
There's also the fact that ParseVariableBool() in HEAD assumes
that an empty value is valid and true, which I think leads to this
inconsistency in the current patch:
\set empty
\if :empty
select 1 as result \gset
\else
select 2 as result \gset
\endif
\echo 'result is' :result
produces: result is 1 (so an empty string evaluates to true)
Yet this sequence:
\if
select 1 as result \gset
\else
select 2 as result \gset
\endif
\echo 'result is' :result
produces: result is 2 (so an empty \if evaluates to false)
The equivalence between empty value and true in
ParseVariableBool() is also suppressed in the above-mentioned
patch.
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 behavior of ParseVariableBool()
in HEAD at the moment, as it's likely to change.
Best regards,
--
Daniel Vérité
PostgreSQL-powered mailer: http://www.manitou-mail.org
Twitter: @DanielVerite
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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 behavior of ParseVariableBool()
in HEAD at the moment, as it's likely to change.
I'd like to keep in sync with ParseVariableBoolean(), but
Also, Fabien has made a good case for invalid parsed values being an
ON_ERROR_STOP-able error, and not defaulted to either true or false.
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
"assuming " warning if the strict function failed?
On Tue, Jan 24, 2017 at 1:25 PM, Corey Huinker <corey.huinker@gmail.com>
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
"assuming " warning if the strict function failed?
Nevermind. Looking at the v7 patch I see that it does everything I need and
more. I should have looked first.
"Daniel Verite" <daniel@manitou-mail.org> 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 that we
discuss /if issues relatively to the behavior of ParseVariableBool()
in HEAD at the moment, as it's likely to change.
AFAIK we do have consensus on changing its behavior to disallow
assignment of invalid values. It's just a matter of getting the
patch to be stylistically nice.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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 of that one. I'm happy to continue with the stop-gap in place.
Changes made:
- \elseif is now \elif
- Invalid boolean values now return an error
- ON_ERROR_STOP is respected in all errors raided by \if, \elsif, \else,
\endif commands.
- Documentation gives a more real-world example of usage.
- Documentation gives a more explicit list of valid boolean values
- Regression tests for out-of-place \endif, \else, and \endif
- Regression test for invalid boolean values
- Removal of debug detritus.
Changes not(yet) made:
- No TAP test for errors respecting ON_ERROR_STOP
- function comments in psqlscan.l follow the style found in other comments
there, which goes counter to global style.
On Tue, Jan 24, 2017 at 3:58 AM, Fabien COELHO <coelho@cri.ensmp.fr> wrote:
Show quoted text
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 TRUE or FALSE, then fine. If it
is anything else, then an error is raised (error message shown), which
should also stop the script on "ON_ERROR_STOP", and if not the script
continues with assuming the value was FALSE.--
Fabien.
Attachments:
0001.if_endif.v2.difftext/plain; charset=US-ASCII; name=0001.if_endif.v2.diffDownload+579-24
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
The 2nd reference to :var has a quoting hazard, but since it's within
an "\if false" branch, at a glance it seems like this script might work.
In fact it barfs with:
psql:script.sql:0: found EOF before closing \endif(s)
AFAICS what happens is that :var gets injected and starts a
runaway string, so that as far as the parser is concerned
the \else ..\endif block slips into the untaken branch, as a part of
that unfinished string.
This contrasts with line 2: -- select :var
where the reference to :var is inoffensive.
To avoid that kind of trouble, would it make sense not to expand
variables in untaken branches?
Best regards,
--
Daniel Vérité
PostgreSQL-powered mailer: http://www.manitou-mail.org
Twitter: @DanielVerite
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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;
\endifTo avoid that kind of trouble, would it make sense not to expand
variables in untaken branches?
Hmmm. This case is somehow contrived (for a string, :'var' could be used,
in which case escaping would avoid the hazard), but I think that what you
suggest is a better behavior, if easy to implement.
--
Fabien.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Jan 26, 2017 at 3:55 PM, Fabien COELHO <coelho@cri.ensmp.fr> 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'
-- select :var;
\if false
select :var ;
\else
select 1;
\endifTo avoid that kind of trouble, would it make sense not to expand
variables in untaken branches?Hmmm. This case is somehow contrived (for a string, :'var' could be used,
in which case escaping would avoid the hazard), but I think that what you
suggest is a better behavior, if easy to implement.--
Fabien.
Good question, Daniel. Variable expansion seems to be done via
psql_get_variable which is invoked via callback, which means that I might
have to move branch_block_active into PsqlSettings. That's slightly
different because the existing boolean is scoped with MainLoop(), but
there's no way to get to a new MainLoop unless you're in an executing
branch, and no way to legally exit a MainLoop with an unbalanced if-endif
state. In short, I think it's better behavior. It does prevent someone from
setting a variable to '\endif' and expecting that to work, like this:
select case
when random() < 0.5 then '\endif'
else E'\else\n\echo fooled you\n\endif'
end as contrived_metaprogramming
\gset
\if false
:contrived_metaprogramming
I'm sure someone will argue that preventing that is a good thing. Unless
someone sees a good reason not to move that PsqlSettings, I'll make that
change.
On Thu, Jan 26, 2017 at 4:06 PM, Corey Huinker <corey.huinker@gmail.com>
wrote:
On Thu, Jan 26, 2017 at 3:55 PM, Fabien COELHO <coelho@cri.ensmp.fr>
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'
-- select :var;
\if false
select :var ;
\else
select 1;
\endifTo avoid that kind of trouble, would it make sense not to expand
variables in untaken branches?Hmmm. This case is somehow contrived (for a string, :'var' could be used,
in which case escaping would avoid the hazard), but I think that what you
suggest is a better behavior, if easy to implement.--
Fabien.Good question, Daniel. Variable expansion seems to be done via
psql_get_variable which is invoked via callback, which means that I might
have to move branch_block_active into PsqlSettings. That's slightly
different because the existing boolean is scoped with MainLoop(), but
there's no way to get to a new MainLoop unless you're in an executing
branch, and no way to legally exit a MainLoop with an unbalanced if-endif
state. In short, I think it's better behavior. It does prevent someone from
setting a variable to '\endif' and expecting that to work, like this:select case
when random() < 0.5 then '\endif'
else E'\else\n\echo fooled you\n\endif'
end as contrived_metaprogramming
\gset\if false
:contrived_metaprogrammingI'm sure someone will argue that preventing that is a good thing. Unless
someone sees a good reason not to move that PsqlSettings, I'll make that
change.
And here it is
Attachments:
0001.if_endif.v3.difftext/plain; charset=US-ASCII; name=0001.if_endif.v3.diffDownload+618-24
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 index only scan on a large table, so maybe it is a better way to do it.
It seems strange that there is no better way to do that in a relational
tool, the relational model being an extension of set theory... and there
is no easy way (?) to check whether a set is empty.
"""If an EOF is reached on the main file or an
<command>\include</command>-ed file before all
<command>\if</command>-<command>\endif</command> are matched, then psql
will raise an error."""
In sentence above: "before all" -> "before all local"? "then" -> ""?
"other options booleans" -> "other booleans of options"? or "options'
booleans" maybe, but that is for people?
"unabigous" -> "unambiguous"
I think that the three paragraph explanation about non evaluation could be
factor into one, maybe something like: """Lines within false branches are
not evaluated in any way: queries are not sent to the server, non
conditional commands are not evaluated but bluntly ignored, nested if
expressions in such branches are also not evaluated but are tallied to
check for nesting."""
I would suggest to merge elif/else constraints by describing what is
expected rather than what is not expected. """An \if command may contain
any number of \elif clauses and may end with one \else clause""".
## CODE
In "read_boolean_expression":
+ if (value)
"if (value != NULL)" is usually used, I think.
+ if (value == NULL)
+ return false; /* not set -> assume "off" */This is dead code, because value has been checked to be non NULL a few
lines above.
+ (pg_strncasecmp(value, "on", (len > 2 ? len : 2)) == 0)
+ (pg_strncasecmp(value, "off", (len > 2 ? len : 2)) == 0)Hmmm, not easy to parse. Maybe it deserves a comment?
"check at least two chars to distinguish on & off"
",action" -> ", action" (space after commas).
The "result" is not set on errors, but maybe it should be set to false
anyway and explicitely, instead of relying on some prior initialization?
Or document that the result is not set on errors.
if command:
if (is active) {
success = ...
if (success) {
...
}
}
if (success) {
...
}
The second test on success may not rely on the value set above. That looks
very strange. ISTM that the state should be pushed whether parsing
succeeded or not. Moreover, it results in:
\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.
On "else" when in state ignored, ISTM that it should remain in state
ignore, not switch to else-false.
Comment about "IFSTATE_FALSE" talks about the state being true, maybe a
copy-paste error?
In comments: "... variables the branch" -> "variables if the branch"
The "if_endifs_balanced" variable is not very useful. Maybe just the test
and error reporting in the right place:
if (... && !psqlscan_branch_empty(scan_state))
psql_error("found EOF before closing \\endif(s)\n");
+
#endif /* MAINLOOP_H */
-
/*
* Main processing loop for reading lines of input
* and sending them to the backend.
Do not add/remove empty lines in places unrelated to the patch?
History saving: I'm wondering whether all read line should be put into
history, whether executed or not.
Is it possible to make some of the added functions static? If so, do it.
I checked that it does stop on errors with -v ON_ERROR_STOP=1. However I
would be more at ease if this was tested somewhere.
--
Fabien.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
