On Tue, Jan 3, 2023 at 5:36 AM vignesh C <vignes...@gmail.com> wrote:
> On Wed, 21 Dec 2022 at 11:04, Corey Huinker <corey.huin...@gmail.com> > wrote: > > > > I've rebased and updated the patch to include documentation. > > > > Regression tests have been moved to a separate patchfile because error > messages will vary by OS and configuration, so we probably can't do a > stable regression test, but having them handy at least demonstrates the > feature. > > > > On Sun, Dec 4, 2022 at 12:35 AM Corey Huinker <corey.huin...@gmail.com> > wrote: > >> > >> Rebased. Still waiting on feedback before working on documentation. > >> > >> On Fri, Nov 4, 2022 at 5:23 AM Corey Huinker <corey.huin...@gmail.com> > wrote: > >>> > >>> Oops, that sample output was from a previous run, should have been: > >>> > >>> -- SHELL_EXIT_CODE is undefined > >>> \echo :SHELL_EXIT_CODE > >>> :SHELL_EXIT_CODE > >>> -- bad \! > >>> \! borp > >>> sh: line 1: borp: command not found > >>> \echo :SHELL_EXIT_CODE > >>> 127 > >>> -- bad backtick > >>> \set var `borp` > >>> sh: line 1: borp: command not found > >>> \echo :SHELL_EXIT_CODE > >>> 127 > >>> -- good \! > >>> \! true > >>> \echo :SHELL_EXIT_CODE > >>> 0 > >>> -- play with exit codes > >>> \! exit 4 > >>> \echo :SHELL_EXIT_CODE > >>> 4 > >>> \set var `exit 3` > >>> \echo :SHELL_EXIT_CODE > >>> 3 > >>> > >>> > >>> On Fri, Nov 4, 2022 at 5:08 AM Corey Huinker <corey.huin...@gmail.com> > wrote: > >>>> > >>>> > >>>> Over in > https://www.postgresql.org/message-id/eaf326ad693e74eba068f33a7f518...@oss.nttdata.com > Justin Pryzby suggested that psql might need the ability to capture the > shell exit code. > >>>> > >>>> This is a POC patch that does that, but doesn't touch on the > ON_ERROR_STOP stuff. > >>>> > >>>> I've added some very rudimentary tests, but haven't touched the > documentation, because I strongly suspect that someone will suggest a > better name for the variable. > >>>> > >>>> But basically, it works like this > >>>> > >>>> -- SHELL_EXIT_CODE is undefined > >>>> \echo :SHELL_EXIT_CODE > >>>> :SHELL_EXIT_CODE > >>>> -- bad \! > >>>> \! borp > >>>> sh: line 1: borp: command not found > >>>> \echo :SHELL_EXIT_CODE > >>>> 32512 > >>>> -- bad backtick > >>>> \set var `borp` > >>>> sh: line 1: borp: command not found > >>>> \echo :SHELL_EXIT_CODE > >>>> 127 > >>>> -- good \! > >>>> \! true > >>>> \echo :SHELL_EXIT_CODE > >>>> 0 > >>>> -- play with exit codes > >>>> \! exit 4 > >>>> \echo :SHELL_EXIT_CODE > >>>> 1024 > >>>> \set var `exit 3` > >>>> \echo :SHELL_EXIT_CODE > >>>> 3 > >>>> > >>>> > >>>> Feedback welcome. > > CFBot shows some compilation errors as in [1], please post an updated > version for the same: > [02:35:49.924] psqlscanslash.l: In function ‘evaluate_backtick’: > [02:35:49.924] psqlscanslash.l:822:11: error: implicit declaration of > function ‘WIFSTOPPED’ [-Werror=implicit-function-declaration] > [02:35:49.924] 822 | exit_code=WSTOPSIG(exit_code); > [02:35:49.924] | ^~~~~~~~~~ > [02:35:49.924] psqlscanslash.l:823:14: error: implicit declaration of > function ‘WSTOPSIG’ [-Werror=implicit-function-declaration] > [02:35:49.924] 823 | } > [02:35:49.924] | ^ > [02:35:49.924] cc1: all warnings being treated as errors > [02:35:49.925] make[3]: *** [<builtin>: psqlscanslash.o] Error 1 > > [1] - https://cirrus-ci.com/task/5424476720988160 > > Regards, > Vignesh > Thanks. I had left sys/wait.h out of psqlscanslash. Attached is v3 of this patch, I've made the following changes: 1. pg_regress now creates an environment variable called PG_OS_TARGET, which regression tests can use to manufacture os-specific commands. For our purposes, this allows the regression test to manufacture a shell command that has either "2> /dev/null" or "2> NUL". This seemed the least invasive way to make this possible. If for some reason it becomes useful in general psql scripting, then we can consider promoting it to a regular psql var. 2. There are now two psql variables, SHELL_EXIT_CODE, which has the return code, and SHELL_ERROR, which is a true/false flag based on whether the exit code was nonzero. These variables are the shell result analogues of SQLSTATE and ERROR.
From 55d59e886fdddf341d70b158415a13bfe7c9858f Mon Sep 17 00:00:00 2001 From: coreyhuinker <corey.huin...@gmail.com> Date: Tue, 3 Jan 2023 22:16:20 -0500 Subject: [PATCH 1/2] 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 40e6c231a3..56c59e0b10 100644 --- a/src/test/regress/pg_regress.c +++ b/src/test/regress/pg_regress.c @@ -595,6 +595,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.38.1
From cac8ccfdf0f0489150f9a485823082fe467f030f Mon Sep 17 00:00:00 2001 From: coreyhuinker <corey.huin...@gmail.com> Date: Wed, 4 Jan 2023 01:55:37 -0500 Subject: [PATCH 2/2] 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 | 8 ++++++ src/bin/psql/help.c | 4 +++ src/bin/psql/psqlscanslash.l | 39 ++++++++++++++++++++++++++---- src/test/regress/expected/psql.out | 25 +++++++++++++++++++ src/test/regress/sql/psql.sql | 21 ++++++++++++++++ 6 files changed, 113 insertions(+), 5 deletions(-) diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml index 8a5285da9a..4ad11eeffb 100644 --- a/doc/src/sgml/ref/psql-ref.sgml +++ b/doc/src/sgml/ref/psql-ref.sgml @@ -4260,6 +4260,27 @@ bar </listitem> </varlistentry> + <varlistentry> + <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> + <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> <term><varname>SHOW_ALL_RESULTS</varname></term> <listitem> diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c index 00b89d941b..8e6ed7cbc0 100644 --- a/src/bin/psql/command.c +++ b/src/bin/psql/command.c @@ -4998,6 +4998,7 @@ static bool do_shell(const char *command) { int result; + char exit_code_buf[32]; fflush(NULL); if (!command) @@ -5025,6 +5026,13 @@ do_shell(const char *command) else result = system(command); + snprintf(exit_code_buf, sizeof(exit_code_buf), "%d", WEXITSTATUS(result)); + SetVariable(pset.vars, "SHELL_EXIT_CODE", exit_code_buf); + if (result == 0) + SetVariable(pset.vars, "SHELL_ERROR", "false"); + else + 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..88931bd677 100644 --- a/src/bin/psql/psqlscanslash.l +++ b/src/bin/psql/psqlscanslash.l @@ -23,6 +23,9 @@ #include "fe_utils/conditional.h" #include "libpq-fe.h" +#include "settings.h" +#include "variables.h" +#include <sys/wait.h> } %{ @@ -774,6 +777,8 @@ evaluate_backtick(PsqlScanState state) bool error = false; char buf[512]; size_t result; + int exit_code = 0; + char exit_code_buf[32]; initPQExpBuffer(&cmd_output); @@ -783,6 +788,7 @@ evaluate_backtick(PsqlScanState state) { pg_log_error("%s: %m", cmd); error = true; + exit_code = -1; } if (!error) @@ -790,7 +796,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 +807,25 @@ 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 = 0; + + /* Capture exit code for SHELL_EXIT_CODE */ + close_exit_code = pclose(fd); + if (close_exit_code == -1) + { + pg_log_error("%s: %m", cmd); + error = true; + } + if (WIFEXITED(close_exit_code)) + exit_code=WEXITSTATUS(close_exit_code); + else if(WIFSIGNALED(close_exit_code)) + exit_code=WTERMSIG(close_exit_code); + else if(WIFSTOPPED(close_exit_code)) + exit_code=WSTOPSIG(close_exit_code); + if (exit_code) + error = true; } if (PQExpBufferDataBroken(cmd_output)) @@ -824,7 +846,14 @@ 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_ERROR", "false"); } + else + SetVariable(pset.vars, "SHELL_ERROR", "true"); - termPQExpBuffer(&cmd_output); + /* + snprintf(exit_code_buf, sizeof(exit_code_buf), "%d", WEXITSTATUS(exit_code)); + */ + snprintf(exit_code_buf, sizeof(exit_code_buf), "%d", exit_code); + SetVariable(pset.vars, "SHELL_EXIT_CODE", exit_code_buf); } diff --git a/src/test/regress/expected/psql.out b/src/test/regress/expected/psql.out index 8fc62cebd2..b2addffa70 100644 --- a/src/test/regress/expected/psql.out +++ b/src/test/regress/expected/psql.out @@ -1306,6 +1306,31 @@ execute q; +----+-------------+ deallocate q; +-- test 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 'nosuchcommand 2> NUL' +\else + \set bad_cmd 'nosuchcommand 2> /dev/null' +\endif +\set bad_shbang '\\! ' :bad_cmd +:bad_shbang +\echo :SHELL_EXIT_CODE +127 +\echo :SHELL_ERROR +true +\set SHELL_EXIT_CODE 'clear' +\set SHELL_ERROR 'clear' +\echo :SHELL_EXIT_CODE +clear +\echo :SHELL_ERROR +clear +\set nosuchvar `:bad_cmd` +\echo :SHELL_EXIT_CODE +127 +\echo :SHELL_ERROR +true -- 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 2da9665a19..be86ef6984 100644 --- a/src/test/regress/sql/psql.sql +++ b/src/test/regress/sql/psql.sql @@ -291,6 +291,27 @@ execute q; deallocate q; +-- test 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 'nosuchcommand 2> NUL' +\else + \set bad_cmd 'nosuchcommand 2> /dev/null' +\endif +\set bad_shbang '\\! ' :bad_cmd + +:bad_shbang +\echo :SHELL_EXIT_CODE +\echo :SHELL_ERROR +\set SHELL_EXIT_CODE 'clear' +\set SHELL_ERROR 'clear' +\echo :SHELL_EXIT_CODE +\echo :SHELL_ERROR +\set nosuchvar `:bad_cmd` +\echo :SHELL_EXIT_CODE +\echo :SHELL_ERROR + -- 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.38.1