public inbox for [email protected]
help / color / mirror / Atom feedMake ON_ERROR_STOP stop on shell script failure
20+ messages / 9 participants
[nested] [flat]
* Make ON_ERROR_STOP stop on shell script failure
@ 2022-09-16 06:55 bt22nakamorit <[email protected]>
2022-09-16 08:30 ` Re: Make ON_ERROR_STOP stop on shell script failure Kyotaro Horiguchi <[email protected]>
2022-11-02 11:58 ` Re: Make ON_ERROR_STOP stop on shell script failure Justin Pryzby <[email protected]>
0 siblings, 2 replies; 20+ messages in thread
From: bt22nakamorit @ 2022-09-16 06:55 UTC (permalink / raw)
To: PostgreSQL Hackers <[email protected]>
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
Attachments:
[text/x-diff] stop_error.patch (363B, ../../[email protected]/2-stop_error.patch)
download | inline diff:
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index a141146e70..7445ca04ff 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -4989,6 +4989,10 @@ do_shell(const char *command)
pg_log_error("\\!: failed");
return false;
}
+ else if (result != 0) {
+ pg_log_error("command failed");
+ return false;
+ }
return true;
}
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Make ON_ERROR_STOP stop on shell script failure
2022-09-16 06:55 Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
@ 2022-09-16 08:30 ` Kyotaro Horiguchi <[email protected]>
2022-09-17 00:44 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
1 sibling, 1 reply; 20+ messages in thread
From: Kyotaro Horiguchi @ 2022-09-16 08:30 UTC (permalink / raw)
To: [email protected]; +Cc: [email protected]
At Fri, 16 Sep 2022 15:55:33 +0900, bt22nakamorit <[email protected]> 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
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Make ON_ERROR_STOP stop on shell script failure
2022-09-16 06:55 Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-09-16 08:30 ` Re: Make ON_ERROR_STOP stop on shell script failure Kyotaro Horiguchi <[email protected]>
@ 2022-09-17 00:44 ` bt22nakamorit <[email protected]>
2022-09-20 06:15 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
0 siblings, 1 reply; 20+ messages in thread
From: bt22nakamorit @ 2022-09-17 00:44 UTC (permalink / raw)
To: Kyotaro Horiguchi <[email protected]>; +Cc: [email protected]
2022-09-16 17:30 に Kyotaro Horiguchi さんは書きました:
> At Fri, 16 Sep 2022 15:55:33 +0900, bt22nakamorit
> <[email protected]> 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
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Make ON_ERROR_STOP stop on shell script failure
2022-09-16 06:55 Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-09-16 08:30 ` Re: Make ON_ERROR_STOP stop on shell script failure Kyotaro Horiguchi <[email protected]>
2022-09-17 00:44 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
@ 2022-09-20 06:15 ` bt22nakamorit <[email protected]>
2022-09-21 02:45 ` Re: Make ON_ERROR_STOP stop on shell script failure Fujii Masao <[email protected]>
2022-09-28 12:49 ` Re: Make ON_ERROR_STOP stop on shell script failure torikoshia <[email protected]>
0 siblings, 2 replies; 20+ messages in thread
From: bt22nakamorit @ 2022-09-20 06:15 UTC (permalink / raw)
To: Kyotaro Horiguchi <[email protected]>; +Cc: [email protected]
2022-09-17 09:44 に bt22nakamorit さんは書きました:
> 2022-09-16 17:30 に Kyotaro Horiguchi さんは書きました:
>> At Fri, 16 Sep 2022 15:55:33 +0900, bt22nakamorit
>> <[email protected]> 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
Attachments:
[text/x-diff] stop_error.patch (1.2K, ../../[email protected]/2-stop_error.patch)
download | inline diff:
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 9494f28063..2395678938 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -4143,7 +4143,9 @@ bar
<para>
By default, command processing continues after an error. When this
variable is set to <literal>on</literal>, processing will instead stop
- immediately. In interactive mode,
+ immediately. A nonzero exit status of a shell command, which indicates
+ failure, is also interpreted as an error that stops the processing.
+ In interactive mode,
<application>psql</application> will return to the command prompt;
otherwise, <application>psql</application> will exit, returning
error code 3 to distinguish this case from fatal error
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index a141146e70..7445ca04ff 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -4989,6 +4989,10 @@ do_shell(const char *command)
pg_log_error("\\!: failed");
return false;
}
+ else if (result != 0) {
+ pg_log_error("command failed");
+ return false;
+ }
return true;
}
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Make ON_ERROR_STOP stop on shell script failure
2022-09-16 06:55 Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-09-16 08:30 ` Re: Make ON_ERROR_STOP stop on shell script failure Kyotaro Horiguchi <[email protected]>
2022-09-17 00:44 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-09-20 06:15 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
@ 2022-09-21 02:45 ` Fujii Masao <[email protected]>
2022-09-27 03:34 ` Re: Make ON_ERROR_STOP stop on shell script failure Kyotaro Horiguchi <[email protected]>
2022-09-28 06:36 ` Re: Make ON_ERROR_STOP stop on shell script failure [email protected]
2022-09-30 07:54 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
1 sibling, 3 replies; 20+ messages in thread
From: Fujii Masao @ 2022-09-21 02:45 UTC (permalink / raw)
To: bt22nakamorit <[email protected]>; Kyotaro Horiguchi <[email protected]>; +Cc: [email protected]
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
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Make ON_ERROR_STOP stop on shell script failure
2022-09-16 06:55 Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-09-16 08:30 ` Re: Make ON_ERROR_STOP stop on shell script failure Kyotaro Horiguchi <[email protected]>
2022-09-17 00:44 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-09-20 06:15 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-09-21 02:45 ` Re: Make ON_ERROR_STOP stop on shell script failure Fujii Masao <[email protected]>
@ 2022-09-27 03:34 ` Kyotaro Horiguchi <[email protected]>
2 siblings, 0 replies; 20+ messages in thread
From: Kyotaro Horiguchi @ 2022-09-27 03:34 UTC (permalink / raw)
To: [email protected]; +Cc: [email protected]; [email protected]
At Wed, 21 Sep 2022 11:45:07 +0900, Fujii Masao <[email protected]> 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
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Make ON_ERROR_STOP stop on shell script failure
2022-09-16 06:55 Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-09-16 08:30 ` Re: Make ON_ERROR_STOP stop on shell script failure Kyotaro Horiguchi <[email protected]>
2022-09-17 00:44 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-09-20 06:15 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-09-21 02:45 ` Re: Make ON_ERROR_STOP stop on shell script failure Fujii Masao <[email protected]>
@ 2022-09-28 06:36 ` [email protected]
2 siblings, 0 replies; 20+ messages in thread
From: [email protected] @ 2022-09-28 06:36 UTC (permalink / raw)
To: Fujii Masao <[email protected]>; bt22nakamorit <[email protected]>; Kyotaro Horiguchi <[email protected]>; +Cc: [email protected]
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
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Make ON_ERROR_STOP stop on shell script failure
2022-09-16 06:55 Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-09-16 08:30 ` Re: Make ON_ERROR_STOP stop on shell script failure Kyotaro Horiguchi <[email protected]>
2022-09-17 00:44 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-09-20 06:15 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-09-21 02:45 ` Re: Make ON_ERROR_STOP stop on shell script failure Fujii Masao <[email protected]>
@ 2022-09-30 07:54 ` bt22nakamorit <[email protected]>
2022-10-07 08:16 ` Re: Make ON_ERROR_STOP stop on shell script failure Fujii Masao <[email protected]>
2 siblings, 1 reply; 20+ messages in thread
From: bt22nakamorit @ 2022-09-30 07:54 UTC (permalink / raw)
To: Fujii Masao <[email protected]>; +Cc: Kyotaro Horiguchi <[email protected]>; [email protected]
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
Attachments:
[text/x-diff] stop_error2.patch (8.8K, ../../[email protected]/2-stop_error2.patch)
download | inline diff:
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 9494f28063..9441b0b931 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -4143,7 +4143,12 @@ bar
<para>
By default, command processing continues after an error. When this
variable is set to <literal>on</literal>, processing will instead stop
- immediately. In interactive mode,
+ immediately upon an error in SQL query. When this variable is set to
+ <literal>shell</literal>, a nonzero exit status of a shell command,
+ which indicates failure, is interpreted as an error that stops the processing.
+ When this variable is set to <literal>all</literal>, errors from both
+ SQL queries and shell commands can stop the processing.
+ In interactive mode,
<application>psql</application> will return to the command prompt;
otherwise, <application>psql</application> will exit, returning
error code 3 to distinguish this case from fatal error
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index a141146e70..40a113630d 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -34,6 +34,7 @@
#include "describe.h"
#include "fe_utils/cancel.h"
#include "fe_utils/print.h"
+#include "fe_utils/psqlscan_int.h"
#include "fe_utils/string_utils.h"
#include "help.h"
#include "input.h"
@@ -2355,9 +2356,13 @@ exec_command_set(PsqlScanState scan_state, bool active_branch)
*/
char *newval;
char *opt;
+ PQExpBuffer output_buf = scan_state->output_buf;
opt = psql_scan_slash_option(scan_state,
OT_NORMAL, NULL, false);
+ if (output_buf->len >= output_buf->maxlen
+ && (pset.on_error_stop == PSQL_ERROR_STOP_SHELL || pset.on_error_stop == PSQL_ERROR_STOP_ALL))
+ success = false;
newval = pg_strdup(opt ? opt : "");
free(opt);
@@ -2693,8 +2698,25 @@ exec_command_write(PsqlScanState scan_state, bool active_branch,
else if (previous_buf && previous_buf->len > 0)
fprintf(fd, "%s\n", previous_buf->data);
- if (is_pipe)
+ if (is_pipe) {
result = pclose(fd);
+
+ if (result != 0)
+ {
+ if (result < 0)
+ pg_log_error("could not close pipe to external command: %m");
+ else
+ {
+ char *reason = wait_result_to_str(result);
+
+ pg_log_error("%s: %s", fname + 1,
+ reason ? reason : "");
+ free(reason);
+ }
+ }
+ if (pset.on_error_stop == PSQL_ERROR_STOP_SHELL || pset.on_error_stop == PSQL_ERROR_STOP_ALL)
+ status = PSQL_CMD_ERROR;
+ }
else
result = fclose(fd);
@@ -4984,10 +5006,19 @@ do_shell(const char *command)
else
result = system(command);
- if (result == 127 || result == -1)
+ if (result != 0)
{
- pg_log_error("\\!: failed");
- return false;
+ if (result < 0)
+ pg_log_error("could not close pipe to external command: %m");
+ else
+ {
+ char *reason = wait_result_to_str(result);
+
+ pg_log_error("%s: %s", command, reason ? reason : "");
+ free(reason);
+ }
+ if (pset.on_error_stop == PSQL_ERROR_STOP_SHELL || pset.on_error_stop == PSQL_ERROR_STOP_ALL)
+ return false;
}
return true;
}
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index e611e3266d..0f6f7448d1 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -90,6 +90,7 @@ setQFout(const char *fname)
{
FILE *fout;
bool is_pipe;
+ bool status = true;
/* First make sure we can open the new output file/pipe */
if (!openQueryOutputFile(fname, &fout, &is_pipe))
@@ -99,7 +100,24 @@ setQFout(const char *fname)
if (pset.queryFout && pset.queryFout != stdout && pset.queryFout != stderr)
{
if (pset.queryFoutPipe)
- pclose(pset.queryFout);
+ {
+ int pclose_rc = pclose(pset.queryFout);
+ if (pclose_rc != 0)
+ {
+ if (pclose_rc < 0)
+ pg_log_error("could not close pipe to external command: %m");
+ else
+ {
+ char *reason = wait_result_to_str(pclose_rc);
+
+ pg_log_error("command failure: %s",
+ reason ? reason : "");
+ free(reason);
+ }
+ if ((pset.on_error_stop == PSQL_ERROR_STOP_SHELL || pset.on_error_stop == PSQL_ERROR_STOP_ALL))
+ status = false;
+ }
+ }
else
fclose(pset.queryFout);
}
@@ -111,7 +129,7 @@ setQFout(const char *fname)
set_sigpipe_trap_state(is_pipe);
restore_sigpipe_trap();
- return true;
+ return status;
}
@@ -689,7 +707,22 @@ PrintQueryTuples(const PGresult *result, const printQueryOpt *opt, FILE *printQu
if (is_pipe)
{
- pclose(fout);
+ int pclose_rc = pclose(fout);
+ if (pclose_rc != 0)
+ {
+ if (pclose_rc < 0)
+ pg_log_error("could not close pipe to external command: %m");
+ else
+ {
+ char *reason = wait_result_to_str(pclose_rc);
+
+ pg_log_error("%s: %s", pset.gfname + 1,
+ reason ? reason : "");
+ free(reason);
+ }
+ if (pset.on_error_stop == PSQL_ERROR_STOP_SHELL || pset.on_error_stop == PSQL_ERROR_STOP_ALL)
+ ok = false;
+ }
restore_sigpipe_trap();
}
else
@@ -1429,7 +1462,7 @@ DescribeQuery(const char *query, double *elapsed_msec)
* For other commands, the results are processed normally, depending on their
* status.
*
- * Returns 1 on complete success, 0 on interrupt and -1 or errors. Possible
+ * Returns 1 on complete success, 0 on interrupt and -1 on errors. Possible
* failure modes include purely client-side problems; check the transaction
* status for the server-side opinion.
*
diff --git a/src/bin/psql/psqlscanslash.l b/src/bin/psql/psqlscanslash.l
index a467b72144..8e5f5b5010 100644
--- a/src/bin/psql/psqlscanslash.l
+++ b/src/bin/psql/psqlscanslash.l
@@ -55,6 +55,10 @@ static int backtick_start_offset;
#define LEXRES_OK 1 /* OK completion of backslash argument */
+/* command execution in evaluate_backtick() results in error*/
+#define CMD_ERR -1
+
+
static void evaluate_backtick(PsqlScanState state);
#define ECHO psqlscan_emit(cur_state, yytext, yyleng)
@@ -800,10 +804,23 @@ evaluate_backtick(PsqlScanState state)
} while (!feof(fd));
}
- if (fd && pclose(fd) == -1)
+ if (fd)
{
- pg_log_error("%s: %m", cmd);
- error = true;
+ int pclose_rc = pclose(fd);
+ if (pclose_rc != 0)
+ {
+ if (pclose_rc < 0)
+ pg_log_error("%s: %m", cmd);
+ else
+ {
+ char *reason = wait_result_to_str(pclose_rc);
+
+ pg_log_error("%s: %s", cmd,
+ reason ? reason : "");
+ free(reason);
+ }
+ error = true;
+ }
}
if (PQExpBufferDataBroken(cmd_output))
@@ -825,6 +842,12 @@ evaluate_backtick(PsqlScanState state)
cmd_output.len--;
appendBinaryPQExpBuffer(output_buf, cmd_output.data, cmd_output.len);
}
+ /* If error, set len to a value greater than or equal
+ * to maxlen to indicate error in command execution.
+ * This can help with ON_ERROR_STOP.
+ */
+ else
+ output_buf->len = CMD_ERR;
termPQExpBuffer(&cmd_output);
}
diff --git a/src/bin/psql/settings.h b/src/bin/psql/settings.h
index 2399cffa3f..54e7aebddb 100644
--- a/src/bin/psql/settings.h
+++ b/src/bin/psql/settings.h
@@ -54,6 +54,14 @@ typedef enum
PSQL_ERROR_ROLLBACK_ON
} PSQL_ERROR_ROLLBACK;
+typedef enum
+{
+ PSQL_ERROR_STOP_OFF,
+ PSQL_ERROR_STOP_ON,
+ PSQL_ERROR_STOP_SHELL,
+ PSQL_ERROR_STOP_ALL
+} PSQL_ERROR_STOP;
+
typedef enum
{
PSQL_COMP_CASE_PRESERVE_UPPER,
@@ -130,7 +138,6 @@ typedef struct _psqlSettings
* functions.
*/
bool autocommit;
- bool on_error_stop;
bool quiet;
bool singleline;
bool singlestep;
@@ -142,6 +149,7 @@ typedef struct _psqlSettings
PSQL_ECHO echo;
PSQL_ECHO_HIDDEN echo_hidden;
PSQL_ERROR_ROLLBACK on_error_rollback;
+ PSQL_ERROR_STOP on_error_stop;
PSQL_COMP_CASE comp_case;
HistControl histcontrol;
const char *prompt1;
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
index f5b9e268f2..cb3fa89400 100644
--- a/src/bin/psql/startup.c
+++ b/src/bin/psql/startup.c
@@ -877,12 +877,6 @@ autocommit_hook(const char *newval)
return ParseVariableBool(newval, "AUTOCOMMIT", &pset.autocommit);
}
-static bool
-on_error_stop_hook(const char *newval)
-{
- return ParseVariableBool(newval, "ON_ERROR_STOP", &pset.on_error_stop);
-}
-
static bool
quiet_hook(const char *newval)
{
@@ -1036,6 +1030,29 @@ on_error_rollback_hook(const char *newval)
return true;
}
+static bool
+on_error_stop_hook(const char *newval)
+{
+ Assert(newval != NULL); /* else substitute hook messed up */
+ if (pg_strcasecmp(newval, "shell") == 0)
+ pset.on_error_stop = PSQL_ERROR_STOP_SHELL;
+ else if (pg_strcasecmp(newval, "all") == 0)
+ pset.on_error_stop = PSQL_ERROR_STOP_ALL;
+ else
+ {
+ bool on_off;
+
+ if (ParseVariableBool(newval, NULL, &on_off))
+ pset.on_error_stop = on_off ? PSQL_ERROR_STOP_ON : PSQL_ERROR_STOP_OFF;
+ else
+ {
+ PsqlVarEnumError("ON_ERROR_STOP", newval, "on, off, shell, all");
+ return false;
+ }
+ }
+ return true;
+}
+
static char *
comp_keyword_case_substitute_hook(char *newval)
{
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Make ON_ERROR_STOP stop on shell script failure
2022-09-16 06:55 Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-09-16 08:30 ` Re: Make ON_ERROR_STOP stop on shell script failure Kyotaro Horiguchi <[email protected]>
2022-09-17 00:44 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-09-20 06:15 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-09-21 02:45 ` Re: Make ON_ERROR_STOP stop on shell script failure Fujii Masao <[email protected]>
2022-09-30 07:54 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
@ 2022-10-07 08:16 ` Fujii Masao <[email protected]>
2022-10-07 10:41 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
0 siblings, 1 reply; 20+ messages in thread
From: Fujii Masao @ 2022-10-07 08:16 UTC (permalink / raw)
To: bt22nakamorit <[email protected]>; +Cc: Kyotaro Horiguchi <[email protected]>; [email protected]
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 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.
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
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Make ON_ERROR_STOP stop on shell script failure
2022-09-16 06:55 Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-09-16 08:30 ` Re: Make ON_ERROR_STOP stop on shell script failure Kyotaro Horiguchi <[email protected]>
2022-09-17 00:44 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-09-20 06:15 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-09-21 02:45 ` Re: Make ON_ERROR_STOP stop on shell script failure Fujii Masao <[email protected]>
2022-09-30 07:54 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-10-07 08:16 ` Re: Make ON_ERROR_STOP stop on shell script failure Fujii Masao <[email protected]>
@ 2022-10-07 10:41 ` bt22nakamorit <[email protected]>
2022-10-12 09:13 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
0 siblings, 1 reply; 20+ messages in thread
From: bt22nakamorit @ 2022-10-07 10:41 UTC (permalink / raw)
To: Fujii Masao <[email protected]>; +Cc: Kyotaro Horiguchi <[email protected]>; [email protected]
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
Attachments:
[text/x-diff] stop_error3.patch (9.4K, ../../[email protected]/2-stop_error3.patch)
download | inline diff:
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 9494f28063..82febf0ace 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -4143,7 +4143,12 @@ bar
<para>
By default, command processing continues after an error. When this
variable is set to <literal>on</literal>, processing will instead stop
- immediately. In interactive mode,
+ immediately upon an error in SQL query. When this variable is set to
+ <literal>shell</literal>, a nonzero exit status of a shell command,
+ which indicates failure, is interpreted as an error that stops the processing.
+ When this variable is set to <literal>all</literal>, errors from both
+ SQL queries and shell commands can stop the processing.
+ In interactive mode,
<application>psql</application> will return to the command prompt;
otherwise, <application>psql</application> will exit, returning
error code 3 to distinguish this case from fatal error
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index ab613dd49e..cc7ca27e3a 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -34,6 +34,7 @@
#include "describe.h"
#include "fe_utils/cancel.h"
#include "fe_utils/print.h"
+#include "fe_utils/psqlscan_int.h"
#include "fe_utils/string_utils.h"
#include "help.h"
#include "input.h"
@@ -2355,9 +2356,13 @@ exec_command_set(PsqlScanState scan_state, bool active_branch)
*/
char *newval;
char *opt;
+ PQExpBuffer output_buf = scan_state->output_buf;
opt = psql_scan_slash_option(scan_state,
OT_NORMAL, NULL, false);
+ if (output_buf->len >= output_buf->maxlen
+ && (pset.on_error_stop == PSQL_ERROR_STOP_SHELL || pset.on_error_stop == PSQL_ERROR_STOP_ALL))
+ success = false;
newval = pg_strdup(opt ? opt : "");
free(opt);
@@ -2693,8 +2698,25 @@ exec_command_write(PsqlScanState scan_state, bool active_branch,
else if (previous_buf && previous_buf->len > 0)
fprintf(fd, "%s\n", previous_buf->data);
- if (is_pipe)
+ if (is_pipe) {
result = pclose(fd);
+
+ if (result != 0)
+ {
+ if (result < 0)
+ pg_log_error("could not close pipe to external command: %m");
+ else
+ {
+ char *reason = wait_result_to_str(result);
+
+ pg_log_error("%s: %s", fname + 1,
+ reason ? reason : "");
+ free(reason);
+ }
+ }
+ if (pset.on_error_stop == PSQL_ERROR_STOP_SHELL || pset.on_error_stop == PSQL_ERROR_STOP_ALL)
+ status = PSQL_CMD_ERROR;
+ }
else
result = fclose(fd);
@@ -4984,10 +5006,19 @@ do_shell(const char *command)
else
result = system(command);
- if (result == 127 || result == -1)
+ if (result != 0)
{
- pg_log_error("\\!: failed");
- return false;
+ if (result < 0)
+ pg_log_error("could not close pipe to external command: %m");
+ else
+ {
+ char *reason = wait_result_to_str(result);
+
+ pg_log_error("%s: %s", command, reason ? reason : "");
+ free(reason);
+ }
+ if (pset.on_error_stop == PSQL_ERROR_STOP_SHELL || pset.on_error_stop == PSQL_ERROR_STOP_ALL)
+ return false;
}
return true;
}
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index 4f310a8019..966cd34d23 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -94,6 +94,7 @@ setQFout(const char *fname)
{
FILE *fout;
bool is_pipe;
+ bool status = true;
/* First make sure we can open the new output file/pipe */
if (!openQueryOutputFile(fname, &fout, &is_pipe))
@@ -103,7 +104,24 @@ setQFout(const char *fname)
if (pset.queryFout && pset.queryFout != stdout && pset.queryFout != stderr)
{
if (pset.queryFoutPipe)
- pclose(pset.queryFout);
+ {
+ int pclose_rc = pclose(pset.queryFout);
+ if (pclose_rc != 0)
+ {
+ if (pclose_rc < 0)
+ pg_log_error("could not close pipe to external command: %m");
+ else
+ {
+ char *reason = wait_result_to_str(pclose_rc);
+
+ pg_log_error("command failure: %s",
+ reason ? reason : "");
+ free(reason);
+ }
+ if ((pset.on_error_stop == PSQL_ERROR_STOP_SHELL || pset.on_error_stop == PSQL_ERROR_STOP_ALL))
+ status = false;
+ }
+ }
else
fclose(pset.queryFout);
}
@@ -115,7 +133,7 @@ setQFout(const char *fname)
set_sigpipe_trap_state(is_pipe);
restore_sigpipe_trap();
- return true;
+ return status;
}
@@ -1373,7 +1391,7 @@ DescribeQuery(const char *query, double *elapsed_msec)
* For other commands, the results are processed normally, depending on their
* status.
*
- * Returns 1 on complete success, 0 on interrupt and -1 or errors. Possible
+ * Returns 1 on complete success, 0 on interrupt and -1 on errors. Possible
* failure modes include purely client-side problems; check the transaction
* status for the server-side opinion.
*
@@ -1635,7 +1653,22 @@ ExecQueryAndProcessResults(const char *query,
{
if (gfile_is_pipe)
{
- pclose(gfile_fout);
+ int pclose_rc = pclose(gfile_fout);
+ if (pclose_rc != 0)
+ {
+ if (pclose_rc < 0)
+ pg_log_error("could not close pipe to external command: %m");
+ else
+ {
+ char *reason = wait_result_to_str(pclose_rc);
+
+ pg_log_error("%s: %s", pset.gfname + 1,
+ reason ? reason : "");
+ free(reason);
+ }
+ if (pset.on_error_stop == PSQL_ERROR_STOP_SHELL || pset.on_error_stop == PSQL_ERROR_STOP_ALL)
+ success = false;
+ }
restore_sigpipe_trap();
}
else
@@ -1851,7 +1884,22 @@ ExecQueryUsingCursor(const char *query, double *elapsed_msec)
/* close \g argument file/pipe */
if (is_pipe)
{
- pclose(fout);
+ int pclose_rc = pclose(fout);
+ if (pclose_rc != 0)
+ {
+ if (pclose_rc < 0)
+ pg_log_error("could not close pipe to external command: %m");
+ else
+ {
+ char *reason = wait_result_to_str(pclose_rc);
+
+ pg_log_error("%s: %s", pset.gfname + 1,
+ reason ? reason : "");
+ free(reason);
+ }
+ if (pset.on_error_stop == PSQL_ERROR_STOP_SHELL || pset.on_error_stop == PSQL_ERROR_STOP_ALL)
+ OK = false;
+ }
restore_sigpipe_trap();
}
else
diff --git a/src/bin/psql/psqlscanslash.l b/src/bin/psql/psqlscanslash.l
index a467b72144..8e5f5b5010 100644
--- a/src/bin/psql/psqlscanslash.l
+++ b/src/bin/psql/psqlscanslash.l
@@ -55,6 +55,10 @@ static int backtick_start_offset;
#define LEXRES_OK 1 /* OK completion of backslash argument */
+/* command execution in evaluate_backtick() results in error*/
+#define CMD_ERR -1
+
+
static void evaluate_backtick(PsqlScanState state);
#define ECHO psqlscan_emit(cur_state, yytext, yyleng)
@@ -800,10 +804,23 @@ evaluate_backtick(PsqlScanState state)
} while (!feof(fd));
}
- if (fd && pclose(fd) == -1)
+ if (fd)
{
- pg_log_error("%s: %m", cmd);
- error = true;
+ int pclose_rc = pclose(fd);
+ if (pclose_rc != 0)
+ {
+ if (pclose_rc < 0)
+ pg_log_error("%s: %m", cmd);
+ else
+ {
+ char *reason = wait_result_to_str(pclose_rc);
+
+ pg_log_error("%s: %s", cmd,
+ reason ? reason : "");
+ free(reason);
+ }
+ error = true;
+ }
}
if (PQExpBufferDataBroken(cmd_output))
@@ -825,6 +842,12 @@ evaluate_backtick(PsqlScanState state)
cmd_output.len--;
appendBinaryPQExpBuffer(output_buf, cmd_output.data, cmd_output.len);
}
+ /* If error, set len to a value greater than or equal
+ * to maxlen to indicate error in command execution.
+ * This can help with ON_ERROR_STOP.
+ */
+ else
+ output_buf->len = CMD_ERR;
termPQExpBuffer(&cmd_output);
}
diff --git a/src/bin/psql/settings.h b/src/bin/psql/settings.h
index 2399cffa3f..54e7aebddb 100644
--- a/src/bin/psql/settings.h
+++ b/src/bin/psql/settings.h
@@ -54,6 +54,14 @@ typedef enum
PSQL_ERROR_ROLLBACK_ON
} PSQL_ERROR_ROLLBACK;
+typedef enum
+{
+ PSQL_ERROR_STOP_OFF,
+ PSQL_ERROR_STOP_ON,
+ PSQL_ERROR_STOP_SHELL,
+ PSQL_ERROR_STOP_ALL
+} PSQL_ERROR_STOP;
+
typedef enum
{
PSQL_COMP_CASE_PRESERVE_UPPER,
@@ -130,7 +138,6 @@ typedef struct _psqlSettings
* functions.
*/
bool autocommit;
- bool on_error_stop;
bool quiet;
bool singleline;
bool singlestep;
@@ -142,6 +149,7 @@ typedef struct _psqlSettings
PSQL_ECHO echo;
PSQL_ECHO_HIDDEN echo_hidden;
PSQL_ERROR_ROLLBACK on_error_rollback;
+ PSQL_ERROR_STOP on_error_stop;
PSQL_COMP_CASE comp_case;
HistControl histcontrol;
const char *prompt1;
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
index f5b9e268f2..cb3fa89400 100644
--- a/src/bin/psql/startup.c
+++ b/src/bin/psql/startup.c
@@ -877,12 +877,6 @@ autocommit_hook(const char *newval)
return ParseVariableBool(newval, "AUTOCOMMIT", &pset.autocommit);
}
-static bool
-on_error_stop_hook(const char *newval)
-{
- return ParseVariableBool(newval, "ON_ERROR_STOP", &pset.on_error_stop);
-}
-
static bool
quiet_hook(const char *newval)
{
@@ -1036,6 +1030,29 @@ on_error_rollback_hook(const char *newval)
return true;
}
+static bool
+on_error_stop_hook(const char *newval)
+{
+ Assert(newval != NULL); /* else substitute hook messed up */
+ if (pg_strcasecmp(newval, "shell") == 0)
+ pset.on_error_stop = PSQL_ERROR_STOP_SHELL;
+ else if (pg_strcasecmp(newval, "all") == 0)
+ pset.on_error_stop = PSQL_ERROR_STOP_ALL;
+ else
+ {
+ bool on_off;
+
+ if (ParseVariableBool(newval, NULL, &on_off))
+ pset.on_error_stop = on_off ? PSQL_ERROR_STOP_ON : PSQL_ERROR_STOP_OFF;
+ else
+ {
+ PsqlVarEnumError("ON_ERROR_STOP", newval, "on, off, shell, all");
+ return false;
+ }
+ }
+ return true;
+}
+
static char *
comp_keyword_case_substitute_hook(char *newval)
{
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Make ON_ERROR_STOP stop on shell script failure
2022-09-16 06:55 Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-09-16 08:30 ` Re: Make ON_ERROR_STOP stop on shell script failure Kyotaro Horiguchi <[email protected]>
2022-09-17 00:44 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-09-20 06:15 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-09-21 02:45 ` Re: Make ON_ERROR_STOP stop on shell script failure Fujii Masao <[email protected]>
2022-09-30 07:54 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-10-07 08:16 ` Re: Make ON_ERROR_STOP stop on shell script failure Fujii Masao <[email protected]>
2022-10-07 10:41 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
@ 2022-10-12 09:13 ` bt22nakamorit <[email protected]>
2022-11-22 23:16 ` Re: Make ON_ERROR_STOP stop on shell script failure Matheus Alcantara <[email protected]>
2022-12-14 00:40 ` Re: Make ON_ERROR_STOP stop on shell script failure David Zhang <[email protected]>
0 siblings, 2 replies; 20+ messages in thread
From: bt22nakamorit @ 2022-10-12 09:13 UTC (permalink / raw)
To: Fujii Masao <[email protected]>; +Cc: Kyotaro Horiguchi <[email protected]>; [email protected]
There was a mistake in the error message for \! so I updated the patch.
Best,
Tatsuhiro Nakamori
Attachments:
[text/x-diff] stop_error4.patch (9.4K, ../../[email protected]/2-stop_error4.patch)
download | inline diff:
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 9494f28063..82febf0ace 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -4143,7 +4143,12 @@ bar
<para>
By default, command processing continues after an error. When this
variable is set to <literal>on</literal>, processing will instead stop
- immediately. In interactive mode,
+ immediately upon an error in SQL query. When this variable is set to
+ <literal>shell</literal>, a nonzero exit status of a shell command,
+ which indicates failure, is interpreted as an error that stops the processing.
+ When this variable is set to <literal>all</literal>, errors from both
+ SQL queries and shell commands can stop the processing.
+ In interactive mode,
<application>psql</application> will return to the command prompt;
otherwise, <application>psql</application> will exit, returning
error code 3 to distinguish this case from fatal error
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index ab613dd49e..2a4086893e 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -34,6 +34,7 @@
#include "describe.h"
#include "fe_utils/cancel.h"
#include "fe_utils/print.h"
+#include "fe_utils/psqlscan_int.h"
#include "fe_utils/string_utils.h"
#include "help.h"
#include "input.h"
@@ -2355,9 +2356,13 @@ exec_command_set(PsqlScanState scan_state, bool active_branch)
*/
char *newval;
char *opt;
+ PQExpBuffer output_buf = scan_state->output_buf;
opt = psql_scan_slash_option(scan_state,
OT_NORMAL, NULL, false);
+ if (output_buf->len >= output_buf->maxlen
+ && (pset.on_error_stop == PSQL_ERROR_STOP_SHELL || pset.on_error_stop == PSQL_ERROR_STOP_ALL))
+ success = false;
newval = pg_strdup(opt ? opt : "");
free(opt);
@@ -2693,8 +2698,25 @@ exec_command_write(PsqlScanState scan_state, bool active_branch,
else if (previous_buf && previous_buf->len > 0)
fprintf(fd, "%s\n", previous_buf->data);
- if (is_pipe)
+ if (is_pipe) {
result = pclose(fd);
+
+ if (result != 0)
+ {
+ if (result < 0)
+ pg_log_error("could not close pipe to external command: %m");
+ else
+ {
+ char *reason = wait_result_to_str(result);
+
+ pg_log_error("%s: %s", fname + 1,
+ reason ? reason : "");
+ free(reason);
+ }
+ }
+ if (pset.on_error_stop == PSQL_ERROR_STOP_SHELL || pset.on_error_stop == PSQL_ERROR_STOP_ALL)
+ status = PSQL_CMD_ERROR;
+ }
else
result = fclose(fd);
@@ -4984,10 +5006,19 @@ do_shell(const char *command)
else
result = system(command);
- if (result == 127 || result == -1)
+ if (result != 0)
{
- pg_log_error("\\!: failed");
- return false;
+ if (result < 0)
+ pg_log_error("could not execute command: %m");
+ else
+ {
+ char *reason = wait_result_to_str(result);
+
+ pg_log_error("%s: %s", command, reason ? reason : "");
+ free(reason);
+ }
+ if (pset.on_error_stop == PSQL_ERROR_STOP_SHELL || pset.on_error_stop == PSQL_ERROR_STOP_ALL)
+ return false;
}
return true;
}
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index 4f310a8019..966cd34d23 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -94,6 +94,7 @@ setQFout(const char *fname)
{
FILE *fout;
bool is_pipe;
+ bool status = true;
/* First make sure we can open the new output file/pipe */
if (!openQueryOutputFile(fname, &fout, &is_pipe))
@@ -103,7 +104,24 @@ setQFout(const char *fname)
if (pset.queryFout && pset.queryFout != stdout && pset.queryFout != stderr)
{
if (pset.queryFoutPipe)
- pclose(pset.queryFout);
+ {
+ int pclose_rc = pclose(pset.queryFout);
+ if (pclose_rc != 0)
+ {
+ if (pclose_rc < 0)
+ pg_log_error("could not close pipe to external command: %m");
+ else
+ {
+ char *reason = wait_result_to_str(pclose_rc);
+
+ pg_log_error("command failure: %s",
+ reason ? reason : "");
+ free(reason);
+ }
+ if ((pset.on_error_stop == PSQL_ERROR_STOP_SHELL || pset.on_error_stop == PSQL_ERROR_STOP_ALL))
+ status = false;
+ }
+ }
else
fclose(pset.queryFout);
}
@@ -115,7 +133,7 @@ setQFout(const char *fname)
set_sigpipe_trap_state(is_pipe);
restore_sigpipe_trap();
- return true;
+ return status;
}
@@ -1373,7 +1391,7 @@ DescribeQuery(const char *query, double *elapsed_msec)
* For other commands, the results are processed normally, depending on their
* status.
*
- * Returns 1 on complete success, 0 on interrupt and -1 or errors. Possible
+ * Returns 1 on complete success, 0 on interrupt and -1 on errors. Possible
* failure modes include purely client-side problems; check the transaction
* status for the server-side opinion.
*
@@ -1635,7 +1653,22 @@ ExecQueryAndProcessResults(const char *query,
{
if (gfile_is_pipe)
{
- pclose(gfile_fout);
+ int pclose_rc = pclose(gfile_fout);
+ if (pclose_rc != 0)
+ {
+ if (pclose_rc < 0)
+ pg_log_error("could not close pipe to external command: %m");
+ else
+ {
+ char *reason = wait_result_to_str(pclose_rc);
+
+ pg_log_error("%s: %s", pset.gfname + 1,
+ reason ? reason : "");
+ free(reason);
+ }
+ if (pset.on_error_stop == PSQL_ERROR_STOP_SHELL || pset.on_error_stop == PSQL_ERROR_STOP_ALL)
+ success = false;
+ }
restore_sigpipe_trap();
}
else
@@ -1851,7 +1884,22 @@ ExecQueryUsingCursor(const char *query, double *elapsed_msec)
/* close \g argument file/pipe */
if (is_pipe)
{
- pclose(fout);
+ int pclose_rc = pclose(fout);
+ if (pclose_rc != 0)
+ {
+ if (pclose_rc < 0)
+ pg_log_error("could not close pipe to external command: %m");
+ else
+ {
+ char *reason = wait_result_to_str(pclose_rc);
+
+ pg_log_error("%s: %s", pset.gfname + 1,
+ reason ? reason : "");
+ free(reason);
+ }
+ if (pset.on_error_stop == PSQL_ERROR_STOP_SHELL || pset.on_error_stop == PSQL_ERROR_STOP_ALL)
+ OK = false;
+ }
restore_sigpipe_trap();
}
else
diff --git a/src/bin/psql/psqlscanslash.l b/src/bin/psql/psqlscanslash.l
index a467b72144..8e5f5b5010 100644
--- a/src/bin/psql/psqlscanslash.l
+++ b/src/bin/psql/psqlscanslash.l
@@ -55,6 +55,10 @@ static int backtick_start_offset;
#define LEXRES_OK 1 /* OK completion of backslash argument */
+/* command execution in evaluate_backtick() results in error*/
+#define CMD_ERR -1
+
+
static void evaluate_backtick(PsqlScanState state);
#define ECHO psqlscan_emit(cur_state, yytext, yyleng)
@@ -800,10 +804,23 @@ evaluate_backtick(PsqlScanState state)
} while (!feof(fd));
}
- if (fd && pclose(fd) == -1)
+ if (fd)
{
- pg_log_error("%s: %m", cmd);
- error = true;
+ int pclose_rc = pclose(fd);
+ if (pclose_rc != 0)
+ {
+ if (pclose_rc < 0)
+ pg_log_error("%s: %m", cmd);
+ else
+ {
+ char *reason = wait_result_to_str(pclose_rc);
+
+ pg_log_error("%s: %s", cmd,
+ reason ? reason : "");
+ free(reason);
+ }
+ error = true;
+ }
}
if (PQExpBufferDataBroken(cmd_output))
@@ -825,6 +842,12 @@ evaluate_backtick(PsqlScanState state)
cmd_output.len--;
appendBinaryPQExpBuffer(output_buf, cmd_output.data, cmd_output.len);
}
+ /* If error, set len to a value greater than or equal
+ * to maxlen to indicate error in command execution.
+ * This can help with ON_ERROR_STOP.
+ */
+ else
+ output_buf->len = CMD_ERR;
termPQExpBuffer(&cmd_output);
}
diff --git a/src/bin/psql/settings.h b/src/bin/psql/settings.h
index 2399cffa3f..54e7aebddb 100644
--- a/src/bin/psql/settings.h
+++ b/src/bin/psql/settings.h
@@ -54,6 +54,14 @@ typedef enum
PSQL_ERROR_ROLLBACK_ON
} PSQL_ERROR_ROLLBACK;
+typedef enum
+{
+ PSQL_ERROR_STOP_OFF,
+ PSQL_ERROR_STOP_ON,
+ PSQL_ERROR_STOP_SHELL,
+ PSQL_ERROR_STOP_ALL
+} PSQL_ERROR_STOP;
+
typedef enum
{
PSQL_COMP_CASE_PRESERVE_UPPER,
@@ -130,7 +138,6 @@ typedef struct _psqlSettings
* functions.
*/
bool autocommit;
- bool on_error_stop;
bool quiet;
bool singleline;
bool singlestep;
@@ -142,6 +149,7 @@ typedef struct _psqlSettings
PSQL_ECHO echo;
PSQL_ECHO_HIDDEN echo_hidden;
PSQL_ERROR_ROLLBACK on_error_rollback;
+ PSQL_ERROR_STOP on_error_stop;
PSQL_COMP_CASE comp_case;
HistControl histcontrol;
const char *prompt1;
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
index f5b9e268f2..cb3fa89400 100644
--- a/src/bin/psql/startup.c
+++ b/src/bin/psql/startup.c
@@ -877,12 +877,6 @@ autocommit_hook(const char *newval)
return ParseVariableBool(newval, "AUTOCOMMIT", &pset.autocommit);
}
-static bool
-on_error_stop_hook(const char *newval)
-{
- return ParseVariableBool(newval, "ON_ERROR_STOP", &pset.on_error_stop);
-}
-
static bool
quiet_hook(const char *newval)
{
@@ -1036,6 +1030,29 @@ on_error_rollback_hook(const char *newval)
return true;
}
+static bool
+on_error_stop_hook(const char *newval)
+{
+ Assert(newval != NULL); /* else substitute hook messed up */
+ if (pg_strcasecmp(newval, "shell") == 0)
+ pset.on_error_stop = PSQL_ERROR_STOP_SHELL;
+ else if (pg_strcasecmp(newval, "all") == 0)
+ pset.on_error_stop = PSQL_ERROR_STOP_ALL;
+ else
+ {
+ bool on_off;
+
+ if (ParseVariableBool(newval, NULL, &on_off))
+ pset.on_error_stop = on_off ? PSQL_ERROR_STOP_ON : PSQL_ERROR_STOP_OFF;
+ else
+ {
+ PsqlVarEnumError("ON_ERROR_STOP", newval, "on, off, shell, all");
+ return false;
+ }
+ }
+ return true;
+}
+
static char *
comp_keyword_case_substitute_hook(char *newval)
{
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Make ON_ERROR_STOP stop on shell script failure
2022-09-16 06:55 Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-09-16 08:30 ` Re: Make ON_ERROR_STOP stop on shell script failure Kyotaro Horiguchi <[email protected]>
2022-09-17 00:44 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-09-20 06:15 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-09-21 02:45 ` Re: Make ON_ERROR_STOP stop on shell script failure Fujii Masao <[email protected]>
2022-09-30 07:54 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-10-07 08:16 ` Re: Make ON_ERROR_STOP stop on shell script failure Fujii Masao <[email protected]>
2022-10-07 10:41 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-10-12 09:13 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
@ 2022-11-22 23:16 ` Matheus Alcantara <[email protected]>
2022-12-04 05:16 ` Re: Make ON_ERROR_STOP stop on shell script failure Corey Huinker <[email protected]>
1 sibling, 1 reply; 20+ messages in thread
From: Matheus Alcantara @ 2022-11-22 23:16 UTC (permalink / raw)
To: bt22nakamorit <[email protected]>; +Cc: Fujii Masao <[email protected]>; Kyotaro Horiguchi <[email protected]>; [email protected]
------- Original Message -------
On Tuesday, November 22nd, 2022 at 20:10, bt22nakamorit <[email protected]> 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
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Make ON_ERROR_STOP stop on shell script failure
2022-09-16 06:55 Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-09-16 08:30 ` Re: Make ON_ERROR_STOP stop on shell script failure Kyotaro Horiguchi <[email protected]>
2022-09-17 00:44 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-09-20 06:15 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-09-21 02:45 ` Re: Make ON_ERROR_STOP stop on shell script failure Fujii Masao <[email protected]>
2022-09-30 07:54 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-10-07 08:16 ` Re: Make ON_ERROR_STOP stop on shell script failure Fujii Masao <[email protected]>
2022-10-07 10:41 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-10-12 09:13 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-11-22 23:16 ` Re: Make ON_ERROR_STOP stop on shell script failure Matheus Alcantara <[email protected]>
@ 2022-12-04 05:16 ` Corey Huinker <[email protected]>
0 siblings, 0 replies; 20+ messages in thread
From: Corey Huinker @ 2022-12-04 05:16 UTC (permalink / raw)
To: Matheus Alcantara <[email protected]>; +Cc: bt22nakamorit <[email protected]>; Fujii Masao <[email protected]>; Kyotaro Horiguchi <[email protected]>; [email protected]
On Tue, Nov 22, 2022 at 6:16 PM Matheus Alcantara <[email protected]> wrote:
> ------- Original Message -------
> On Tuesday, November 22nd, 2022 at 20:10, bt22nakamorit <
> [email protected]> 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.
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Make ON_ERROR_STOP stop on shell script failure
2022-09-16 06:55 Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-09-16 08:30 ` Re: Make ON_ERROR_STOP stop on shell script failure Kyotaro Horiguchi <[email protected]>
2022-09-17 00:44 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-09-20 06:15 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-09-21 02:45 ` Re: Make ON_ERROR_STOP stop on shell script failure Fujii Masao <[email protected]>
2022-09-30 07:54 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-10-07 08:16 ` Re: Make ON_ERROR_STOP stop on shell script failure Fujii Masao <[email protected]>
2022-10-07 10:41 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-10-12 09:13 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
@ 2022-12-14 00:40 ` David Zhang <[email protected]>
1 sibling, 0 replies; 20+ messages in thread
From: David Zhang @ 2022-12-14 00:40 UTC (permalink / raw)
To: bt22nakamorit <[email protected]>; Fujii Masao <[email protected]>; +Cc: Kyotaro Horiguchi <[email protected]>; [email protected]
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
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Make ON_ERROR_STOP stop on shell script failure
2022-09-16 06:55 Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-09-16 08:30 ` Re: Make ON_ERROR_STOP stop on shell script failure Kyotaro Horiguchi <[email protected]>
2022-09-17 00:44 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-09-20 06:15 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
@ 2022-09-28 12:49 ` torikoshia <[email protected]>
2022-09-29 02:29 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
1 sibling, 1 reply; 20+ messages in thread
From: torikoshia @ 2022-09-28 12:49 UTC (permalink / raw)
To: bt22nakamorit <[email protected]>; +Cc: [email protected]
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
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Make ON_ERROR_STOP stop on shell script failure
2022-09-16 06:55 Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-09-16 08:30 ` Re: Make ON_ERROR_STOP stop on shell script failure Kyotaro Horiguchi <[email protected]>
2022-09-17 00:44 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-09-20 06:15 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-09-28 12:49 ` Re: Make ON_ERROR_STOP stop on shell script failure torikoshia <[email protected]>
@ 2022-09-29 02:29 ` bt22nakamorit <[email protected]>
2022-09-29 03:35 ` Re: Make ON_ERROR_STOP stop on shell script failure Kyotaro Horiguchi <[email protected]>
0 siblings, 1 reply; 20+ messages in thread
From: bt22nakamorit @ 2022-09-29 02:29 UTC (permalink / raw)
To: torikoshia <[email protected]>; +Cc: [email protected]
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
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Make ON_ERROR_STOP stop on shell script failure
2022-09-16 06:55 Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-09-16 08:30 ` Re: Make ON_ERROR_STOP stop on shell script failure Kyotaro Horiguchi <[email protected]>
2022-09-17 00:44 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-09-20 06:15 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-09-28 12:49 ` Re: Make ON_ERROR_STOP stop on shell script failure torikoshia <[email protected]>
2022-09-29 02:29 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
@ 2022-09-29 03:35 ` Kyotaro Horiguchi <[email protected]>
2022-09-29 04:51 ` Re: Make ON_ERROR_STOP stop on shell script failure Kyotaro Horiguchi <[email protected]>
0 siblings, 1 reply; 20+ messages in thread
From: Kyotaro Horiguchi @ 2022-09-29 03:35 UTC (permalink / raw)
To: [email protected]; +Cc: [email protected]; [email protected]
At Thu, 29 Sep 2022 11:29:40 +0900, bt22nakamorit <[email protected]> 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
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Make ON_ERROR_STOP stop on shell script failure
2022-09-16 06:55 Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-09-16 08:30 ` Re: Make ON_ERROR_STOP stop on shell script failure Kyotaro Horiguchi <[email protected]>
2022-09-17 00:44 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-09-20 06:15 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-09-28 12:49 ` Re: Make ON_ERROR_STOP stop on shell script failure torikoshia <[email protected]>
2022-09-29 02:29 ` Re: Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-09-29 03:35 ` Re: Make ON_ERROR_STOP stop on shell script failure Kyotaro Horiguchi <[email protected]>
@ 2022-09-29 04:51 ` Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 20+ messages in thread
From: Kyotaro Horiguchi @ 2022-09-29 04:51 UTC (permalink / raw)
To: [email protected]; +Cc: [email protected]; [email protected]
At Thu, 29 Sep 2022 12:35:04 +0900 (JST), Kyotaro Horiguchi <[email protected]> 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
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Make ON_ERROR_STOP stop on shell script failure
2022-09-16 06:55 Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
@ 2022-11-02 11:58 ` Justin Pryzby <[email protected]>
2022-11-04 09:10 ` Re: Make ON_ERROR_STOP stop on shell script failure Corey Huinker <[email protected]>
1 sibling, 1 reply; 20+ messages in thread
From: Justin Pryzby @ 2022-11-02 11:58 UTC (permalink / raw)
To: bt22nakamorit <[email protected]>; +Cc: [email protected]
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
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Make ON_ERROR_STOP stop on shell script failure
2022-09-16 06:55 Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-11-02 11:58 ` Re: Make ON_ERROR_STOP stop on shell script failure Justin Pryzby <[email protected]>
@ 2022-11-04 09:10 ` Corey Huinker <[email protected]>
0 siblings, 0 replies; 20+ messages in thread
From: Corey Huinker @ 2022-11-04 09:10 UTC (permalink / raw)
To: Justin Pryzby <[email protected]>; +Cc: bt22nakamorit <[email protected]>; [email protected]
>
> 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/[email protected]...
so as not to clutter up this one.
^ permalink raw reply [nested|flat] 20+ messages in thread
end of thread, other threads:[~2022-12-14 00:40 UTC | newest]
Thread overview: 20+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2022-09-16 06:55 Make ON_ERROR_STOP stop on shell script failure bt22nakamorit <[email protected]>
2022-09-16 08:30 ` Kyotaro Horiguchi <[email protected]>
2022-09-17 00:44 ` bt22nakamorit <[email protected]>
2022-09-20 06:15 ` bt22nakamorit <[email protected]>
2022-09-21 02:45 ` Fujii Masao <[email protected]>
2022-09-27 03:34 ` Kyotaro Horiguchi <[email protected]>
2022-09-28 06:36 ` [email protected]
2022-09-30 07:54 ` bt22nakamorit <[email protected]>
2022-10-07 08:16 ` Fujii Masao <[email protected]>
2022-10-07 10:41 ` bt22nakamorit <[email protected]>
2022-10-12 09:13 ` bt22nakamorit <[email protected]>
2022-11-22 23:16 ` Matheus Alcantara <[email protected]>
2022-12-04 05:16 ` Corey Huinker <[email protected]>
2022-12-14 00:40 ` David Zhang <[email protected]>
2022-09-28 12:49 ` torikoshia <[email protected]>
2022-09-29 02:29 ` bt22nakamorit <[email protected]>
2022-09-29 03:35 ` Kyotaro Horiguchi <[email protected]>
2022-09-29 04:51 ` Kyotaro Horiguchi <[email protected]>
2022-11-02 11:58 ` Justin Pryzby <[email protected]>
2022-11-04 09:10 ` Corey Huinker <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox