Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Commit2573f08

Browse files
committed
Clean up error cases in psql's COPY TO STDOUT/FROM STDIN code.
Adjust handleCopyOut() to stop trying to write data once it's failedone time. For typical cases such as out-of-disk-space or broken-pipe,additional attempts aren't going to do anything but waste time, andin any case clean truncation of the output seems like a better behaviorthan randomly dropping blocks in the middle.Also remove dubious (and misleadingly documented) attempt to force our wayout of COPY_OUT state if libpq didn't do that. If we did have a situationlike that, it'd be a bug in libpq and would be better fixed there, IMO.We can hope that commitfa4440f took careof any such problems, anyway.Also fix longstanding bug in handleCopyIn(): PQputCopyEnd() only supportsa non-null errormsg parameter in protocol version 3, and will activelyfail if one is passed in version 2. This would've made our attemptsto get out of COPY_IN state after a failure into infinite loops whentalking to pre-7.4 servers.Back-patch the COPY_OUT state change business back to 9.2 where it wasintroduced, and the other two fixes into all supported branches.
1 parent8439ee4 commit2573f08

File tree

1 file changed

+37
-38
lines changed

1 file changed

+37
-38
lines changed

‎src/bin/psql/copy.c‎

Lines changed: 37 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -363,15 +363,15 @@ handleCopyOut(PGconn *conn, FILE *copystream)
363363
ret=PQgetCopyData(conn,&buf,0);
364364

365365
if (ret<0)
366-
break;/* done or error */
366+
break;/* done orserver/connectionerror */
367367

368368
if (buf)
369369
{
370-
if (fwrite(buf,1,ret,copystream)!=ret)
370+
if (OK&&fwrite(buf,1,ret,copystream)!=ret)
371371
{
372-
if (OK)/* complain only once, keep readingdata */
373-
psql_error("could not write COPY data: %s\n",
374-
strerror(errno));
372+
psql_error("could not write COPYdata: %s\n",
373+
strerror(errno));
374+
/* complain only once, keep reading data from server */
375375
OK= false;
376376
}
377377
PQfreemem(buf);
@@ -392,29 +392,18 @@ handleCopyOut(PGconn *conn, FILE *copystream)
392392
}
393393

394394
/*
395-
* Check command status and return to normal libpq state. After a
396-
* client-side error, the server will remain ready to deliver data. The
397-
* cleanest thing is to fully drain and discard that data.If the
398-
* client-side error happened early in a large file, this takes a long
399-
* time. Instead, take advantage of the fact that PQexec() will silently
400-
* end any ongoing PGRES_COPY_OUT state. This does cause us to lose the
401-
* results of any commands following the COPY in a single command string.
402-
* It also only works for protocol version 3. XXX should we clean up
403-
* using the slow way when the connection is using protocol version 2?
395+
* Check command status and return to normal libpq state.
404396
*
405-
* We must not ever return with the status still PGRES_COPY_OUT. Our
406-
* caller is unable to distinguish that situation from reaching the next
407-
* COPY in a command string that happened to contain two consecutive COPY
408-
* TO STDOUT commands.We trust that no condition can make PQexec() fail
409-
* indefinitely while retaining status PGRES_COPY_OUT.
397+
* If for some reason libpq is still reporting PGRES_COPY_OUT state, we
398+
* would like to forcibly exit that state, since our caller would be
399+
* unable to distinguish that situation from reaching the next COPY in a
400+
* command string that happened to contain two consecutive COPY TO STDOUT
401+
* commands. However, libpq provides no API for doing that, and in
402+
* principle it's a libpq bug anyway if PQgetCopyData() returns -1 or -2
403+
* but hasn't exited COPY_OUT state internally. So we ignore the
404+
* possibility here.
410405
*/
411-
while (res=PQgetResult(conn),PQresultStatus(res)==PGRES_COPY_OUT)
412-
{
413-
OK= false;
414-
PQclear(res);
415-
416-
PQexec(conn,"-- clear PGRES_COPY_OUT state");
417-
}
406+
res=PQgetResult(conn);
418407
if (PQresultStatus(res)!=PGRES_COMMAND_OK)
419408
{
420409
psql_error("%s",PQerrorMessage(conn));
@@ -457,7 +446,9 @@ handleCopyIn(PGconn *conn, FILE *copystream, bool isbinary)
457446
/* got here with longjmp */
458447

459448
/* Terminate data transfer */
460-
PQputCopyEnd(conn,_("canceled by user"));
449+
PQputCopyEnd(conn,
450+
(PQprotocolVersion(conn)<3) ?NULL :
451+
_("canceled by user"));
461452

462453
OK= false;
463454
gotocopyin_cleanup;
@@ -578,29 +569,37 @@ handleCopyIn(PGconn *conn, FILE *copystream, bool isbinary)
578569
if (ferror(copystream))
579570
OK= false;
580571

581-
/* Terminate data transfer */
572+
/*
573+
* Terminate data transfer. We can't send an error message if we're using
574+
* protocol version 2.
575+
*/
582576
if (PQputCopyEnd(conn,
583-
OK ?NULL :_("aborted because of read failure")) <=0)
577+
(OK||PQprotocolVersion(conn)<3) ?NULL :
578+
_("aborted because of read failure")) <=0)
584579
OK= false;
585580

586581
copyin_cleanup:
587582

588583
/*
589-
* Check command status and return to normal libpq state
584+
* Check command status and return to normal libpq state.
590585
*
591-
* We must not ever return with the status still PGRES_COPY_IN. Our
592-
* caller is unable to distinguish that situation from reaching the next
593-
* COPY in a command string that happened to contain two consecutive COPY
594-
* FROM STDIN commands. XXX if something makes PQputCopyEnd() fail
595-
* indefinitely while retaining status PGRES_COPY_IN, we get an infinite
596-
* loop. This is more realistic than handleCopyOut()'s counterpart risk.
586+
* We do not want to return with the status still PGRES_COPY_IN: our
587+
* caller would be unable to distinguish that situation from reaching the
588+
* next COPY in a command string that happened to contain two consecutive
589+
* COPY FROM STDIN commands. We keep trying PQputCopyEnd() in the hope
590+
* it'll work eventually. (What's actually likely to happen is that in
591+
* attempting to flush the data, libpq will eventually realize that the
592+
* connection is lost.But that's fine; it will get us out of COPY_IN
593+
* state, which is what we need.)
597594
*/
598595
while (res=PQgetResult(conn),PQresultStatus(res)==PGRES_COPY_IN)
599596
{
600597
OK= false;
601598
PQclear(res);
602-
603-
PQputCopyEnd(pset.db,_("trying to exit copy mode"));
599+
/* We can't send an error message if we're using protocol version 2 */
600+
PQputCopyEnd(conn,
601+
(PQprotocolVersion(conn)<3) ?NULL :
602+
_("trying to exit copy mode"));
604603
}
605604
if (PQresultStatus(res)!=PGRES_COMMAND_OK)
606605
{

0 commit comments

Comments
 (0)

[8]ページ先頭

©2009-2025 Movatter.jp