Re: Add SHELL_EXIT_CODE to psql

From: Corey Huinker <corey(dot)huinker(at)gmail(dot)com>
To: vignesh C <vignesh21(at)gmail(dot)com>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Add SHELL_EXIT_CODE to psql
Date: 2023-01-04 07:09:23
Message-ID: CADkLM=eywT=EvYK+=NCAavrXBOdqXT+qdd9P2XRyF95AuOhsdw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Jan 3, 2023 at 5:36 AM vignesh C <vignesh21(at)gmail(dot)com> wrote:

> On Wed, 21 Dec 2022 at 11:04, Corey Huinker <corey(dot)huinker(at)gmail(dot)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(dot)huinker(at)gmail(dot)com>
> wrote:
> >>
> >> Rebased. Still waiting on feedback before working on documentation.
> >>
> >> On Fri, Nov 4, 2022 at 5:23 AM Corey Huinker <corey(dot)huinker(at)gmail(dot)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(dot)huinker(at)gmail(dot)com>
> wrote:
> >>>>
> >>>>
> >>>> Over in
> https://www.postgresql.org/message-id/eaf326ad693e74eba068f33a7f518039@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.

Attachment Content-Type Size
v3-0001-Add-PG_OS_TARGET-environment-variable-to-enable-OS-s.patch text/x-patch 1.1 KB
v3-0002-Add-psql-variables-SHELL_ERROR-and-SHELL_EXIT_CODE-w.patch text/x-patch 7.4 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Richard Guo 2023-01-04 07:11:02 Some compiling warnings
Previous Message vignesh C 2023-01-04 07:07:59 Re: [DOCS] Stats views and functions not in order?