psql ignores failure to open -o target file

Started by Tom Laneabout 10 years ago6 messages
#1Tom Lane
tgl@sss.pgh.pa.us

I just noticed that parse_psql_options() ignores the result of setQFout(),
meaning that if the argument of a -o command line option is bogus, we'll
ignore the switch entirely after printing an error report. For example

$ psql -o /dev/foo -c 'select 1'
/dev/foo: Permission denied
?column?
----------
1
(1 row)

$

This seems surprising to me: any other program in the world would do
exit(1) after discovering that it couldn't write where it had been
told to. Should we change this?

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tom Lane (#1)
Re: psql ignores failure to open -o target file

I wrote:

I just noticed that parse_psql_options() ignores the result of setQFout(),
meaning that if the argument of a -o command line option is bogus, we'll
ignore the switch entirely after printing an error report.

There's more silliness in the same area. \o with an invalid target spec
is treated like \o with no argument, ie, revert the target to stdout:

regression=# \o somefile
regression=# select 1; -- output goes to somefile, as expected
regression=# \o /dev/bogus
/dev/bogus: Permission denied
regression=# select 1;
?column?
----------
1
(1 row)

Surely that should have no effect beyond printing the error message.

Also, I notice that we try to set SIGPIPE handling to SIG_IGN whenever
the \o target is a pipe, which is good; but places that transiently
change SIGPIPE handling will unconditionally reset it back to SIG_DFL.
One such place is ClosePager(). That bug seems only latent because we
only invoke a pager when queryFout == stdout, so we would never get
there while the target is a pipe. But another such place is do_copy(),
and we definitely can call that while \o is set to a pipe.

In short, a bit of refactoring is needed around setQFout...

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#1)
Re: psql ignores failure to open -o target file

On Wed, Dec 2, 2015 at 11:07 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I just noticed that parse_psql_options() ignores the result of setQFout(),
meaning that if the argument of a -o command line option is bogus, we'll
ignore the switch entirely after printing an error report. For example

$ psql -o /dev/foo -c 'select 1'
/dev/foo: Permission denied
?column?
----------
1
(1 row)

$

This seems surprising to me: any other program in the world would do
exit(1) after discovering that it couldn't write where it had been
told to. Should we change this?

I assume this is a rhetorical question.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#4David G. Johnston
david.g.johnston@gmail.com
In reply to: Robert Haas (#3)
Re: psql ignores failure to open -o target file

On Wed, Dec 2, 2015 at 11:04 AM, Robert Haas <robertmhaas@gmail.com> wrote:

On Wed, Dec 2, 2015 at 11:07 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I just noticed that parse_psql_options() ignores the result of

setQFout(),

meaning that if the argument of a -o command line option is bogus, we'll
ignore the switch entirely after printing an error report. For example

$ psql -o /dev/foo -c 'select 1'
/dev/foo: Permission denied
?column?
----------
1
(1 row)

$

This seems surprising to me: any other program in the world would do
exit(1) after discovering that it couldn't write where it had been
told to. Should we change this?

I assume this is a rhetorical question.

How about this one: do we change this behavior in the back branches?

​Given other instances of in script errors not causing psql to exit (hence
ON_ERROR_STOP) this doesn't seem as surprising as it is being made out to
be.

David J.

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: David G. Johnston (#4)
Re: psql ignores failure to open -o target file

"David G. Johnston" <david.g.johnston@gmail.com> writes:

On Wed, Dec 2, 2015 at 11:04 AM, Robert Haas <robertmhaas@gmail.com> wrote:

On Wed, Dec 2, 2015 at 11:07 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

This seems surprising to me: any other program in the world would do
exit(1) after discovering that it couldn't write where it had been
told to. Should we change this?

I assume this is a rhetorical question.

How about this one: do we change this behavior in the back branches?

I don't think we should change this in stable branches. I would vote
for fixing it in 9.5, though, mainly because the fix is going to interact
with the extended-mode-wrap fixes I'm also working on.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#6Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#5)
Re: psql ignores failure to open -o target file

On Wed, Dec 2, 2015 at 1:24 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

"David G. Johnston" <david.g.johnston@gmail.com> writes:

On Wed, Dec 2, 2015 at 11:04 AM, Robert Haas <robertmhaas@gmail.com> wrote:

On Wed, Dec 2, 2015 at 11:07 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

This seems surprising to me: any other program in the world would do
exit(1) after discovering that it couldn't write where it had been
told to. Should we change this?

I assume this is a rhetorical question.

How about this one: do we change this behavior in the back branches?

I don't think we should change this in stable branches. I would vote
for fixing it in 9.5, though, mainly because the fix is going to interact
with the extended-mode-wrap fixes I'm also working on.

Oh. I would assume this was an obvious back-patch back to the stone
age. It seems like flagrant bug. Let's revive 7.4 just to fix this.
:-)

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers