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
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)
{
