Prevent "\g filename" from affecting subsequent commands after an error.

In the previous coding, psql's state variable saying that output should
go to a file was only reset after successful completion of a query
returning tuples.  Thus for example,

regression=# select 1/0
regression-# \g somefile
ERROR:  division by zero
regression=# select 1/2;
regression=#

... huh, I wonder where that output went.  Even more oddly, the state
was not reset even if it's the file that's causing the failure:

regression=# select 1/2 \g /foo
/foo: Permission denied
regression=# select 1/2;
/foo: Permission denied
regression=# select 1/2;
/foo: Permission denied

This seems to me not to satisfy the principle of least surprise.
\g is certainly not documented in a way that suggests its effects are
at all persistent.

To fix, adjust the code so that the flag is reset at exit from SendQuery
no matter what happened.

Noted while reviewing the \gset patch, which had comparable issues.
Arguably this is a bug fix, but I'll refrain from back-patching for now.

diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 4c87d8ad7fe88459a532bef1fc81d51d43e2ad2a..eb5e3b19048d535b5f5ae711971b9f8fb97fcc25 100644 (file)
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -1608,9 +1608,13 @@ Tue Oct 26 21:40:57 CEST 1999
         optionally stores the query's output in <replaceable
         class="parameter">filename</replaceable> or pipes the output
         into a separate Unix shell executing <replaceable
-        class="parameter">command</replaceable>. A bare
-        <literal>\g</literal> is virtually equivalent to a semicolon. A
-        <literal>\g</literal> with argument is a <quote>one-shot</quote>
+        class="parameter">command</replaceable>.  The file or command is
+        written to only if the query successfully returns zero or more tuples,
+        not if the query fails or is a non-data-returning SQL command.
+        </para>
+        <para>
+        A bare <literal>\g</literal> is essentially equivalent to a semicolon.
+        A <literal>\g</literal> with argument is a <quote>one-shot</quote>
         alternative to the <command>\o</command> command.
         </para>
         </listitem>

diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 20c45e2d0a5f3d1c4053d6d059e92c6c126b442d..1e9aa89089ee0b3b4ac13e6db184cc7e3ed06d5e 100644 (file)
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -731,7 +731,7 @@ exec_command(const char *cmd,
                free(fname);
        }
 
-       /* \g means send query */
+       /* \g [filename] means send query, optionally with output to file/pipe */
        else if (strcmp(cmd, "g") == 0)
        {
                char       *fname = psql_scan_slash_option(scan_state,

diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index 5fb031650e3e09388a05aed160e2cfe6b3f2ddbc..ab517906fca744bfca610a766ac4d7d7804b3b50 100644 (file)
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -607,9 +607,6 @@ PrintQueryTuples(const PGresult *results)
 
                pset.queryFout = queryFout_copy;
                pset.queryFoutPipe = queryFoutPipe_copy;
-
-               free(pset.gfname);
-               pset.gfname = NULL;
        }
        else
                printQuery(results, &my_popt, pset.queryFout, pset.logfile);
@@ -835,14 +832,14 @@ SendQuery(const char *query)
        PGresult   *results;
        PGTransactionStatusType transaction_status;
        double          elapsed_msec = 0;
-       bool            OK,
-                               on_error_rollback_savepoint = false;
+       bool            OK = false;
+       bool            on_error_rollback_savepoint = false;
        static bool on_error_rollback_warning = false;
 
        if (!pset.db)
        {
                psql_error("You are currently not connected to a database.\n");
-               return false;
+               goto sendquery_cleanup;
        }
 
        if (pset.singlestep)
@@ -856,7 +853,7 @@ SendQuery(const char *query)
                fflush(stdout);
                if (fgets(buf, sizeof(buf), stdin) != NULL)
                        if (buf[0] == 'x')
-                               return false;
+                               goto sendquery_cleanup;
        }
        else if (pset.echo == PSQL_ECHO_QUERIES)
        {
@@ -887,7 +884,7 @@ SendQuery(const char *query)
                        psql_error("%s", PQerrorMessage(pset.db));
                        PQclear(results);
                        ResetCancelConn();
-                       return false;
+                       goto sendquery_cleanup;
                }
                PQclear(results);
                transaction_status = PQtransactionStatus(pset.db);
@@ -912,7 +909,7 @@ SendQuery(const char *query)
                                psql_error("%s", PQerrorMessage(pset.db));
                                PQclear(results);
                                ResetCancelConn();
-                               return false;
+                               goto sendquery_cleanup;
                        }
                        PQclear(results);
                        on_error_rollback_savepoint = true;
@@ -1008,10 +1005,11 @@ SendQuery(const char *query)
                        {
                                psql_error("%s", PQerrorMessage(pset.db));
                                PQclear(svptres);
+                               OK = false;
 
                                PQclear(results);
                                ResetCancelConn();
-                               return false;
+                               goto sendquery_cleanup;
                        }
                        PQclear(svptres);
                }
@@ -1037,6 +1035,17 @@ SendQuery(const char *query)
 
        PrintNotifications();
 
+       /* perform cleanup that should occur after any attempted query */
+
+sendquery_cleanup:
+
+       /* reset \g's output-to-filename trigger */
+       if (pset.gfname)
+       {
+               free(pset.gfname);
+               pset.gfname = NULL;
+       }
+
        return OK;
 }
 
@@ -1218,9 +1227,6 @@ ExecQueryUsingCursor(const char *query, double *elapsed_msec)
 
                pset.queryFout = queryFout_copy;
                pset.queryFoutPipe = queryFoutPipe_copy;
-
-               free(pset.gfname);
-               pset.gfname = NULL;
        }
        else if (did_pager)
        {