From 7182bd239457b9cc3a1db337e3f10a9ee1388f78 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 13 Feb 2014 18:45:23 -0500 Subject: [PATCH] Clean up error cases in psql's COPY TO STDOUT/FROM STDIN code. 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 | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/src/bin/psql/copy.c b/src/bin/psql/copy.c index ebe5ee9ea55..2e1dff7f483 100644 --- a/src/bin/psql/copy.c +++ b/src/bin/psql/copy.c @@ -350,7 +350,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); } @@ -397,15 +399,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); @@ -469,7 +471,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); @@ -597,7 +601,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 */