Thread: psql -1 -f - busted
Greetings,
-1 appears to be ignored when '-f -' is set.
test_file contains:
set client_min_messages=error;
create schema tiger_us;
These produce different results:
BEGIN, but no COMMIT:
beren:/data3/tiger> psql -o /dev/null -e \
--no-psqlrc \
-d gis -1 -q \
--set VERBOSITY=terse \
--set AUTOCOMMIT=off \
--set ON_ERROR_STOP=true \
--set ON_ERROR_ROLLBACK=off \
-f - < test_file
BEGIN and COMMIT:
beren:/data3/tiger> psql -o /dev/null -e \
--no-psqlrc \
-d gis -1 -q \
--set VERBOSITY=terse \
--set AUTOCOMMIT=off \
--set ON_ERROR_STOP=true \
--set ON_ERROR_ROLLBACK=off \
-f test_file
Seems like -1 should work even if -f is set to stdin..
Thanks,
Stephen
On Thu, Oct 22, 2009 at 2:42 PM, Stephen Frost <sfrost@snowman.net> wrote: > =A0-1 appears to be ignored when '-f -' is set. I've been bitten by this, too. It appears that "-f -" is in general equivalent to not specifying "-f" at all. In startup.c we have a test for: options.action =3D=3D ACT_FILE && strcmp(options.action_string, "-") !=3D 0 I suppose we could change it to: options.action =3D=3D ACT_FILE && (strcmp(options.action_string, "-") !=3D 0 || pset.notty) But I sort of think it should just be: options.action =3D=3D ACT_FILE ISTM that if you run psql with "-f -", you shouldn't expect to get an interactive shell. Rather, you should expect psql to do whatever it normally does when given -f somefilename, except using stdin rather than the file. After all, you could have left out -f altogether if you'd wanted the interactive behavior. But then IJWH. ...Robert
On tor, 2009-11-26 at 22:59 -0500, Robert Haas wrote: > ISTM that if you run psql with "-f -", you shouldn't expect to get an > interactive shell. Rather, you should expect psql to do whatever it > normally does when given -f somefilename, except using stdin rather > than the file. After all, you could have left out -f altogether if > you'd wanted the interactive behavior. But then IJWH. But by that logic, psql < file should also set interactive mode.
On Fri, Nov 27, 2009 at 1:42 AM, Peter Eisentraut <peter_e@gmx.net> wrote: > On tor, 2009-11-26 at 22:59 -0500, Robert Haas wrote: >> ISTM that if you run psql with "-f -", you shouldn't expect to get an >> interactive shell. =A0Rather, you should expect psql to do whatever it >> normally does when given -f somefilename, except using stdin rather >> than the file. =A0After all, you could have left out -f altogether if >> you'd wanted the interactive behavior. =A0But then IJWH. > > But by that logic, psql < file should also set interactive mode. Those two cases are not symmetrical. If you're reading from something other than a terminal, you certainly don't want interactive mode. If you ARE reading from a terminal, you might nevertheless want non-interactive mode. And you CERTAINLY might want -1 when reading a pipe, as when you do this: ssh otherhost pg_dump | psql -1 -f - Currently, this silently fails to deliver the expected behavior. ...Robert
Robert Haas wrote:
> On Fri, Nov 27, 2009 at 1:42 AM, Peter Eisentraut <peter_e@gmx.net> wrote:
> > On tor, 2009-11-26 at 22:59 -0500, Robert Haas wrote:
> >> ISTM that if you run psql with "-f -", you shouldn't expect to get an
> >> interactive shell. ?Rather, you should expect psql to do whatever it
> >> normally does when given -f somefilename, except using stdin rather
> >> than the file. ?After all, you could have left out -f altogether if
> >> you'd wanted the interactive behavior. ?But then IJWH.
> >
> > But by that logic, psql < file should also set interactive mode.
>
> Those two cases are not symmetrical. If you're reading from something
> other than a terminal, you certainly don't want interactive mode. If
> you ARE reading from a terminal, you might nevertheless want
> non-interactive mode. And you CERTAINLY might want -1 when reading a
> pipe, as when you do this:
>
> ssh otherhost pg_dump | psql -1 -f -
>
> Currently, this silently fails to deliver the expected behavior.
You are right --- there is definitely something wrong with that code. I
tried to find out why it was coded that way but that code hasn't changed
since 2000 or 2001. My guess is that is was OK then, but when we added
"-1" we didn't think about its placement.
I think the proper, consistent fix is to check for "-" inside
process_file(), rather than earlier. The attached patch accomplishes
this. The code still goes into interactive mode, though:
$ psql -f - test
test=>
Should that prompt not appear? Seems OK to me.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Index: src/bin/psql/command.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/command.c,v
retrieving revision 1.211
diff -c -c -r1.211 command.c
*** src/bin/psql/command.c 22 Nov 2009 05:20:41 -0000 1.211
--- src/bin/psql/command.c 30 Nov 2009 13:47:17 -0000
***************
*** 1691,1698 ****
if (!filename)
return EXIT_FAILURE;
! canonicalize_path(filename);
! fd = fopen(filename, PG_BINARY_R);
if (!fd)
{
--- 1691,1703 ----
if (!filename)
return EXIT_FAILURE;
! if (strcmp(filename, "-") != 0)
! {
! canonicalize_path(filename);
! fd = fopen(filename, PG_BINARY_R);
! }
! else
! fd = stdin;
if (!fd)
{
Index: src/bin/psql/startup.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/startup.c,v
retrieving revision 1.156
diff -c -c -r1.156 startup.c
*** src/bin/psql/startup.c 5 Apr 2009 04:19:58 -0000 1.156
--- src/bin/psql/startup.c 30 Nov 2009 13:47:17 -0000
***************
*** 225,231 ****
/*
* process file given by -f
*/
! if (options.action == ACT_FILE && strcmp(options.action_string, "-") != 0)
{
if (!options.no_psqlrc)
process_psqlrc(argv[0]);
--- 225,231 ----
/*
* process file given by -f
*/
! if (options.action == ACT_FILE)
{
if (!options.no_psqlrc)
process_psqlrc(argv[0]);
Applied. Thanks for the report. --------------------------------------------------------------------------- Bruce Momjian wrote: > Robert Haas wrote: > > On Fri, Nov 27, 2009 at 1:42 AM, Peter Eisentraut <peter_e@gmx.net> wrote: > > > On tor, 2009-11-26 at 22:59 -0500, Robert Haas wrote: > > >> ISTM that if you run psql with "-f -", you shouldn't expect to get an > > >> interactive shell. ?Rather, you should expect psql to do whatever it > > >> normally does when given -f somefilename, except using stdin rather > > >> than the file. ?After all, you could have left out -f altogether if > > >> you'd wanted the interactive behavior. ?But then IJWH. > > > > > > But by that logic, psql < file should also set interactive mode. > > > > Those two cases are not symmetrical. If you're reading from something > > other than a terminal, you certainly don't want interactive mode. If > > you ARE reading from a terminal, you might nevertheless want > > non-interactive mode. And you CERTAINLY might want -1 when reading a > > pipe, as when you do this: > > > > ssh otherhost pg_dump | psql -1 -f - > > > > Currently, this silently fails to deliver the expected behavior. > > You are right --- there is definitely something wrong with that code. I > tried to find out why it was coded that way but that code hasn't changed > since 2000 or 2001. My guess is that is was OK then, but when we added > "-1" we didn't think about its placement. > > I think the proper, consistent fix is to check for "-" inside > process_file(), rather than earlier. The attached patch accomplishes > this. The code still goes into interactive mode, though: > > $ psql -f - test > test=> > > Should that prompt not appear? Seems OK to me. > > -- > Bruce Momjian <bruce@momjian.us> http://momjian.us > EnterpriseDB http://enterprisedb.com > > + If your life is a hard drive, Christ can be your backup. + [ text/x-diff is unsupported, treating like TEXT/PLAIN ] > Index: src/bin/psql/command.c > =================================================================== > RCS file: /cvsroot/pgsql/src/bin/psql/command.c,v > retrieving revision 1.211 > diff -c -c -r1.211 command.c > *** src/bin/psql/command.c 22 Nov 2009 05:20:41 -0000 1.211 > --- src/bin/psql/command.c 30 Nov 2009 13:47:17 -0000 > *************** > *** 1691,1698 **** > if (!filename) > return EXIT_FAILURE; > > ! canonicalize_path(filename); > ! fd = fopen(filename, PG_BINARY_R); > > if (!fd) > { > --- 1691,1703 ---- > if (!filename) > return EXIT_FAILURE; > > ! if (strcmp(filename, "-") != 0) > ! { > ! canonicalize_path(filename); > ! fd = fopen(filename, PG_BINARY_R); > ! } > ! else > ! fd = stdin; > > if (!fd) > { > Index: src/bin/psql/startup.c > =================================================================== > RCS file: /cvsroot/pgsql/src/bin/psql/startup.c,v > retrieving revision 1.156 > diff -c -c -r1.156 startup.c > *** src/bin/psql/startup.c 5 Apr 2009 04:19:58 -0000 1.156 > --- src/bin/psql/startup.c 30 Nov 2009 13:47:17 -0000 > *************** > *** 225,231 **** > /* > * process file given by -f > */ > ! if (options.action == ACT_FILE && strcmp(options.action_string, "-") != 0) > { > if (!options.no_psqlrc) > process_psqlrc(argv[0]); > --- 225,231 ---- > /* > * process file given by -f > */ > ! if (options.action == ACT_FILE) > { > if (!options.no_psqlrc) > process_psqlrc(argv[0]); > > -- > Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-bugs -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +