On Fri, Mar 10, 2023 at 3:49 PM Tom Lane <t...@sss.pgh.pa.us> wrote:
> I'm okay with adopting bash's rule, but then it should actually match
> bash --- signal N is reported as 128+N, not -N.
>
128+N is implemented.
Nonzero pclose errors of any kind will now overwrite an existing exit_code.
I didn't write a formal test for the signals, but instead tested it
manually:
[310444:16:54:32 EDT] corey=# \! sleep 1000
-- in another window, i found the pid of the sleep command and did a kill
-9 on it
[310444:16:55:15 EDT] corey=# \echo :SHELL_EXIT_CODE
137
137 = 128+9, so that checks out.
The OS-specific regression test has been modified. On Windows systems it
attempts to execute "CON", and on other systems it attempts to execute "/".
These are marginally better than "nosuchcommand" in that they should always
exist on their host OS and could never be a legit executable. I have no
opinion whether the test should live on past the development phase, but if
it does not then the 0001 patch is not needed.
From a5dddedcc7bf654b4b65c14ff7b89b9d83bb5c27 Mon Sep 17 00:00:00 2001
From: coreyhuinker <corey.huin...@gmail.com>
Date: Fri, 17 Mar 2023 06:43:24 -0400
Subject: [PATCH v9 1/3] Add PG_OS_TARGET environment variable to enable
OS-specific changes to regression tests.
---
src/test/regress/pg_regress.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 7b23cc80dc..333aaab139 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -594,6 +594,17 @@ initialize_environment(void)
#endif
}
+ /*
+ * Regression tests that invoke OS commands must occasionally use
+ * OS-specific syntax (example: NUL vs /dev/null). This env var allows
+ * those tests to adjust commands accordingly.
+ */
+#if defined(WIN32)
+ setenv("PG_OS_TARGET", "WIN32", 1);
+#else
+ setenv("PG_OS_TARGET", "OTHER", 1);
+#endif
+
/*
* Set translation-related settings to English; otherwise psql will
* produce translated messages and produce diffs. (XXX If we ever support
--
2.39.2
From fe578661eef7dbe33aec8f4eaa6dca80a1304c9b Mon Sep 17 00:00:00 2001
From: coreyhuinker <corey.huin...@gmail.com>
Date: Fri, 17 Mar 2023 06:44:43 -0400
Subject: [PATCH v9 2/3] Add wait_result_to_exit_code().
---
src/common/wait_error.c | 15 +++++++++++++++
src/include/port.h | 1 +
2 files changed, 16 insertions(+)
diff --git a/src/common/wait_error.c b/src/common/wait_error.c
index 4a3c3c61af..8e1c0e3fc8 100644
--- a/src/common/wait_error.c
+++ b/src/common/wait_error.c
@@ -127,3 +127,18 @@ wait_result_is_any_signal(int exit_status, bool include_command_not_found)
return true;
return false;
}
+
+/*
+ * Return the POSIX exit code (0 to 255) that corresponds to the argument.
+ * The argument is a return code returned by wait(2) or waitpid(2), which
+ * also applies to pclose(3) and system(3).
+ */
+int
+wait_result_to_exit_code(int exit_status)
+{
+ if (WIFEXITED(exit_status))
+ return WEXITSTATUS(exit_status);
+ if (WIFSIGNALED(exit_status))
+ return 128 + WTERMSIG(exit_status);
+ return 0;
+}
diff --git a/src/include/port.h b/src/include/port.h
index e66193bed9..1b119a3c56 100644
--- a/src/include/port.h
+++ b/src/include/port.h
@@ -495,6 +495,7 @@ extern char *escape_single_quotes_ascii(const char *src);
extern char *wait_result_to_str(int exitstatus);
extern bool wait_result_is_signal(int exit_status, int signum);
extern bool wait_result_is_any_signal(int exit_status, bool include_command_not_found);
+extern int wait_result_to_exit_code(int exit_status);
/*
* Interfaces that we assume all Unix system have. We retain individual macros
--
2.39.2
From 251863ddcbc4eecf4fa15e332724acbd3c652478 Mon Sep 17 00:00:00 2001
From: coreyhuinker <corey.huin...@gmail.com>
Date: Fri, 17 Mar 2023 16:46:57 -0400
Subject: [PATCH v9 3/3] Add psql variables SHELL_ERROR and SHELL_EXIT_CODE
which report the success or failure of the most recent psql OS-command
executed via the \! command or `backticks`. These variables are roughly
analogous to ERROR and SQLSTATE, but for OS level operations instead of SQL
commands.
---
doc/src/sgml/ref/psql-ref.sgml | 21 ++++++++++++++++++
src/bin/psql/command.c | 15 +++++++++++++
src/bin/psql/help.c | 4 ++++
src/bin/psql/psqlscanslash.l | 35 +++++++++++++++++++++++++-----
src/test/regress/expected/psql.out | 34 +++++++++++++++++++++++++++++
src/test/regress/sql/psql.sql | 30 +++++++++++++++++++++++++
6 files changed, 134 insertions(+), 5 deletions(-)
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 7b8ae9fac3..e6e307fd18 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -4267,6 +4267,27 @@ bar
</listitem>
</varlistentry>
+ <varlistentry id="app-psql-variables-shell-error">
+ <term><varname>SHELL_ERROR</varname></term>
+ <listitem>
+ <para>
+ <literal>true</literal> if the last shell command failed, <literal>false</literal> if
+ it succeeded. This applies to shell commands invoked via either <literal>\!</literal>
+ or <literal>`</literal>. See also <varname>SHELL_EXIT_CODE</varname>.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry id="app-psql-variables-exit-code">
+ <term><varname>SHELL_EXIT_CODE</varname></term>
+ <listitem>
+ <para>
+ The exit code return by the last shell command, invoked via either <literal>\!</literal> or <literal>`</literal>.
+ 0 means no error. See also <varname>SHELL_ERROR</varname>.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="app-psql-variables-show-all-results">
<term><varname>SHOW_ALL_RESULTS</varname></term>
<listitem>
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 61ec049f05..aa44d4f7f5 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -5041,6 +5041,21 @@ do_shell(const char *command)
else
result = system(command);
+ if (result == 0)
+ {
+ SetVariable(pset.vars, "SHELL_EXIT_CODE", "0");
+ SetVariable(pset.vars, "SHELL_ERROR", "false");
+ }
+ else
+ {
+ int exit_code = wait_result_to_exit_code(result);
+ char buf[32];
+
+ snprintf(buf, sizeof(buf), "%d", exit_code);
+ SetVariable(pset.vars, "SHELL_EXIT_CODE", buf);
+ SetVariable(pset.vars, "SHELL_ERROR", "true");
+ }
+
if (result == 127 || result == -1)
{
pg_log_error("\\!: failed");
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index e45c4aaca5..6d5226f793 100644
--- a/src/bin/psql/help.c
+++ b/src/bin/psql/help.c
@@ -455,6 +455,10 @@ helpVariables(unsigned short int pager)
" show all results of a combined query (\\;) instead of only the last\n");
HELP0(" SHOW_CONTEXT\n"
" controls display of message context fields [never, errors, always]\n");
+ HELP0(" SHELL_ERROR\n"
+ " true if last shell command failed, else false\n");
+ HELP0(" SHELL_EXIT_CODE\n"
+ " Exit code of the last shell command\n");
HELP0(" SINGLELINE\n"
" if set, end of line terminates SQL commands (same as -S option)\n");
HELP0(" SINGLESTEP\n"
diff --git a/src/bin/psql/psqlscanslash.l b/src/bin/psql/psqlscanslash.l
index 8449ee1a52..1db4d4a82e 100644
--- a/src/bin/psql/psqlscanslash.l
+++ b/src/bin/psql/psqlscanslash.l
@@ -23,6 +23,7 @@
#include "fe_utils/conditional.h"
#include "libpq-fe.h"
+#include "settings.h"
}
%{
@@ -774,6 +775,7 @@ evaluate_backtick(PsqlScanState state)
bool error = false;
char buf[512];
size_t result;
+ int exit_code = 0;
initPQExpBuffer(&cmd_output);
@@ -783,6 +785,7 @@ evaluate_backtick(PsqlScanState state)
{
pg_log_error("%s: %m", cmd);
error = true;
+ exit_code = -1;
}
if (!error)
@@ -790,7 +793,8 @@ evaluate_backtick(PsqlScanState state)
do
{
result = fread(buf, 1, sizeof(buf), fd);
- if (ferror(fd))
+ exit_code = ferror(fd);
+ if (exit_code)
{
pg_log_error("%s: %m", cmd);
error = true;
@@ -800,10 +804,22 @@ evaluate_backtick(PsqlScanState state)
} while (!feof(fd));
}
- if (fd && pclose(fd) == -1)
+ if (fd)
{
- pg_log_error("%s: %m", cmd);
- error = true;
+ int close_exit_code = pclose(fd);
+
+ /*
+ * Any error reported on pclose() takes precedence over a pre-existing
+ * error from during the run.
+ */
+
+ if (close_exit_code)
+ {
+ error = true;
+ exit_code = close_exit_code;
+ if (close_exit_code == -1)
+ pg_log_error("%s: %m", cmd);
+ }
}
if (PQExpBufferDataBroken(cmd_output))
@@ -824,7 +840,16 @@ evaluate_backtick(PsqlScanState state)
cmd_output.data[cmd_output.len - 1] == '\n')
cmd_output.len--;
appendBinaryPQExpBuffer(output_buf, cmd_output.data, cmd_output.len);
+ SetVariable(pset.vars, "SHELL_EXIT_CODE", "0");
+ SetVariable(pset.vars, "SHELL_ERROR", "false");
}
+ else
+ {
+ char exit_code_buf[32];
- termPQExpBuffer(&cmd_output);
+ snprintf(exit_code_buf, sizeof(exit_code_buf), "%d",
+ wait_result_to_exit_code(exit_code));
+ SetVariable(pset.vars, "SHELL_EXIT_CODE", exit_code_buf);
+ SetVariable(pset.vars, "SHELL_ERROR", "true");
+ }
}
diff --git a/src/test/regress/expected/psql.out b/src/test/regress/expected/psql.out
index c00e28361c..f54a8ac481 100644
--- a/src/test/regress/expected/psql.out
+++ b/src/test/regress/expected/psql.out
@@ -1306,6 +1306,40 @@ execute q;
+----+-------------+
deallocate q;
+-- test SHELL_ERROR / SHELL_EXIT_CODE
+\getenv pg_os_target PG_OS_TARGET
+SELECT :'pg_os_target' = 'WIN32' AS windows_target \gset
+\if :windows_target
+ \set bad_cmd 'CON 2> NUL'
+ \set expected_exit_code 1
+\else
+ \set bad_cmd '/ 2> /dev/null'
+ \set expected_exit_code 126
+\endif
+\set bad_shbang '\\! ' :bad_cmd
+:bad_shbang
+\echo :SHELL_ERROR
+true
+\pset format unaligned
+SELECT CASE
+ WHEN :'SHELL_EXIT_CODE' = :'expected_exit_code' THEN 'OK'
+ ELSE :'SHELL_EXIT_CODE'
+END AS exit_code_check;
+exit_code_check|OK
+\set SHELL_ERROR 'clear'
+\set SHELL_EXIT_CODE 'clear'
+\echo :SHELL_ERROR
+clear
+\echo :SHELL_EXIT_CODE
+clear
+\set nosuchvar `:bad_cmd`
+\echo :SHELL_ERROR
+true
+SELECT CASE
+ WHEN :'SHELL_EXIT_CODE' = :'expected_exit_code' THEN 'OK'
+ ELSE :'SHELL_EXIT_CODE'
+END AS exit_code_check;
+exit_code_check|OK
-- test single-line header and data
prepare q as select repeat('x',2*n) as "0123456789abcdef", repeat('y',20-2*n) as "0123456789" from generate_series(1,10) as n;
\pset linestyle ascii
diff --git a/src/test/regress/sql/psql.sql b/src/test/regress/sql/psql.sql
index 961783d6ea..c11739b736 100644
--- a/src/test/regress/sql/psql.sql
+++ b/src/test/regress/sql/psql.sql
@@ -291,6 +291,36 @@ execute q;
deallocate q;
+-- test SHELL_ERROR / SHELL_EXIT_CODE
+\getenv pg_os_target PG_OS_TARGET
+SELECT :'pg_os_target' = 'WIN32' AS windows_target \gset
+\if :windows_target
+ \set bad_cmd 'CON 2> NUL'
+ \set expected_exit_code 1
+\else
+ \set bad_cmd '/ 2> /dev/null'
+ \set expected_exit_code 126
+\endif
+\set bad_shbang '\\! ' :bad_cmd
+
+:bad_shbang
+\echo :SHELL_ERROR
+\pset format unaligned
+SELECT CASE
+ WHEN :'SHELL_EXIT_CODE' = :'expected_exit_code' THEN 'OK'
+ ELSE :'SHELL_EXIT_CODE'
+END AS exit_code_check;
+\set SHELL_ERROR 'clear'
+\set SHELL_EXIT_CODE 'clear'
+\echo :SHELL_ERROR
+\echo :SHELL_EXIT_CODE
+\set nosuchvar `:bad_cmd`
+\echo :SHELL_ERROR
+SELECT CASE
+ WHEN :'SHELL_EXIT_CODE' = :'expected_exit_code' THEN 'OK'
+ ELSE :'SHELL_EXIT_CODE'
+END AS exit_code_check;
+
-- test single-line header and data
prepare q as select repeat('x',2*n) as "0123456789abcdef", repeat('y',20-2*n) as "0123456789" from generate_series(1,10) as n;
--
2.39.2
