Clean up error cases in psql's COPY TO STDOUT/FROM STDIN code.
authorTom Lane <tgl@sss.pgh.pa.us>
Thu, 13 Feb 2014 23:45:32 +0000 (18:45 -0500)
committerTom Lane <tgl@sss.pgh.pa.us>
Thu, 13 Feb 2014 23:45:32 +0000 (18:45 -0500)
Adjust handleCopyOut() to stop trying to write data once it's failed
one time.  For typical cases such as out-of-disk-space or broken-pipe,
additional attempts aren't going to do anything but waste time, and
in any case clean truncation of the output seems like a better behavior
than randomly dropping blocks in the middle.

Also remove dubious (and misleadingly documented) attempt to force our way
out of COPY_OUT state if libpq didn't do that.  If we did have a situation
like that, it'd be a bug in libpq and would be better fixed there, IMO.
We can hope that commit fa4440f51628d692f077d54b8313aea31af087ea took care
of any such problems, anyway.

Also fix longstanding bug in handleCopyIn(): PQputCopyEnd() only supports
a non-null errormsg parameter in protocol version 3, and will actively
fail if one is passed in version 2.  This would've made our attempts
to get out of COPY_IN state after a failure into infinite loops when
talking to pre-7.4 servers.

Back-patch the COPY_OUT state change business back to 9.2 where it was
introduced, and the other two fixes into all supported branches.

src/bin/psql/copy.c

index 6d291099b1f6c04f3f8f27c19663028fae97d22d..045ad927278d21e00df2fb2525a59495b5dfea30 100644 (file)
@@ -573,7 +573,9 @@ do_copy(const char *args)
                   PQresultStatus(result));
        /* if still in COPY IN state, try to get out of it */
        if (PQresultStatus(result) == PGRES_COPY_IN)
-           PQputCopyEnd(pset.db, _("trying to exit copy mode"));
+           PQputCopyEnd(pset.db,
+                        (PQprotocolVersion(pset.db) < 3) ? NULL :
+                        _("trying to exit copy mode"));
        PQclear(result);
    }
 
@@ -620,15 +622,15 @@ handleCopyOut(PGconn *conn, FILE *copystream)
        ret = PQgetCopyData(conn, &buf, 0);
 
        if (ret < 0)
-           break;              /* done or error */
+           break;              /* done or server/connection error */
 
        if (buf)
        {
-           if (fwrite(buf, 1, ret, copystream) != ret)
+           if (OK && fwrite(buf, 1, ret, copystream) != ret)
            {
-               if (OK)         /* complain only once, keep reading data */
-                   psql_error("could not write COPY data: %s\n",
-                              strerror(errno));
+               psql_error("could not write COPY data: %s\n",
+                          strerror(errno));
+               /* complain only once, keep reading data from server */
                OK = false;
            }
            PQfreemem(buf);
@@ -692,7 +694,9 @@ handleCopyIn(PGconn *conn, FILE *copystream, bool isbinary)
        /* got here with longjmp */
 
        /* Terminate data transfer */
-       PQputCopyEnd(conn, _("canceled by user"));
+       PQputCopyEnd(conn,
+                    (PQprotocolVersion(conn) < 3) ? NULL :
+                    _("canceled by user"));
 
        /* Check command status and return to normal libpq state */
        res = PQgetResult(conn);
@@ -820,7 +824,8 @@ handleCopyIn(PGconn *conn, FILE *copystream, bool isbinary)
 
    /* Terminate data transfer */
    if (PQputCopyEnd(conn,
-                    OK ? NULL : _("aborted because of read failure")) <= 0)
+                    (OK || PQprotocolVersion(conn) < 3) ? NULL :
+                    _("aborted because of read failure")) <= 0)
        OK = false;
 
    /* Check command status and return to normal libpq state */