Thread: psql: exit status with multiple -c and -f

I raised this issue a few years ago.
https://www.postgresql.org/message-id/20181217175841.GS13019%40telsasoft.com

|[pryzbyj@database ~]$ psql -v VERBOSITY=terse ts -xtc 'ONE' -c "SELECT 'TWO'"; echo "exit status $?"
|ERROR:  syntax error at or near "ONE" at character 1
|?column? | TWO
|
|exit status 0

The documentation doen't say what the exit status should be in this case:
| psql returns 0 to the shell if it finished normally, 1 if a fatal error of its own occurs (e.g., out of memory, file
notfound), 2 if the connection to the server went bad and the session was not interactive, and 3 if an error occurred
ina script and the variable ON_ERROR_STOP was set.
 

It returns 1 if the final command fails, even though it's not a "fatal error"
(it would've happily kept running more commands).

| x=`some_command_that_fails`
| rm -fr "$x/$y # removes all your data

| psql -c "begin; C REATE TABLE newtable(LIKE oldtable) INSERT INTO newtable SELECT * FROM oldtable; commit" -c "DROP
TABLEoldtable
 
| psql -v VERBOSITY=terse ts -xtc '0CREATE TABLE newtbl(i int)' -c 'INSERT INTO newtbl SELECT * FROM tbl' -c 'DROP
TABLEIF EXISTS tbl' -c 'ALTER TABLE newtbl RENAME TO tbl'; echo ret=$?
 

David J suggested to change the default value of ON_ERROR_STOP.  The exit
status in the non-default case would have to be documented.  That's one
solution, and allows the old behavior if anybody wants it.  That probably does
what most people want, too.  This is more likely to expose a real problem that
someone would have missed than to break a legitimate use.  That doesn't appear
to break regression tests at all.

The alternative to David's suggestion is to define some non-zero exit status to
mean "an error occurred and ON_ERROR_STOP was not set".

If the new definition said that the new exit status was only used for errors
which occur in the final command (-c or -f), and if the exit status in that
case were "1", then this would be a back-patchable documentation change,
removing the word "fatal" and removing or updating the parenthetical examples.
That would make psql behave exactly like /bin/sh does when used without
"set -e" - which is usually the opposite of the desirable behavior...

I think it's not viable to change the exit status in the case of a single
-c/-f, nor to change the exit status in back branches.  David's suggestion is
more radical than the minimal change to a nonzero exit status, but maybe that's
okay ?

-- 
Justin

On Mon, Dec 06, 2021 at 09:08:56AM -0600, Justin Pryzby wrote:
> I raised this issue a few years ago.
> https://www.postgresql.org/message-id/20181217175841.GS13019%40telsasoft.com
> 
> |[pryzbyj@database ~]$ psql -v VERBOSITY=terse ts -xtc 'ONE' -c "SELECT 'TWO'"; echo "exit status $?"
> |ERROR:  syntax error at or near "ONE" at character 1
> |?column? | TWO
> |
> |exit status 0
> 
> The documentation doen't say what the exit status should be in this case:
> | psql returns 0 to the shell if it finished normally, 1 if a fatal error of its own occurs (e.g., out of memory,
filenot found), 2 if the connection to the server went bad and the session was not interactive, and 3 if an error
occurredin a script and the variable ON_ERROR_STOP was set.
 
> 
> It returns 1 if the final command fails, even though it's not a "fatal error"
> (it would've happily kept running more commands).
> 
> | x=`some_command_that_fails`
> | rm -fr "$x/$y # removes all your data
> 
> | psql -c "begin; C REATE TABLE newtable(LIKE oldtable) INSERT INTO newtable SELECT * FROM oldtable; commit" -c "DROP
TABLEoldtable
 
> | psql -v VERBOSITY=terse ts -xtc '0CREATE TABLE newtbl(i int)' -c 'INSERT INTO newtbl SELECT * FROM tbl' -c 'DROP
TABLEIF EXISTS tbl' -c 'ALTER TABLE newtbl RENAME TO tbl'; echo ret=$?
 
> 
> David J suggested to change the default value of ON_ERROR_STOP.  The exit
> status in the non-default case would have to be documented.  That's one
> solution, and allows the old behavior if anybody wants it.  That probably does
> what most people want, too.  This is more likely to expose a real problem that
> someone would have missed than to break a legitimate use.  That doesn't appear
> to break regression tests at all.

I was wrong - the regression tests specifically exercise failure cases, so the
scripts must continue after errors.

I think the current behavior of the regression test SQL scripts is exactly the
opposite of what's desirable for almost all other scripts.  The attached makes
ON_ERROR_STOP the default, and runs the regression tests with ON_ERROR_STOP=0.

Is it viable to consider changing this ?

Justin Pryzby <pryzby@telsasoft.com> writes:
> I think the current behavior of the regression test SQL scripts is exactly the
> opposite of what's desirable for almost all other scripts.  The attached makes
> ON_ERROR_STOP the default, and runs the regression tests with ON_ERROR_STOP=0.

> Is it viable to consider changing this ?

I don't think so.  The number of scripts you will break is far greater
than the number whose behavior will be improved, because people who
wanted this behavior will already be selecting it.  Maybe this wasn't
the greatest choice of default, but it's about twenty years too late
to change it.

I'd also note that I see a fairly direct parallel to "set -e" in
shell scripts, which is likewise not the default.

We could consider documentation changes to make this issue
more visible, perhaps.  Not sure what would be a good place.

            regards, tom lane

Hi,

On Mon, Dec 27, 2021 at 12:31:07PM -0500, Tom Lane wrote:
> Justin Pryzby <pryzby@telsasoft.com> writes:
> > I think the current behavior of the regression test SQL scripts is exactly the
> > opposite of what's desirable for almost all other scripts.  The attached makes
> > ON_ERROR_STOP the default, and runs the regression tests with ON_ERROR_STOP=0.
> 
> > Is it viable to consider changing this ?
> 
> I don't think so.  The number of scripts you will break is far greater
> than the number whose behavior will be improved, because people who
> wanted this behavior will already be selecting it.  Maybe this wasn't
> the greatest choice of default, but it's about twenty years too late
> to change it.
> 
> I'd also note that I see a fairly direct parallel to "set -e" in
> shell scripts, which is likewise not the default.
> 
> We could consider documentation changes to make this issue
> more visible, perhaps.  Not sure what would be a good place.

I'm marking the CF entry as returned with feedback as it's been a few weeks
without proposal for documentation change.