Corey Huinker writes:
> This is a follow up patch to apply the committed pattern to the various
> piped output commands.
Pushed with some changes:
* You didn't update the documentation.
* I thought this was way too many copies of the same logic. I made a
subroutine to set these variables,
Corey Huinker writes:
> Following up here. This addendum patch clearly isn't as important as
> anything currently trying to make it in before the feature freeze, but I
> think the lack of setting the SHELL_ERROR and SHELL_EXIT_CODE vars on piped
> commands would be viewed as a bug, so I'm hoping
On Fri, Mar 24, 2023 at 5:21 PM Corey Huinker
wrote:
>
>
> On Tue, Mar 21, 2023 at 3:33 PM Corey Huinker
> wrote:
>
>>
>>> As you had it, any nonzero result would prevent backtick substitution.
>>> I'm not really sure how much thought went into the existing behavior,
>>> but I am pretty sure
On Tue, Mar 21, 2023 at 3:33 PM Corey Huinker
wrote:
>
>> As you had it, any nonzero result would prevent backtick substitution.
>> I'm not really sure how much thought went into the existing behavior,
>> but I am pretty sure that it's not part of this patch's charter to
>> change that.
>>
>>
>
>
> As you had it, any nonzero result would prevent backtick substitution.
> I'm not really sure how much thought went into the existing behavior,
> but I am pretty sure that it's not part of this patch's charter to
> change that.
>
> regards, tom lane
>
The changes all
Corey Huinker writes:
> On Mon, Mar 20, 2023 at 1:01 PM Tom Lane wrote:
>> * Why do you have wait_result_to_exit_code defaulting to return 0
>> if it doesn't recognize the code as either WIFEXITED or WIFSIGNALED?
>> That seems pretty misleading; again -1 would be a better idea.
> That makes
On Mon, Mar 20, 2023 at 1:01 PM Tom Lane wrote:
> Corey Huinker writes:
> > 128+N is implemented.
>
> I think this mostly looks OK, but:
>
> * I still say there is no basis whatever for believing that the result
> of ferror() is an exit code, errno code, or anything else with
> significance
Corey Huinker writes:
> 128+N is implemented.
I think this mostly looks OK, but:
* I still say there is no basis whatever for believing that the result
of ferror() is an exit code, errno code, or anything else with
significance beyond zero-or-not. Feeding it to wait_result_to_exit_code
as
I did a look at the patch, and it seems to be in a good condition.
It implements declared functionality with no visible defects.
As for test, I think it is possible to implement "signals" case in tap
tests. But let the actual commiter decide does it worth it or not.
--
Best regards,
Maxim
On Fri, Mar 10, 2023 at 3:49 PM Tom Lane 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
Justin Pryzby writes:
> The exit status is one byte. I think you should define the status
> variable along the lines of:
> - 0 if successful; or,
> - a positive number 1..255 indicating its exit status. or,
> - a negative number N indicating it was terminated by signal -N; or,
> - 256 if an
On Wed, Feb 22, 2023 at 03:04:33PM -0500, Corey Huinker wrote:
> +
> +/*
> + * 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
>
On Thu, Mar 2, 2023 at 1:35 PM Tom Lane wrote:
> Corey Huinker writes:
> > [ v9-0003-Add-psql-variables-SHELL_ERROR-and-SHELL_EXIT_COD.patch ]
>
> I took a brief look through this. I'm on board with the general
> concept, but I think you've spent too much time on an ultimately
> futile attempt
Corey Huinker writes:
> [ v9-0003-Add-psql-variables-SHELL_ERROR-and-SHELL_EXIT_COD.patch ]
I took a brief look through this. I'm on board with the general
concept, but I think you've spent too much time on an ultimately
futile attempt to get a committable test case, and not enough time
on
On Wed, Feb 22, 2023 at 4:18 PM Thomas Munro wrote:
> On Tue, Jan 31, 2023 at 9:23 AM Corey Huinker
> wrote:
> >> Unfortunately, there is a fail in FreeBSD
> https://cirrus-ci.com/task/6466749487382528
> >>
> >> Maybe, this patch is need to be rebased?
> >>
> >
> > That failure is in
On Tue, Jan 31, 2023 at 9:23 AM Corey Huinker wrote:
>> Unfortunately, there is a fail in FreeBSD
>> https://cirrus-ci.com/task/6466749487382528
>>
>> Maybe, this patch is need to be rebased?
>>
>
> That failure is in postgres_fdw, which this code doesn't touch.
>
> I'm not able to get to
>
>
>
>
> The patch set seem to be in a good shape and pretty stable for quite a
> while.
> Could you add one more minor improvement, a new line after variables
> declaration?
>
As you wish. Attached.
>
> After this changes, I think, we make this patch RfC, shall we?
>
>
Fingers crossed.
From
On Mon, 30 Jan 2023 at 23:23, Corey Huinker wrote:
>
>
> I rebased, but there are no code differences.
>
The patch set seem to be in a good shape and pretty stable for quite a
while.
Could you add one more minor improvement, a new line after variables
declaration?
+ int
>
>
> Unfortunately, there is a fail in FreeBSD
> https://cirrus-ci.com/task/6466749487382528
>
> Maybe, this patch is need to be rebased?
>
>
That failure is in postgres_fdw, which this code doesn't touch.
I'm not able to get
to
Unfortunately, there is a fail in FreeBSD
https://cirrus-ci.com/task/6466749487382528
Maybe, this patch is need to be rebased?
--
Best regards,
Maxim Orlov.
>
> Thanks! But CF bot still not happy. I think, we should address issues from
> here https://cirrus-ci.com/task/5391002618298368
>
Sure enough, exit codes are shell dependent...adjusted the tests to reflect
that.
From 237b892e5efe739bc8e75d4af30140520d445491 Mon Sep 17 00:00:00 2001
From:
On Mon, Jan 23, 2023 at 2:53 PM Robert Haas wrote:
> On Mon, Jan 23, 2023 at 1:59 PM Corey Huinker
> wrote:
> > SHELL_ERROR is helpful in that it is a ready-made boolean that works for
> \if tests in the same way that ERROR is set to true any time SQLSTATE is
> nonzero. We don't yet have inline
On Mon, Jan 23, 2023 at 1:59 PM Corey Huinker wrote:
> SHELL_ERROR is helpful in that it is a ready-made boolean that works for \if
> tests in the same way that ERROR is set to true any time SQLSTATE is nonzero.
> We don't yet have inline expressions for \if so the ready-made boolean is a
>
On Fri, Jan 20, 2023 at 8:54 AM Robert Haas wrote:
> On Wed, Jan 4, 2023 at 2:09 AM Corey Huinker
> wrote:
> > 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
On Wed, Jan 4, 2023 at 2:09 AM Corey Huinker wrote:
> 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
On Fri, 13 Jan 2023 at 07:50, Corey Huinker wrote:
>
> I named it wait_result_to_exit_code(), but I welcome suggestions of a
> better name.
>
Thanks! But CF bot still not happy. I think, we should address issues from
here https://cirrus-ci.com/task/5391002618298368
--
Best regards,
Maxim
>
> I belive, we need proper includes.
>
Given that wait_error.c already seems to have the right includes worked out
for WEXITSTATUS/WIFSTOPPED/etc, I decided to just add a function there.
I named it wait_result_to_exit_code(), but I welcome suggestions of a
better name.
From
Unfortunately, cirrus-ci still not happy
https://cirrus-ci.com/task/6502730475241472
[23:14:34.206] time make -s -j${BUILD_JOBS} world-bin
[23:14:43.174] psqlscanslash.l: In function ‘evaluate_backtick’:
[23:14:43.174] psqlscanslash.l:827:11: error: implicit declaration of
function ‘WIFSTOPPED’
>
>
>
> The patch does not apply on top of HEAD as in [1], please post a rebased
> patch:
>
>
Conflict was due to the doc patch applying id tags to psql variable names.
I've rebased and added my own id tags to the two new variables.
From 9e2827a6f955e7cebf87ca538fab113a359951b4 Mon Sep 17 00:00:00
On Tue, 10 Jan 2023 at 00:06, Corey Huinker wrote:
>
> On Mon, Jan 9, 2023 at 10:01 AM Maxim Orlov wrote:
>>
>> Hi!
>>
>> In overall, I think we move in the right direction. But we could make code
>> better, should we?
>>
>> + /* Capture exit code for SHELL_EXIT_CODE */
>> +
On Tue, Jan 10, 2023 at 3:54 AM Maxim Orlov wrote:
>
>
> On Mon, 9 Jan 2023 at 21:36, Corey Huinker
> wrote:
>
>>
>> I chose a name that would avoid collisions with anything a user might
>> potentially throw into their environment, so if the var "OS" is fairly
>> standard is a reason to avoid
On Mon, 9 Jan 2023 at 21:36, Corey Huinker wrote:
>
> I chose a name that would avoid collisions with anything a user might
> potentially throw into their environment, so if the var "OS" is fairly
> standard is a reason to avoid using it. Also, going with our own env var
> allows us to stay in
On Mon, Jan 9, 2023 at 10:01 AM Maxim Orlov wrote:
> Hi!
>
> In overall, I think we move in the right direction. But we could make code
> better, should we?
>
> + /* Capture exit code for SHELL_EXIT_CODE */
> + close_exit_code = pclose(fd);
> + if
Hi!
In overall, I think we move in the right direction. But we could make code
better, should we?
+ /* Capture exit code for SHELL_EXIT_CODE */
+ close_exit_code = pclose(fd);
+ if (close_exit_code == -1)
+ {
+
On Tue, Jan 3, 2023 at 5:36 AM vignesh C wrote:
> On Wed, 21 Dec 2022 at 11:04, Corey Huinker
> 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
On Wed, 21 Dec 2022 at 11:04, Corey Huinker 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
On Sat, Dec 31, 2022 at 5:28 PM Isaac Morland
wrote:
> On Sat, 31 Dec 2022 at 16:47, Corey Huinker
> wrote:
>
>>
>>> I wonder if there is value in setting up a psql on/off var
>>> SHELL_ERROR_OUTPUT construct that when set to "off/false"
>>> suppresses standard error via appending "2>
On Sat, 31 Dec 2022 at 16:47, Corey Huinker wrote:
>
>> I wonder if there is value in setting up a psql on/off var
>> SHELL_ERROR_OUTPUT construct that when set to "off/false"
>> suppresses standard error via appending "2> /dev/null" (or "2> nul" if
>> #ifdef WIN32). At the very least, it would
On Fri, Dec 30, 2022 at 2:17 PM Corey Huinker
wrote:
> On Wed, Dec 28, 2022 at 5:59 AM Maxim Orlov wrote:
>
>> Hi!
>>
>> The patch is implementing what is declared to do. Shell return code is
>> now accessible is psql var.
>> Overall code is in a good condition. Applies with no errors on
On Wed, Dec 28, 2022 at 5:59 AM Maxim Orlov wrote:
> Hi!
>
> The patch is implementing what is declared to do. Shell return code is now
> accessible is psql var.
> Overall code is in a good condition. Applies with no errors on master.
> Unfortunately, regression tests are failing on the macOS
Hi!
The patch is implementing what is declared to do. Shell return code is now
accessible is psql var.
Overall code is in a good condition. Applies with no errors on master.
Unfortunately, regression tests are failing on the macOS due to the
different shell output.
@@ -1308,13 +1308,13 @@
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
Rebased. Still waiting on feedback before working on documentation.
On Fri, Nov 4, 2022 at 5:23 AM Corey Huinker
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
>
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
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
45 matches
Mail list logo
