Skip to content

Commit f70a78b

Browse files
tglsfdctglsfdc
committed
Allow psql to print COPY command status in more cases.
Previously, psql would print the "COPY nnn" command status only for COPY commands executed server-side. Now it will print that for frontend copies too (including \copy). However, we continue to suppress the command status for COPY TO STDOUT, since in that case the copy data has been routed to the same place that the command status would go, and there is a risk of the status line being mistaken for another line of COPY data. Doing that would break existing scripts, and it doesn't seem worth the benefit --- this case seems fairly analogous to SELECT, for which we also suppress the command status. Kumar Rajeev Rastogi, with substantial review by Amit Khandekar
1 parent 7bae028 commit f70a78b

File tree

5 files changed

+82
-48
lines changed

5 files changed

+82
-48
lines changed

‎doc/src/sgml/ref/copy.sgml

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -370,6 +370,16 @@ COPY <replaceable class="parameter">count</replaceable>
370370
The <replaceable class="parameter">count</replaceable> is the number
371371
of rows copied.
372372
</para>
373+
374+
<note>
375+
<para>
376+
<application>psql</> will print this command tag only if the command
377+
was not <literal>COPY ... TO STDOUT</>, or the
378+
equivalent <application>psql</> meta-command
379+
<literal>\copy ... to stdout</>. This is to prevent confusing the
380+
command tag with the data that was just printed.
381+
</para>
382+
</note>
373383
</refsect1>
374384

375385
<refsect1>

‎doc/src/sgml/ref/psql-ref.sgml

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -863,36 +863,36 @@ testdb=&gt;
863863
<para>
864864
When <literal>program</> is specified,
865865
<replaceable class="parameter">command</replaceable> is
866-
executed by <application>psql</application> and the data from
866+
executed by <application>psql</application> and the data passed from
867867
or to <replaceable class="parameter">command</replaceable> is
868868
routed between the server and the client.
869-
This means that the execution privileges are those of
869+
Again, the execution privileges are those of
870870
the local user, not the server, and no SQL superuser
871871
privileges are required.
872872
</para>
873873

874-
<para><literal>\copy ... from stdin | to stdout</literal>
875-
reads/writes based on the command input and output respectively.
876-
All rows are read from the same source that issued the command,
877-
continuing until <literal>\.</literal> is read or the stream
878-
reaches <acronym>EOF</>. Output is sent to the same place as
879-
command output. To read/write from
880-
<application>psql</application>'s standard input or output, use
881-
<literal>pstdin</> or <literal>pstdout</>. This option is useful
874+
<para>
875+
For <literal>\copy ... from stdin</>, data rows are read from the same
876+
source that issued the command, continuing until <literal>\.</literal>
877+
is read or the stream reaches <acronym>EOF</>. This option is useful
882878
for populating tables in-line within a SQL script file.
879+
For <literal>\copy ... to stdout</>, output is sent to the same place
880+
as <application>psql</> command output, and
881+
the <literal>COPY <replaceable>count</></literal> command status is
882+
not printed (since it might be confused with a data row).
883+
To read/write <application>psql</application>'s standard input or
884+
output regardless of the current command source or <literal>\o</>
885+
option, write <literal>from pstdin</> or <literal>to pstdout</>.
883886
</para>
884887

885888
<para>
886-
The syntax of the command is similar to that of the
889+
The syntax of this command is similar to that of the
887890
<acronym>SQL</acronym> <xref linkend="sql-copy">
888-
command, and
889-
<replaceable class="parameter">option</replaceable>
890-
must indicate one of the options of the
891-
<acronym>SQL</acronym> <xref linkend="sql-copy"> command.
892-
Note that, because of this,
893-
special parsing rules apply to the <command>\copy</command>
894-
command. In particular, the variable substitution rules and
895-
backslash escapes do not apply.
891+
command. All options other than the data source/destination are
892+
as specified for <xref linkend="sql-copy">.
893+
Because of this, special parsing rules apply to the <command>\copy</>
894+
command. In particular, <application>psql</>'s variable substitution
895+
rules and backslash escapes do not apply.
896896
</para>
897897

898898
<tip>

‎src/bin/psql/common.c

Lines changed: 37 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -632,7 +632,9 @@ StoreQueryTuple(const PGresult *result)
632632
* degenerates to an AcceptResult() call.
633633
*
634634
* Changes its argument to point to the last PGresult of the command string,
635-
* or NULL if that result was for a COPY FROM STDIN or COPY TO STDOUT.
635+
* or NULL if that result was for a COPY TO STDOUT. (Returning NULL prevents
636+
* the command status from being printed, which we want in that case so that
637+
* the status line doesn't get taken as part of the COPY data.)
636638
*
637639
* Returns true on complete success, false otherwise. Possible failure modes
638640
* include purely client-side problems; check the transaction status for the
@@ -641,14 +643,14 @@ StoreQueryTuple(const PGresult *result)
641643
static bool
642644
ProcessResult(PGresult **results)
643645
{
644-
PGresult *next_result;
645646
bool success = true;
646647
bool first_cycle = true;
647648

648-
do
649+
for (;;)
649650
{
650651
ExecStatusType result_status;
651652
bool is_copy;
653+
PGresult *next_result;
652654

653655
if (!AcceptResult(*results))
654656
{
@@ -693,45 +695,65 @@ ProcessResult(PGresult **results)
693695
* otherwise use queryFout or cur_cmd_source as appropriate.
694696
*/
695697
FILE *copystream = pset.copyStream;
698+
PGresult *copy_result;
696699

697700
SetCancelConn();
698701
if (result_status == PGRES_COPY_OUT)
699702
{
700703
if (!copystream)
701704
copystream = pset.queryFout;
702705
success = handleCopyOut(pset.db,
703-
copystream) && success;
706+
copystream,
707+
&copy_result) && success;
708+
709+
/*
710+
* Suppress status printing if the report would go to the same
711+
* place as the COPY data just went. Note this doesn't
712+
* prevent error reporting, since handleCopyOut did that.
713+
*/
714+
if (copystream == pset.queryFout)
715+
{
716+
PQclear(copy_result);
717+
copy_result = NULL;
718+
}
704719
}
705720
else
706721
{
707722
if (!copystream)
708723
copystream = pset.cur_cmd_source;
709724
success = handleCopyIn(pset.db,
710725
copystream,
711-
PQbinaryTuples(*results)) && success;
726+
PQbinaryTuples(*results),
727+
&copy_result) && success;
712728
}
713729
ResetCancelConn();
714730

715731
/*
716-
* Call PQgetResult() once more. In the typical case of a
717-
* single-command string, it will return NULL. Otherwise, we'll
718-
* have other results to process that may include other COPYs.
732+
* Replace the PGRES_COPY_OUT/IN result with COPY command's exit
733+
* status, or with NULL if we want to suppress printing anything.
719734
*/
720735
PQclear(*results);
721-
*results = next_result = PQgetResult(pset.db);
736+
*results = copy_result;
722737
}
723738
else if (first_cycle)
739+
{
724740
/* fast path: no COPY commands; PQexec visited all results */
725741
break;
726-
else if ((next_result = PQgetResult(pset.db)))
727-
{
728-
/* non-COPY command(s) after a COPY: keep the last one */
729-
PQclear(*results);
730-
*results = next_result;
731742
}
732743

744+
/*
745+
* Check PQgetResult() again. In the typical case of a single-command
746+
* string, it will return NULL. Otherwise, we'll have other results
747+
* to process that may include other COPYs. We keep the last result.
748+
*/
749+
next_result = PQgetResult(pset.db);
750+
if (!next_result)
751+
break;
752+
753+
PQclear(*results);
754+
*results = next_result;
733755
first_cycle = false;
734-
} while (next_result);
756+
}
735757

736758
/* may need this to recover from conn loss during COPY */
737759
if (!first_cycle && !CheckConnection())

‎src/bin/psql/copy.c

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -429,16 +429,17 @@ do_copy(const char *args)
429429
* conn should be a database connection that you just issued COPY TO on
430430
* and got back a PGRES_COPY_OUT result.
431431
* copystream is the file stream for the data to go to.
432+
* The final status for the COPY is returned into *res (but note
433+
* we already reported the error, if it's not a success result).
432434
*
433435
* result is true if successful, false if not.
434436
*/
435437
bool
436-
handleCopyOut(PGconn *conn, FILE *copystream)
438+
handleCopyOut(PGconn *conn, FILE *copystream, PGresult **res)
437439
{
438440
bool OK = true;
439441
char *buf;
440442
int ret;
441-
PGresult *res;
442443

443444
for (;;)
444445
{
@@ -485,13 +486,12 @@ handleCopyOut(PGconn *conn, FILE *copystream)
485486
* but hasn't exited COPY_OUT state internally. So we ignore the
486487
* possibility here.
487488
*/
488-
res = PQgetResult(conn);
489-
if (PQresultStatus(res) != PGRES_COMMAND_OK)
489+
*res = PQgetResult(conn);
490+
if (PQresultStatus(*res) != PGRES_COMMAND_OK)
490491
{
491492
psql_error("%s", PQerrorMessage(conn));
492493
OK = false;
493494
}
494-
PQclear(res);
495495

496496
return OK;
497497
}
@@ -504,6 +504,8 @@ handleCopyOut(PGconn *conn, FILE *copystream)
504504
* and got back a PGRES_COPY_IN result.
505505
* copystream is the file stream to read the data from.
506506
* isbinary can be set from PQbinaryTuples().
507+
* The final status for the COPY is returned into *res (but note
508+
* we already reported the error, if it's not a success result).
507509
*
508510
* result is true if successful, false if not.
509511
*/
@@ -512,12 +514,11 @@ handleCopyOut(PGconn *conn, FILE *copystream)
512514
#define COPYBUFSIZ 8192
513515

514516
bool
515-
handleCopyIn(PGconn *conn, FILE *copystream, bool isbinary)
517+
handleCopyIn(PGconn *conn, FILE *copystream, bool isbinary, PGresult **res)
516518
{
517519
bool OK;
518520
const char *prompt;
519521
char buf[COPYBUFSIZ];
520-
PGresult *res;
521522

522523
/*
523524
* Establish longjmp destination for exiting from wait-for-input. (This is
@@ -679,21 +680,20 @@ handleCopyIn(PGconn *conn, FILE *copystream, bool isbinary)
679680
* connection is lost. But that's fine; it will get us out of COPY_IN
680681
* state, which is what we need.)
681682
*/
682-
while (res = PQgetResult(conn), PQresultStatus(res) == PGRES_COPY_IN)
683+
while (*res = PQgetResult(conn), PQresultStatus(*res) == PGRES_COPY_IN)
683684
{
684685
OK = false;
685-
PQclear(res);
686+
PQclear(*res);
686687
/* We can't send an error message if we're using protocol version 2 */
687688
PQputCopyEnd(conn,
688689
(PQprotocolVersion(conn) < 3) ? NULL :
689690
_("trying to exit copy mode"));
690691
}
691-
if (PQresultStatus(res) != PGRES_COMMAND_OK)
692+
if (PQresultStatus(*res) != PGRES_COMMAND_OK)
692693
{
693694
psql_error("%s", PQerrorMessage(conn));
694695
OK = false;
695696
}
696-
PQclear(res);
697697

698698
return OK;
699699
}

‎src/bin/psql/copy.h

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,13 @@
1212

1313

1414
/* handler for \copy */
15-
bool do_copy(const char *args);
15+
extern bool do_copy(const char *args);
1616

1717
/* lower level processors for copy in/out streams */
1818

19-
bool handleCopyOut(PGconn *conn, FILE *copystream);
20-
bool handleCopyIn(PGconn *conn, FILE *copystream, bool isbinary);
19+
extern bool handleCopyOut(PGconn *conn, FILE *copystream,
20+
PGresult **res);
21+
extern bool handleCopyIn(PGconn *conn, FILE *copystream, bool isbinary,
22+
PGresult **res);
2123

2224
#endif

0 commit comments

Comments
 (0)