Обсуждение: psql exit status with multiple -c or -f
Our deployment script failed to notice dozens of commands failed in a transaction block and I only noticed due to keeping full logs and monitoring for error_severity>'LOG'. I would have thought that exit status would be nonzero had an error occured in an earlier script. The docs since PG9.6 say: https://www.postgresql.org/docs/9.6/app-psql.html |Exit Status | |psql returns 0 to the shell if it finished normally, 1 if a fatal error of its |own occurs (e.g. out of memory, file not found), 2 if the connection to the |server went bad and the session was not interactive, and 3 if an error occurred |in a script and the variable ON_ERROR_STOP was set. d5563d7df94488bf0ab52ac0678e8a07e5b8297e psql: Support multiple -c and -f options, and allow mixing them. If there's an error, it returns 1 (although that's not "a fatal error of its own"). |[pryzbyj@database ~]$ psql ts -c foo 2>/dev/null ; echo $? |1 But the error is "lost" if another script or -c runs without failing: |[pryzbyj@database ~]$ psql ts -txqc foo -c SELECT 2>/dev/null ; echo $? |0 Note, this works as expected: |[pryzbyj@database ~]$ psql ts -v ON_ERROR_STOP=1 -txqc foo -f /dev/null 2>/dev/null ; echo $? |1 Justin
Hello. At Mon, 17 Dec 2018 11:58:41 -0600, Justin Pryzby <pryzby@telsasoft.com> wrote in <20181217175841.GS13019@telsasoft.com> > Our deployment script failed to notice dozens of commands failed in a > transaction block and I only noticed due to keeping full logs and monitoring > for error_severity>'LOG'. I would have thought that exit status would be > nonzero had an error occured in an earlier script. > > The docs since PG9.6 say: > https://www.postgresql.org/docs/9.6/app-psql.html > |Exit Status > | > |psql returns 0 to the shell if it finished normally, 1 if a fatal error of its > |own occurs (e.g. out of memory, file not found), 2 if the connection to the > |server went bad and the session was not interactive, and 3 if an error occurred > |in a script and the variable ON_ERROR_STOP was set. > > d5563d7df94488bf0ab52ac0678e8a07e5b8297e > psql: Support multiple -c and -f options, and allow mixing them. > > If there's an error, it returns 1 (although that's not "a fatal error of its > own"). > > |[pryzbyj@database ~]$ psql ts -c foo 2>/dev/null ; echo $? > |1 > > But the error is "lost" if another script or -c runs without failing: > > |[pryzbyj@database ~]$ psql ts -txqc foo -c SELECT 2>/dev/null ; echo $? > |0 As written in the documentation[1]: > Because of this behavior, putting more than one SQL command in a > single -c string often has unexpected results. It's better to use > repeated -c commands or feed multiple commands to psql's standard > input, This seems saying that -c is equvalent to each line fed from stdin or a line in a script. The attached 0001 patch makes it clear in app-psql.html. > Note, this works as expected: > > |[pryzbyj@database ~]$ psql ts -v ON_ERROR_STOP=1 -txqc foo -f /dev/null 2>/dev/null ; echo $? > |1 The status should be 3, according to the documentation. Addition to that, the current behavior looks inconsistent (if) considering the equivalency between -c and a script line. y.txt: a) foo; select 1; b) select 1; foo; $ psql postgres -v ON_ERROR_STOP=0 -f ~/work/y.txt ; echo $? $ psql postgres -v ON_ERROR_STOP=0 < ~/work/y.txt ; echo $? All combinations return 0, and 3 when ON_ERROR_STOP=1. But, c) psql postgres -v ON_ERROR_STOP=0 -c foo -c 'select 1'; echo $? d) psql postgres -v ON_ERROR_STOP=0 -c 'select 1' -c foo; echo $? (c) returns 0 and (d) returns 1, but both return 1 when ON_ERROR_STOP=1. The attached second patch lets (c) and (d) behave the same as (a) and (b). Does it make sense? regards. [1]: https://www.postgresql.org/docs/11/app-psql.html -- Kyotaro Horiguchi NTT Open Source Software Center From 227c6014f3c50445ca1c46c4293085e6e635e91f Mon Sep 17 00:00:00 2001 From: Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp> Date: Tue, 18 Dec 2018 16:44:28 +0900 Subject: [PATCH 1/2] Add description on a behavior of psql psql's '-c command' is equivalent to a line fed from stdin or a line written in a script. It is suggested in the documentation but not explicitly described. Add a such description. --- doc/src/sgml/ref/psql-ref.sgml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml index 6c76cf2f00..4e94e8b6cf 100644 --- a/doc/src/sgml/ref/psql-ref.sgml +++ b/doc/src/sgml/ref/psql-ref.sgml @@ -147,6 +147,10 @@ psql <<EOF SELECT * FROM foo; EOF </programlisting> + + Every <option>-c</option> command is equivalent to a line in a + script. All the commands are executed ignoring command errors and psql + returns the exit status 0 unless ON_ERROR_STOP is set. </para> </listitem> </varlistentry> -- 2.16.3 From 449ee0b1003fed38ebd29fec6a19dec1cf12cf5e Mon Sep 17 00:00:00 2001 From: Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp> Date: Tue, 18 Dec 2018 16:50:11 +0900 Subject: [PATCH 2/2] Unify psql's behavior on -c with scripts Each -c command is equivalent to a line in a script, but they behaves differently on failure. Fix it. --- src/bin/psql/startup.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c index e7536a8a06..fc50301fdc 100644 --- a/src/bin/psql/startup.c +++ b/src/bin/psql/startup.c @@ -347,7 +347,7 @@ main(int argc, char *argv[]) puts(cell->val); successResult = SendQuery(cell->val) - ? EXIT_SUCCESS : EXIT_FAILURE; + ? EXIT_SUCCESS : EXIT_USER; } else if (cell->action == ACT_SINGLE_SLASH) { @@ -368,7 +368,7 @@ main(int argc, char *argv[]) cond_stack, NULL, NULL) != PSQL_CMD_ERROR - ? EXIT_SUCCESS : EXIT_FAILURE; + ? EXIT_SUCCESS : EXIT_USER; psql_scan_destroy(scan_state); conditional_stack_destroy(cond_stack); @@ -383,7 +383,11 @@ main(int argc, char *argv[]) Assert(false); } - if (successResult != EXIT_SUCCESS && pset.on_error_stop) + /* EXIT_USER shuld be forgotten if ON_ERROR_STOP is not set */ + if (successResult == EXIT_USER && !pset.on_error_stop) + successResult = EXIT_SUCCESS; + + if (successResult != EXIT_SUCCESS) break; } -- 2.16.3
út 18. 12. 2018 v 9:14 odesílatel Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> napsal:
Hello.
At Mon, 17 Dec 2018 11:58:41 -0600, Justin Pryzby <pryzby@telsasoft.com> wrote in <20181217175841.GS13019@telsasoft.com>
> Our deployment script failed to notice dozens of commands failed in a
> transaction block and I only noticed due to keeping full logs and monitoring
> for error_severity>'LOG'. I would have thought that exit status would be
> nonzero had an error occured in an earlier script.
>
> The docs since PG9.6 say:
> https://www.postgresql.org/docs/9.6/app-psql.html
> |Exit Status
> |
> |psql returns 0 to the shell if it finished normally, 1 if a fatal error of its
> |own occurs (e.g. out of memory, file not found), 2 if the connection to the
> |server went bad and the session was not interactive, and 3 if an error occurred
> |in a script and the variable ON_ERROR_STOP was set.
>
> d5563d7df94488bf0ab52ac0678e8a07e5b8297e
> psql: Support multiple -c and -f options, and allow mixing them.
>
> If there's an error, it returns 1 (although that's not "a fatal error of its
> own").
>
> |[pryzbyj@database ~]$ psql ts -c foo 2>/dev/null ; echo $?
> |1
>
> But the error is "lost" if another script or -c runs without failing:
>
> |[pryzbyj@database ~]$ psql ts -txqc foo -c SELECT 2>/dev/null ; echo $?
> |0
As written in the documentation[1]:
> Because of this behavior, putting more than one SQL command in a
> single -c string often has unexpected results. It's better to use
> repeated -c commands or feed multiple commands to psql's standard
> input,
This seems saying that -c is equvalent to each line fed from
stdin or a line in a script.
The attached 0001 patch makes it clear in app-psql.html.
> Note, this works as expected:
>
> |[pryzbyj@database ~]$ psql ts -v ON_ERROR_STOP=1 -txqc foo -f /dev/null 2>/dev/null ; echo $?
> |1
The status should be 3, according to the documentation. Addition
to that, the current behavior looks inconsistent (if) considering
the equivalency between -c and a script line.
y.txt:
a)
foo;
select 1;
b)
select 1;
foo;
$ psql postgres -v ON_ERROR_STOP=0 -f ~/work/y.txt ; echo $?
$ psql postgres -v ON_ERROR_STOP=0 < ~/work/y.txt ; echo $?
All combinations return 0, and 3 when ON_ERROR_STOP=1.
But,
c) psql postgres -v ON_ERROR_STOP=0 -c foo -c 'select 1'; echo $?
d) psql postgres -v ON_ERROR_STOP=0 -c 'select 1' -c foo; echo $?
(c) returns 0 and (d) returns 1, but both return 1 when
ON_ERROR_STOP=1.
The attached second patch lets (c) and (d) behave the same as (a)
and (b).
Does it make sense?
+1
Pavel
regards.
[1]: https://www.postgresql.org/docs/11/app-psql.html
--
Kyotaro Horiguchi
NTT Open Source Software Center
On Tue, Dec 18, 2018 at 05:13:40PM +0900, Kyotaro HORIGUCHI wrote:
> $ psql postgres -v ON_ERROR_STOP=0 -f ~/work/y.txt ; echo $?
> $ psql postgres -v ON_ERROR_STOP=0 < ~/work/y.txt ; echo $?
> c) psql postgres -v ON_ERROR_STOP=0 -c foo -c 'select 1'; echo $?
> d) psql postgres -v ON_ERROR_STOP=0 -c 'select 1' -c foo; echo $?
>
> (c) returns 0 and (d) returns 1, but both return 1 when
> ON_ERROR_STOP=1.
> The attached second patch lets (c) and (d) behave the same as (a)
> and (b).
I don't think the behavior in the single command case should be changed:
|[pryzbyj@database postgresql]$ ./src/bin/psql/psql ts -c FOO 2>/dev/null ; echo $?
|1
|[pryzbyj@database postgresql]$ patch -p1 </tmp/0002-Unify-psql-s-behavior-on-c-with-scripts.patch
|patching file src/bin/psql/startup.c
|[pryzbyj@database postgresql]$ make >/dev/null
|[pryzbyj@database postgresql]$ ./src/bin/psql/psql ts -c FOO 2>/dev/null ; echo $?
|0
Also, unpatched v11 psql returns status of last command
|[pryzbyj@database postgresql]$ psql ts -xtc 'SELECT 1' -c FOO 2>/dev/null ; echo $?
|?column? | 1
|
|1
patched psql returns 0:
|[pryzbyj@database postgresql]$ ./src/bin/psql/psql ts -xtc 'SELECT 1' -c FOO 2>/dev/null ; echo $?
|?column? | 1
|
|0
I'd prefer to see the exit status of the -f scripts (your cases a and b)
changed to 3 if the last command failed. I realized that's not what the
documentation says so possibly not backpatchable.
|3 if an error occurred in a script and the variable ON_ERROR_STOP was set.
Think of:
$ cat /tmp/sql
begin;
foo;
select 1;
$ psql ts -f /tmp/sql ; echo ret=$?
BEGIN
psql:/tmp/sql:2: ERROR: syntax error at or near "foo"
LINE 1: foo;
^
psql:/tmp/sql:3: ERROR: current transaction is aborted, commands ignored until end of transaction block
ret=0
I think ON_ERROR_STOP would control whether the script stops, but it should
fail the exit status should reflect any error in the last command. The shell
does that even without set -e.
Justin
Hello. At Tue, 18 Dec 2018 10:24:39 -0600, Justin Pryzby <pryzby@telsasoft.com> wrote in <20181218162439.GB8042@telsasoft.com> > On Tue, Dec 18, 2018 at 05:13:40PM +0900, Kyotaro HORIGUCHI wrote: > > $ psql postgres -v ON_ERROR_STOP=0 -f ~/work/y.txt ; echo $? > > $ psql postgres -v ON_ERROR_STOP=0 < ~/work/y.txt ; echo $? > > > c) psql postgres -v ON_ERROR_STOP=0 -c foo -c 'select 1'; echo $? > > d) psql postgres -v ON_ERROR_STOP=0 -c 'select 1' -c foo; echo $? > > > > (c) returns 0 and (d) returns 1, but both return 1 when > > ON_ERROR_STOP=1. > > > The attached second patch lets (c) and (d) behave the same as (a) > > and (b). > > I don't think the behavior in the single command case should be changed: I guess the reason is that psql is widely used with just a single -c command and acutually the fix breaks the cases. So it doesn't seem back-pachable but it is apparently contradicting to documentation, which seems perfectly reasonable. So I propose to fix the behavior for 12 and back-patch documentation fix. | Exit Status | | psql returns 0 to the shell if it finished normally, 1 if a fatal | error of its own occurs (e.g. out of memory, file not found), 2 | if the connection to the server went bad and the session was not | interactive, and 3 if an error occurred in a script and the | variable ON_ERROR_STOP was set. + As the only exception, irrespective of ON_ERROR_STOP setting, + psql returns 1 if the last executed command failed and it was + givin by -c option. # What a ... > |[pryzbyj@database postgresql]$ ./src/bin/psql/psql ts -c FOO 2>/dev/null ; echo $? > |1 > |[pryzbyj@database postgresql]$ patch -p1 </tmp/0002-Unify-psql-s-behavior-on-c-with-scripts.patch > |patching file src/bin/psql/startup.c > |[pryzbyj@database postgresql]$ make >/dev/null > |[pryzbyj@database postgresql]$ ./src/bin/psql/psql ts -c FOO 2>/dev/null ; echo $? > |0 > > Also, unpatched v11 psql returns status of last command > |[pryzbyj@database postgresql]$ psql ts -xtc 'SELECT 1' -c FOO 2>/dev/null ; echo $? > |?column? | 1 > | > |1 > > patched psql returns 0: > |[pryzbyj@database postgresql]$ ./src/bin/psql/psql ts -xtc 'SELECT 1' -c FOO 2>/dev/null ; echo $? > |?column? | 1 > | > |0 > > I'd prefer to see the exit status of the -f scripts (your cases a and b) > changed to 3 if the last command failed. I realized that's not what the > documentation says so possibly not backpatchable. It doesn't make sense that psql exits with ERROR stauts when accidentially the last command was failed while erros occurred so far was ignored. It is achieved by ON_ERROR_STOP=1 in a better way. > |3 if an error occurred in a script and the variable ON_ERROR_STOP was set. > > Think of: > > $ cat /tmp/sql > begin; > foo; > select 1; > > $ psql ts -f /tmp/sql ; echo ret=$? > BEGIN > psql:/tmp/sql:2: ERROR: syntax error at or near "foo" > LINE 1: foo; > ^ > psql:/tmp/sql:3: ERROR: current transaction is aborted, commands ignored until end of transaction block > ret=0 As you wrote below, ON_ERROR_STOP=1 would give the desired behavior. > I think ON_ERROR_STOP would control whether the script stops, but it should > fail the exit status should reflect any error in the last command. The shell > does that even without set -e. At least bash behaves exactly the same way to psql for me. (Just so there's not confusion, set -e works opposite as you think. I always use explcit exit to do that, though). ===t.sh: hoge hage echo Ho-Ho-Ho === $ bash t.sh ; echo $? test.sh: line 1: hoge: command not found test.sh: line 2: hage: command not found Ho-Ho-Ho 0 ===t.sh: set -e hoge hage echo Ho-Ho-Ho === $ bash t.sh ; echo $? test.sh: line 2: hage: command not found 127 regards. -- Kyotaro Horiguchi NTT Open Source Software Center
At Thu, 31 Jan 2019 10:37:28 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> wrote in <20190131.103728.153290385.horiguchi.kyotaro@lab.ntt.co.jp> > > I think ON_ERROR_STOP would control whether the script stops, but it should > > fail the exit status should reflect any error in the last command. The shell > > does that even without set -e. > > At least bash behaves exactly the same way to psql for me. (Just > so there's not confusion, set -e works opposite as you think. I > always use explcit exit to do that, though). > > ===t.sh: > hoge > hage > echo Ho-Ho-Ho > === > $ bash t.sh ; echo $? > test.sh: line 1: hoge: command not found > test.sh: line 2: hage: command not found > Ho-Ho-Ho > 0 > > ===t.sh: > set -e > hoge > hage > echo Ho-Ho-Ho > === > $ bash t.sh ; echo $? > test.sh: line 2: hage: command not found > 127 Sorry, that was not so good as an example. === t.sh set -e ls -impossible echo HoHoHoHo === $ bash ~/test.sh; echo $? ls: invalid option -- 'e' Try 'ls --help' for more information. 2 === without set -e $ bash ~/test.sh; echo $? ls: invalid option -- 'e' Try 'ls --help' for more information. HoHoHoHo 0 # Wow. ls -impossibl works! regards. -- Kyotaro Horiguchi NTT Open Source Software Center
On Wed, Jan 30, 2019 at 6:38 PM Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> wrote: > I guess the reason is that psql is widely used with just a single > -c command and acutually the fix breaks the cases. So it doesn't > seem back-pachable but it is apparently contradicting to > documentation, which seems perfectly reasonable. > > So I propose to fix the behavior for 12 and back-patch > documentation fix. > > | Exit Status > | > | psql returns 0 to the shell if it finished normally, 1 if a fatal > | error of its own occurs (e.g. out of memory, file not found), 2 > | if the connection to the server went bad and the session was not > | interactive, and 3 if an error occurred in a script and the > | variable ON_ERROR_STOP was set. > + As the only exception, irrespective of ON_ERROR_STOP setting, > + psql returns 1 if the last executed command failed and it was > + givin by -c option. > In head can we just turn ON_ERROR_STOP on by default when more than one -c/-f is encountered, return 3, and call it a day. Then, if the user unsets ON_ERROR_STOP and does the same have psql always returns 0 unless a file specified with "-f" cannot be found (or some other application error...). If so can we maybe reconsider having ON_ERROR_STOP off by default generally... I don't like saying -c "is the same as a line in a script" since -c requires complete statements (and doesn't share transactions with other -c lines). Each -c simply provides psql with a single auto-commit statement to execute - in the supplied order. If we need a word here "procedure" might be a good choice. David J.
At Tue, 18 Dec 2018 10:24:39 -0600, Justin Pryzby <pryzby@telsasoft.com> wrote: > I think ON_ERROR_STOP would control whether the script stops, but it should > fail the exit status should reflect any error in the last command. The > shell does that even without set -e. Let me correct my own language: | I think ON_ERROR_STOP would control whether the script stops, but | the exit status should reflect any error in the last command. The | shell does that even without set -e. What I mean is that "the exit status of the shell reflects the status of the last command". That's why you'll see at the bottom of shscripts an "exit 0" which might seem to be implied. Actually falling off the end of the script is same as "exit" which is same as "exit $?" which may be nonzero if the preceding command was a conditional/test, like: for ... if ... fi done # without this, script will variously "fail" due to conditional evaluating to # false: exit 0 So I'm pointing out serious concern if psql would return 0 if last command failed, a case where it's currently exiting with status 1. Justin