public inbox for [email protected]
help / color / mirror / Atom feedFrom: bt22nakamorit <[email protected]>
To: Fujii Masao <[email protected]>
Cc: Kyotaro Horiguchi <[email protected]>
Cc: [email protected]
Subject: Re: Make ON_ERROR_STOP stop on shell script failure
Date: Fri, 30 Sep 2022 16:54:06 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<[email protected]>
<[email protected]>
<[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)
{
view thread (20+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: Make ON_ERROR_STOP stop on shell script failure
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox