Thread: Make ON_ERROR_STOP stop on shell script failure
Hi, """\set ON_ERROR_STOP on""" stops any subsequent incoming query that comes after an error of an SQL, but does not stop after a shell script ran by """\! <some command>""" returning values other than 0, -1, or 127, which suggests a failure in the result of the shell script. For example, suppose that below is an SQL file. \set ON_ERROR_STOP on SELECT 1; \! false SELECT 2; The current design allows SELECT 2 even though the shell script returns a value indicating a failure. I thought that this action is rather unexpected since, based on the word """ON_ERROR_STOP""", ones may expect that failures of shell scripts should halt the incoming instructions as well. One clear solution is to let failures of shell script stop incoming queries just like how errors of SQLs do currently. Thoughts? Tatsu
Attachment
At Fri, 16 Sep 2022 15:55:33 +0900, bt22nakamorit <bt22nakamorit@oss.nttdata.com> wrote in > Hi, > > """\set ON_ERROR_STOP on""" stops any subsequent incoming query that > comes after an error of an SQL, but does not stop after a shell script > ran by """\! <some command>""" returning values other than 0, -1, or > 127, which suggests a failure in the result of the shell script. > > For example, suppose that below is an SQL file. > \set ON_ERROR_STOP on > SELECT 1; > \! false > SELECT 2; > > The current design allows SELECT 2 even though the shell script > returns a value indicating a failure. Since the "false" command did not "error out"? > I thought that this action is rather unexpected since, based on the > word """ON_ERROR_STOP""", ones may expect that failures of shell > scripts should halt the incoming instructions as well. > One clear solution is to let failures of shell script stop incoming > queries just like how errors of SQLs do currently. Thoughts? I'm not sure we want to regard any exit status from a succssful run as a failure. On the other hand, the proposed behavior seems useful to me. So +1 from me to the proposal, assuming the corresponding edit of the documentation happens. regards. -- Kyotaro Horiguchi NTT Open Source Software Center
2022-09-16 17:30 に Kyotaro Horiguchi さんは書きました: > At Fri, 16 Sep 2022 15:55:33 +0900, bt22nakamorit > <bt22nakamorit@oss.nttdata.com> wrote in >> Hi, >> >> """\set ON_ERROR_STOP on""" stops any subsequent incoming query that >> comes after an error of an SQL, but does not stop after a shell script >> ran by """\! <some command>""" returning values other than 0, -1, or >> 127, which suggests a failure in the result of the shell script. >> >> For example, suppose that below is an SQL file. >> \set ON_ERROR_STOP on >> SELECT 1; >> \! false >> SELECT 2; >> >> The current design allows SELECT 2 even though the shell script >> returns a value indicating a failure. > > Since the "false" command did not "error out"? > >> I thought that this action is rather unexpected since, based on the >> word """ON_ERROR_STOP""", ones may expect that failures of shell >> scripts should halt the incoming instructions as well. >> One clear solution is to let failures of shell script stop incoming >> queries just like how errors of SQLs do currently. Thoughts? > > I'm not sure we want to regard any exit status from a succssful run as > a failure. > > On the other hand, the proposed behavior seems useful to me. > > So +1 from me to the proposal, assuming the corresponding edit of the > documentation happens. > > regards. > Since the "false" command did not "error out"? "false" command returns 1 which is an exit status code that indicates failure, but not error. I think it does not "error out" if that is what you mean. > So +1 from me to the proposal, assuming the corresponding edit of the > documentation happens. I will work on editing the document and share further updates. Thank you! Tatsu
2022-09-17 09:44 に bt22nakamorit さんは書きました: > 2022-09-16 17:30 に Kyotaro Horiguchi さんは書きました: >> At Fri, 16 Sep 2022 15:55:33 +0900, bt22nakamorit >> <bt22nakamorit@oss.nttdata.com> wrote in >>> Hi, >>> >>> """\set ON_ERROR_STOP on""" stops any subsequent incoming query that >>> comes after an error of an SQL, but does not stop after a shell >>> script >>> ran by """\! <some command>""" returning values other than 0, -1, or >>> 127, which suggests a failure in the result of the shell script. >>> >>> For example, suppose that below is an SQL file. >>> \set ON_ERROR_STOP on >>> SELECT 1; >>> \! false >>> SELECT 2; >>> >>> The current design allows SELECT 2 even though the shell script >>> returns a value indicating a failure. >> >> Since the "false" command did not "error out"? >> >>> I thought that this action is rather unexpected since, based on the >>> word """ON_ERROR_STOP""", ones may expect that failures of shell >>> scripts should halt the incoming instructions as well. >>> One clear solution is to let failures of shell script stop incoming >>> queries just like how errors of SQLs do currently. Thoughts? >> >> I'm not sure we want to regard any exit status from a succssful run as >> a failure. >> >> On the other hand, the proposed behavior seems useful to me. >> >> So +1 from me to the proposal, assuming the corresponding edit of the >> documentation happens. >> >> regards. > >> Since the "false" command did not "error out"? > "false" command returns 1 which is an exit status code that indicates > failure, but not error. > I think it does not "error out" if that is what you mean. > >> So +1 from me to the proposal, assuming the corresponding edit of the >> documentation happens. > I will work on editing the document and share further updates. > > Thank you! > Tatsu I edited the documentation for ON_ERROR_STOP. Any other suggestions? Tatsu
Attachment
On 2022/09/20 15:15, bt22nakamorit wrote: >>>> I thought that this action is rather unexpected since, based on the >>>> word """ON_ERROR_STOP""", ones may expect that failures of shell >>>> scripts should halt the incoming instructions as well. >>>> One clear solution is to let failures of shell script stop incoming >>>> queries just like how errors of SQLs do currently. Thoughts? +1 > I edited the documentation for ON_ERROR_STOP. > Any other suggestions? Thanks for the patch! Could you add it to the next CommitFest so that we don't forget it? We can execute the shell commands via psql in various ways other than \! meta-command. For example, * `command` * \g | command * \gx | command * \o | command * \w | command * \copy ... program 'command' ON_ERROR_STOP should handle not only \! but also all the above in the same way? One concern about this patch is that some applications already depend on the current behavior of ON_ERROR_STOP, i.e., psql doesn't stop even when the shell command returns non-zero exit code. If so, we might need to extend ON_ERROR_STOP so that it accepts the following setting values. * off - don't stop even when either sql or shell fails (same as the current behavior) * on or sql - stop only whensql fails (same as the current behavior) * shell - stop only when shell fails * all - stop when either sql or shell fails Thought? Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION
At Wed, 21 Sep 2022 11:45:07 +0900, Fujii Masao <masao.fujii@oss.nttdata.com> wrote in > > > On 2022/09/20 15:15, bt22nakamorit wrote: > >>>> I thought that this action is rather unexpected since, based on the > >>>> word """ON_ERROR_STOP""", ones may expect that failures of shell > >>>> scripts should halt the incoming instructions as well. > >>>> One clear solution is to let failures of shell script stop incoming > >>>> queries just like how errors of SQLs do currently. Thoughts? > > +1 > > > > I edited the documentation for ON_ERROR_STOP. > > Any other suggestions? > > Thanks for the patch! > Could you add it to the next CommitFest so that we don't forget it? > > > We can execute the shell commands via psql in various ways > other than \! meta-command. For example, > > * `command` > * \g | command > * \gx | command > * \o | command > * \w | command > * \copy ... program 'command' > > ON_ERROR_STOP should handle not only \! but also all the above in the > same way? +1 > One concern about this patch is that some applications already depend > on > the current behavior of ON_ERROR_STOP, i.e., psql doesn't stop even > when > the shell command returns non-zero exit code. If so, we might need to > extend ON_ERROR_STOP so that it accepts the following setting values. > > * off - don't stop even when either sql or shell fails (same as the > * current behavior) > * on or sql - stop only whensql fails (same as the current behavior) > * shell - stop only when shell fails > * all - stop when either sql or shell fails > > Thought? +1 regards. -- Kyotaro Horiguchi NTT Open Source Software Center
Fujii Masao: > One concern about this patch is that some applications already depend on > the current behavior of ON_ERROR_STOP, i.e., psql doesn't stop even when > the shell command returns non-zero exit code. If so, we might need to > extend ON_ERROR_STOP so that it accepts the following setting values. I just got bitten by this and I definitely consider this a bug. I expect psql to stop when a shell script fails and I have ON_ERROR_STOP set. I don't think this should be made more complicated with different settings. If someone needs to have ON_ERROR_STOP set, but continue execution after a certain shell command, they could still do something like this: \! might_fail || true Best Wolfgang
On 2022-09-20 15:15, bt22nakamorit wrote: > I edited the documentation for ON_ERROR_STOP. > Any other suggestions? Thanks for the patch! > if (result == 127 || result == -1) > { > pg_log_error("\\!: failed"); > return false; > } > else if (result != 0) { > pg_log_error("command failed"); > return false; Since it would be hard to understand the cause of failures from these two messages, it might be better to clarify them in the messages. The former comes from failures of child process creation or execution on it and the latter occurs when child process creation and execution succeeded but the return code is not 0, doesn't it? I also felt it'd be natural that the latter message also begins with "\\!" since both message concerns with \!. How do you think? -- Regards, -- Atsushi Torikoshi NTT DATA CORPORATION
2022-09-28 21:49 に torikoshia さんは書きました: >> if (result == 127 || result == -1) >> { >> pg_log_error("\\!: failed"); >> return false; >> } >> else if (result != 0) { >> pg_log_error("command failed"); >> return false; > > Since it would be hard to understand the cause of failures from these > two messages, it might be better to clarify them in the messages. > > The former comes from failures of child process creation or execution > on it and the latter occurs when child process creation and execution > succeeded but the return code is not 0, doesn't it? > > > I also felt it'd be natural that the latter message also begins with > "\\!" since both message concerns with \!. > > How do you think? Thank you for the feedback! I agree that the messages should be more clear. \\!: command was not executed \\!: command failed Would these two messages be enough to describe the two cases? Tatsu
At Thu, 29 Sep 2022 11:29:40 +0900, bt22nakamorit <bt22nakamorit@oss.nttdata.com> wrote in > 2022-09-28 21:49 に torikoshia さんは書きました: > >> if (result == 127 || result == -1) > >> { > >> pg_log_error("\\!: failed"); > >> return false; > >> } > >> else if (result != 0) { > >> pg_log_error("command failed"); > >> return false; > > Since it would be hard to understand the cause of failures from these > > two messages, it might be better to clarify them in the messages. > > The former comes from failures of child process creation or execution > > on it and the latter occurs when child process creation and execution > > succeeded but the return code is not 0, doesn't it? > > I also felt it'd be natural that the latter message also begins with > > "\\!" since both message concerns with \!. > > How do you think? > > Thank you for the feedback! > I agree that the messages should be more clear. > \\!: command was not executed > \\!: command failed > Would these two messages be enough to describe the two cases? FWIW, I would spell these as something like this: > \\!: command execution failure: %m > \\!: command returned failure status: %d regards. -- Kyotaro Horiguchi NTT Open Source Software Center
At Thu, 29 Sep 2022 12:35:04 +0900 (JST), Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote in > > Thank you for the feedback! > > I agree that the messages should be more clear. > > \\!: command was not executed > > \\!: command failed > > Would these two messages be enough to describe the two cases? > > FWIW, I would spell these as something like this: > > > \\!: command execution failure: %m The following might be more complient to our policy. > \\!: failed to execute command \"%s\": %m > > \\!: command returned failure status: %d regards. -- Kyotaro Horiguchi NTT Open Source Software Center
2022-09-21 11:45 に Fujii Masao wrote: > We can execute the shell commands via psql in various ways > other than \! meta-command. For example, > > * `command` > * \g | command > * \gx | command > * \o | command > * \w | command > * \copy ... program 'command' > > ON_ERROR_STOP should handle not only \! but also all the above in the > same way? > > > One concern about this patch is that some applications already depend > on > the current behavior of ON_ERROR_STOP, i.e., psql doesn't stop even > when > the shell command returns non-zero exit code. If so, we might need to > extend ON_ERROR_STOP so that it accepts the following setting values. > > * off - don't stop even when either sql or shell fails (same as the > current behavior) > * on or sql - stop only whensql fails (same as the current behavior) > * shell - stop only when shell fails > * all - stop when either sql or shell fails > > Thought? > > Regards, I agree that some applications may depend on the behavior of previous ON_ERROR_STOP. I created a patch that implements off, on, shell, and all option for ON_ERROR_STOP. I also edited the code for \g, \o, \w, and \set in addition to \! to return exit status of shell commands for ON_ERROR_STOP. There were discussions regarding the error messages for when shell command fails. I have found that \copy already handles exit status of shell commands when it executes one, so I copied the messages from there. More specifically, I referred to """bool do_copy(const char *args)""" in src/bin/psql/copy.c Any feedback would be very much appreciated. Tatsu
Attachment
On 2022/09/30 16:54, bt22nakamorit wrote: > 2022-09-21 11:45 に Fujii Masao wrote: >> We can execute the shell commands via psql in various ways >> other than \! meta-command. For example, >> >> * `command` >> * \g | command >> * \gx | command >> * \o | command >> * \w | command >> * \copy ... program 'command' >> >> ON_ERROR_STOP should handle not only \! but also all the above in the same way? >> >> >> One concern about this patch is that some applications already depend on >> the current behavior of ON_ERROR_STOP, i.e., psql doesn't stop even when >> the shell command returns non-zero exit code. If so, we might need to >> extend ON_ERROR_STOP so that it accepts the following setting values. >> >> * off - don't stop even when either sql or shell fails (same as the >> current behavior) >> * on or sql - stop only whensql fails (same as the current behavior) >> * shell - stop only when shell fails >> * all - stop when either sql or shell fails >> >> Thought? >> >> Regards, > > I agree that some applications may depend on the behavior of previous ON_ERROR_STOP. > I created a patch that implements off, on, shell, and all option for ON_ERROR_STOP. > I also edited the code for \g, \o, \w, and \set in addition to \! to return exit status of shell commands for ON_ERROR_STOP. > > There were discussions regarding the error messages for when shell command fails. > I have found that \copy already handles exit status of shell commands when it executes one, so I copied the messages fromthere. > More specifically, I referred to """bool do_copy(const char *args)""" in src/bin/psql/copy.c > > Any feedback would be very much appreciated. Thanks for updating the patch! The patch failed to be applied into the master cleanly. Could you rebase it? patching file src/bin/psql/common.c Hunk #1 succeeded at 94 (offset 4 lines). Hunk #2 succeeded at 104 (offset 4 lines). Hunk #3 succeeded at 133 (offset 4 lines). Hunk #4 succeeded at 1869 with fuzz 1 (offset 1162 lines). Hunk #5 FAILED at 2624. 1 out of 5 hunks FAILED -- saving rejects to file src/bin/psql/common.c.rej Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION
2022-10-07 17:16 Fujii Masao wrote: > The patch failed to be applied into the master cleanly. Could you > rebase it? > > patching file src/bin/psql/common.c > Hunk #1 succeeded at 94 (offset 4 lines). > Hunk #2 succeeded at 104 (offset 4 lines). > Hunk #3 succeeded at 133 (offset 4 lines). > Hunk #4 succeeded at 1869 with fuzz 1 (offset 1162 lines). > Hunk #5 FAILED at 2624. > 1 out of 5 hunks FAILED -- saving rejects to file > src/bin/psql/common.c.rej Thank you for checking. I edited the patch so that it would apply to the latest master branch. Please mention if there are any other problems. Best, Tatsuhiro Nakamori
Attachment
There was a mistake in the error message for \! so I updated the patch. Best, Tatsuhiro Nakamori
Attachment
On Fri, Sep 16, 2022 at 03:55:33PM +0900, bt22nakamorit wrote: > Hi, > > """\set ON_ERROR_STOP on""" stops any subsequent incoming query that comes > after an error of an SQL, but does not stop after a shell script ran by > """\! <some command>""" returning values other than 0, -1, or 127, which > suggests a failure in the result of the shell script. Actually, I think this could be described as a wider problem (not just ON_ERROR_STOP). The shell's exit status is being ignored (except for -1 and 127). Shouldn't the user be able to do something with the exit status ? Right now, it seems like they'd need to wrap the shellscript with "if ! ...; then echo failed; fi" and then \gset and compare with "failed" I think it'd be a lot better to expose the script status to psql. (without having to write "foo; echo status=$?"). Another consideration is that shellscripts can exit with a nonzero status due to the most recent conditional (like: if || &&). For example, consider shell command like: "if foo; then bar; fi" or "foo && bar" If foo has nonzero status, then bar isn't run. If that's the entire shell script, the shell will *also* exit with foo's nonzero status. (That's the reason why people write "exit 0" as the last line of a shell script. It's easy to believe that it was going to "exit 0" in any case; but, what it was actually going to do was to "exit $?", and $? can be nonzero after conditionals, even in "set -e" mode). So a psql script like this would start to report as a failure any time "foo" was false, even if that's the normal/typical case. -- Justin
I think it'd be a lot better to expose the script status to psql.
(without having to write "foo; echo status=$?").
I agree, and I hacked up a proof of concept, but started another thread at https://www.postgresql.org/message-id/CADkLM=cWao2x2f+UDw15W1JkVFr_bsxfstw=NGea7r9m4j-7rQ@mail.gmail.com so as not to clutter up this one.
------- Original Message ------- On Tuesday, November 22nd, 2022 at 20:10, bt22nakamorit <bt22nakamorit@oss.nttdata.com> wrote: > There was a mistake in the error message for \! so I updated the patch. > > Best, > Tatsuhiro Nakamori Hi I was checking your patch and seems that it failed to be applied into the master cleanly. Could you please rebase it? -- Matheus Alcantara
On Tue, Nov 22, 2022 at 6:16 PM Matheus Alcantara <mths.dev@pm.me> wrote:
------- Original Message -------
On Tuesday, November 22nd, 2022 at 20:10, bt22nakamorit <bt22nakamorit@oss.nttdata.com> wrote:
> There was a mistake in the error message for \! so I updated the patch.
>
> Best,
> Tatsuhiro Nakamori
Hi
I was checking your patch and seems that it failed to be applied into the
master cleanly. Could you please rebase it?
Yes. My apologies, I had several life events get in the way.
On 2022-10-12 2:13 a.m., bt22nakamorit wrote: > There was a mistake in the error message for \! so I updated the patch. > Tried to apply this patch to the master branch, but got the error like below. $ git apply --check patch-view.diff error: patch failed: src/bin/psql/command.c:2693 error: src/bin/psql/command.c: patch does not apply I think there are some tests related with "ON_ERROR_STOP" in src/bin/psql/t/001_basic.pl, and we should consider to add corresponding test cases for "on/off/shell/all" to this patch. Best regards, David
On 23/11/2022 00:16, Matheus Alcantara wrote: > ------- Original Message ------- > On Tuesday, November 22nd, 2022 at 20:10, bt22nakamorit <bt22nakamorit@oss.nttdata.com> wrote: > > >> There was a mistake in the error message for \! so I updated the patch. >> >> Best, >> Tatsuhiro Nakamori > Hi > > I was checking your patch and seems that it failed to be applied into the > master cleanly. Could you please rebase it? Was also looking into this patch. Tatsuhiro: can you please send a rebased version? Thanks -- Andreas 'ads' Scherbaum German PostgreSQL User Group European PostgreSQL User Group - Board of Directors Volunteer Regional Contact, Germany - PostgreSQL Project
On 16/02/2023 20:33, Andreas 'ads' Scherbaum wrote: > On 23/11/2022 00:16, Matheus Alcantara wrote: >> ------- Original Message ------- >> On Tuesday, November 22nd, 2022 at 20:10, bt22nakamorit >> <bt22nakamorit@oss.nttdata.com> wrote: >> >> >>> There was a mistake in the error message for \! so I updated the patch. >>> >>> Best, >>> Tatsuhiro Nakamori >> Hi >> >> I was checking your patch and seems that it failed to be applied into >> the >> master cleanly. Could you please rebase it? > > Was also looking into this patch. > > Tatsuhiro: can you please send a rebased version? The email address is bouncing. That might be why ... -- Andreas 'ads' Scherbaum German PostgreSQL User Group European PostgreSQL User Group - Board of Directors Volunteer Regional Contact, Germany - PostgreSQL Project
So I took a look at this patch. The conflict is with 2fe3bdbd691 committed by Peter Eisentraut which added error checks for pipes. Afaics the behaviour is now for pipe commands returning non-zero to cause an error *always* regardless of the setting of ON_ERROR_STOP. I'm not entirely sure that's sensible actually. If you write to a pipe that ends in grep and it happens to produce no matching rows you may actually be quite surprised when that causes your script to fail... But if you remove that failing hunk the resulting patch does apply. I don't see any tests so ... I don't know if the behaviour is still sensible. A quick read gives me the impression that now it's actually inconsistent in the other direction where it stops sometimes more often than the user might expect. I also don't understand the difference between ON_ERROR_STOP_ON and ON_ERROR_STOP_ALL. Nor why we would want ON_ERROR_STOP_SHELL which stops only on shell errors, rather than, say, ON_ERROR_STOP_SQL to do the converse which would at least match the historical behaviour?
Attachment
Pruning bouncing email address -- please respond from this point in thread, not previous messages. -- Gregory Stark As Commitfest Manager
On Mon, 20 Mar 2023 at 14:34, Gregory Stark (as CFM) <stark.cfm@gmail.com> wrote: > > Pruning bouncing email address -- please respond from this point in > thread, not previous messages. Oh for heaven's sake. Trying again to prune bouncing email address. Please respond from *here* on the thread.... Sorry -- Gregory Stark As Commitfest Manager
On 20.03.23 19:31, Greg Stark wrote: > So I took a look at this patch. The conflict is with 2fe3bdbd691 > committed by Peter Eisentraut which added error checks for pipes. > Afaics the behaviour is now for pipe commands returning non-zero to > cause an error*always* regardless of the setting of ON_ERROR_STOP. > > I'm not entirely sure that's sensible actually. If you write to a pipe > that ends in grep and it happens to produce no matching rows you may > actually be quite surprised when that causes your script to fail... The only thing that that patch changed in psql was the \w command, and AFAICT, ON_ERROR_STOP is still respected: $ cat test.sql \w |foo $ psql -f test.sql sh: foo: command not found psql:test.sql:1: error: |foo: command not found $ echo $? 0 $ psql -f test.sql -v ON_ERROR_STOP=1 sh: foo: command not found psql:test.sql:1: error: |foo: command not found $ echo $? 3
On Fri, Mar 24, 2023 at 11:07 AM Peter Eisentraut <peter.eisentraut@enterprisedb.com> wrote:
On 20.03.23 19:31, Greg Stark wrote:
> So I took a look at this patch. The conflict is with 2fe3bdbd691
> committed by Peter Eisentraut which added error checks for pipes.
> Afaics the behaviour is now for pipe commands returning non-zero to
> cause an error*always* regardless of the setting of ON_ERROR_STOP.
Commit b0d8f2d983cb25d1035fae1cd7de214dd67809b4 adds SHELL_ERROR as a set to 'true' whenever a \! or backtick command has a nonzero exit code. So it'll need some rebasing to remove the duplicated work.
So it's now possible to do this:
\set result = `some command`
\if :SHELL_ERROR
-- maybe test SHELL_EXIT_CODE to see what kind of error
\echo some command failed
-- nah, just quit
\q
\endif
> I'm not entirely sure that's sensible actually. If you write to a pipe
> that ends in grep and it happens to produce no matching rows you may
> actually be quite surprised when that causes your script to fail...
I agree that that would be quite surprising, and this feature would be a non-starter for them. But if we extended the SHELL_ERROR and SHELL_EXIT_CODE patch to handle output pipes (which maybe we should have done in the first place), the handling would look like this:
SELECT ... \g grep Frobozz
\if :SHELL_ERROR
SELECT :SHELL_EXIT_CODE = 1 AS grep_found_nothing \gset
\if :grep_found_nothing
\if :SHELL_ERROR
SELECT :SHELL_EXIT_CODE = 1 AS grep_found_nothing \gset
\if :grep_found_nothing
...not-a-real-error handling...
\else
...regular error handling...
\endif
\endif
...and that would be the solution for people who wanted to do something more nuanced than ON_ERROR_STOP.
\else
...regular error handling...
\endif
\endif
...and that would be the solution for people who wanted to do something more nuanced than ON_ERROR_STOP.
On Fri, Mar 24, 2023 at 2:16 PM Corey Huinker <corey.huinker@gmail.com> wrote:
On Fri, Mar 24, 2023 at 11:07 AM Peter Eisentraut <peter.eisentraut@enterprisedb.com> wrote:On 20.03.23 19:31, Greg Stark wrote:
> So I took a look at this patch. The conflict is with 2fe3bdbd691
> committed by Peter Eisentraut which added error checks for pipes.
> Afaics the behaviour is now for pipe commands returning non-zero to
> cause an error*always* regardless of the setting of ON_ERROR_STOP.Commit b0d8f2d983cb25d1035fae1cd7de214dd67809b4 adds SHELL_ERROR as a set to 'true' whenever a \! or backtick command has a nonzero exit code. So it'll need some rebasing to remove the duplicated work.
So it's now possible to do this:
\set result = `some command`
\if :SHELL_ERROR
-- maybe test SHELL_EXIT_CODE to see what kind of error
\echo some command failed
-- nah, just quit
\q\endif
> I'm not entirely sure that's sensible actually. If you write to a pipe
> that ends in grep and it happens to produce no matching rows you may
> actually be quite surprised when that causes your script to fail...
I agree that that would be quite surprising, and this feature would be a non-starter for them. But if we extended the SHELL_ERROR and SHELL_EXIT_CODE patch to handle output pipes (which maybe we should have done in the first place), the handling would look like this:SELECT ... \g grep Frobozz
\if :SHELL_ERROR
SELECT :SHELL_EXIT_CODE = 1 AS grep_found_nothing \gset
\if :grep_found_nothing...not-a-real-error handling...
\else
...regular error handling...
\endif
\endif
...and that would be the solution for people who wanted to do something more nuanced than ON_ERROR_STOP.
Dangit. Replied to Peter's email thinking he had gone off Greg's culling of the recipients. Re-culled.
This patch hasn't applied for quite some time, has been waiting on author since December, and the thread has stalled. I'm marking this Returned with Feedback for now, please feel free to resubmit to a future CF when there is renewed interest in working on this. -- Daniel Gustafsson