Skip to content
Snippets Groups Projects
Commit 2573f08a authored by Tom Lane's avatar Tom Lane
Browse files

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 fa4440f5 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.
parent 8439ee41
No related branches found
No related tags found
No related merge requests found
...@@ -363,15 +363,15 @@ handleCopyOut(PGconn *conn, FILE *copystream) ...@@ -363,15 +363,15 @@ handleCopyOut(PGconn *conn, FILE *copystream)
ret = PQgetCopyData(conn, &buf, 0); ret = PQgetCopyData(conn, &buf, 0);
if (ret < 0) if (ret < 0)
break; /* done or error */ break; /* done or server/connection error */
if (buf) 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",
psql_error("could not write COPY data: %s\n", strerror(errno));
strerror(errno)); /* complain only once, keep reading data from server */
OK = false; OK = false;
} }
PQfreemem(buf); PQfreemem(buf);
...@@ -392,29 +392,18 @@ handleCopyOut(PGconn *conn, FILE *copystream) ...@@ -392,29 +392,18 @@ handleCopyOut(PGconn *conn, FILE *copystream)
} }
/* /*
* Check command status and return to normal libpq state. After a * Check command status and return to normal libpq state.
* client-side error, the server will remain ready to deliver data. The
* cleanest thing is to fully drain and discard that data. If the
* client-side error happened early in a large file, this takes a long
* time. Instead, take advantage of the fact that PQexec() will silently
* end any ongoing PGRES_COPY_OUT state. This does cause us to lose the
* results of any commands following the COPY in a single command string.
* It also only works for protocol version 3. XXX should we clean up
* using the slow way when the connection is using protocol version 2?
* *
* We must not ever return with the status still PGRES_COPY_OUT. Our * If for some reason libpq is still reporting PGRES_COPY_OUT state, we
* caller is unable to distinguish that situation from reaching the next * would like to forcibly exit that state, since our caller would be
* COPY in a command string that happened to contain two consecutive COPY * unable to distinguish that situation from reaching the next COPY in a
* TO STDOUT commands. We trust that no condition can make PQexec() fail * command string that happened to contain two consecutive COPY TO STDOUT
* indefinitely while retaining status PGRES_COPY_OUT. * commands. However, libpq provides no API for doing that, and in
* principle it's a libpq bug anyway if PQgetCopyData() returns -1 or -2
* but hasn't exited COPY_OUT state internally. So we ignore the
* possibility here.
*/ */
while (res = PQgetResult(conn), PQresultStatus(res) == PGRES_COPY_OUT) res = PQgetResult(conn);
{
OK = false;
PQclear(res);
PQexec(conn, "-- clear PGRES_COPY_OUT state");
}
if (PQresultStatus(res) != PGRES_COMMAND_OK) if (PQresultStatus(res) != PGRES_COMMAND_OK)
{ {
psql_error("%s", PQerrorMessage(conn)); psql_error("%s", PQerrorMessage(conn));
...@@ -457,7 +446,9 @@ handleCopyIn(PGconn *conn, FILE *copystream, bool isbinary) ...@@ -457,7 +446,9 @@ handleCopyIn(PGconn *conn, FILE *copystream, bool isbinary)
/* got here with longjmp */ /* got here with longjmp */
/* Terminate data transfer */ /* Terminate data transfer */
PQputCopyEnd(conn, _("canceled by user")); PQputCopyEnd(conn,
(PQprotocolVersion(conn) < 3) ? NULL :
_("canceled by user"));
OK = false; OK = false;
goto copyin_cleanup; goto copyin_cleanup;
...@@ -578,29 +569,37 @@ handleCopyIn(PGconn *conn, FILE *copystream, bool isbinary) ...@@ -578,29 +569,37 @@ handleCopyIn(PGconn *conn, FILE *copystream, bool isbinary)
if (ferror(copystream)) if (ferror(copystream))
OK = false; OK = false;
/* Terminate data transfer */ /*
* Terminate data transfer. We can't send an error message if we're using
* protocol version 2.
*/
if (PQputCopyEnd(conn, if (PQputCopyEnd(conn,
OK ? NULL : _("aborted because of read failure")) <= 0) (OK || PQprotocolVersion(conn) < 3) ? NULL :
_("aborted because of read failure")) <= 0)
OK = false; OK = false;
copyin_cleanup: copyin_cleanup:
/* /*
* Check command status and return to normal libpq state * Check command status and return to normal libpq state.
* *
* We must not ever return with the status still PGRES_COPY_IN. Our * We do not want to return with the status still PGRES_COPY_IN: our
* caller is unable to distinguish that situation from reaching the next * caller would be unable to distinguish that situation from reaching the
* COPY in a command string that happened to contain two consecutive COPY * next COPY in a command string that happened to contain two consecutive
* FROM STDIN commands. XXX if something makes PQputCopyEnd() fail * COPY FROM STDIN commands. We keep trying PQputCopyEnd() in the hope
* indefinitely while retaining status PGRES_COPY_IN, we get an infinite * it'll work eventually. (What's actually likely to happen is that in
* loop. This is more realistic than handleCopyOut()'s counterpart risk. * attempting to flush the data, libpq will eventually realize that the
* connection is lost. But that's fine; it will get us out of COPY_IN
* state, which is what we need.)
*/ */
while (res = PQgetResult(conn), PQresultStatus(res) == PGRES_COPY_IN) while (res = PQgetResult(conn), PQresultStatus(res) == PGRES_COPY_IN)
{ {
OK = false; OK = false;
PQclear(res); PQclear(res);
/* We can't send an error message if we're using protocol version 2 */
PQputCopyEnd(pset.db, _("trying to exit copy mode")); PQputCopyEnd(conn,
(PQprotocolVersion(conn) < 3) ? NULL :
_("trying to exit copy mode"));
} }
if (PQresultStatus(res) != PGRES_COMMAND_OK) if (PQresultStatus(res) != PGRES_COMMAND_OK)
{ {
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment