Re: psql COPY vs. ON_ERROR_ROLLBACK, multi-command strings

On Wed, Mar 07, 2012 at 04:57:12PM -0500, Robert Haas wrote:
> On Sat, Feb 25, 2012 at 12:57 AM, Noah Misch <noah(at)leadboat(dot)com> wrote:
> > Thanks. ?While testing a crashing function, I noticed that my above patch
> > added some noise to psql output when the server crashes:
> >
> > [local] test=# select crashme();
> > The connection to the server was lost. Attempting reset: Failed.
> > The connection to the server was lost. Attempting reset: Failed.
> > unexpected transaction status (4)
> > Time: 6.681 ms
> > ?!> \q
> >
> > Status 4 is PQTRANS_UNKNOWN, which is expected when the connection is not
> > CONNECTION_OK. ?The double message arrives because ProcessResult() now calls
> > CheckConnection() at least twice, for the benefit of COPY. ?(Incidentally, the
> > reconnect fails because the server has not yet finished recovering; that part
> > is nothing new.)
> >
> > The attached small patch has SendQuery() keep quiet about PQTRANS_UNKNOWN when
> > the connection is down. ?It makes ProcessResult() skip the second
> > CheckConnection() when the command string had no COPY results. ?This restores
> > the pre-08146775acd8bfe0fcc509c71857abb928697171 psql output:
> >
> > [local] test=# select crashme();
> > The connection to the server was lost. Attempting reset: Failed.
> > Time: 4.798 ms
> > ?!> \q
>
> Committed, but... why do we EVER need to call CheckConnection() at the
> bottom of ProcessResult(), after exiting that function's main loop? I
> don't see any way to get out of that loop without first calling
> AcceptResult on every PGresult we've seen, and that function already
> calls CheckConnection() if any failure is observed.

You can reproduce a case where it helps by sending SIGKILL to a backend
serving a long-running COPY TO, such as this:

copy (select n, pg_sleep(case when n > 1000 then 1 else 0 end)
from generate_series(1,1030) t(n)) to stdout;

The connection dies during handleCopyOut(). By the time control returns to
ProcessResult(), there's no remaining PGresult to check. Only that last-ditch
CheckConnection() notices the lost connection.

[I notice more examples of poor error reporting cosmetics in some of these
COPY corner cases, so I anticipate another patch someday.]

> As a side note, the documentation for PQexec() is misleading about
> what will happen if COPY is present in a multi-command string. It
> says: "Note however that the returned PGresult structure describes
> only the result of the last command executed from the string. Should
> one of the commands fail, processing of the string stops with it and
> the returned PGresult describes the error condition. It does not
> explain that it also stops if it hits a COPY. I had to read the
> source code for libpq to understand why this psql logic was coded the
> way it is.

Agreed; I went through a similar process. Awhile after reading the code, I
found the behavior documented in section "Functions Associated with the COPY
Command":

If a COPY command is issued via PQexec in a string that could contain
additional commands, the application must continue fetching results via
PQgetResult after completing the COPY sequence. Only when PQgetResult
returns NULL is it certain that the PQexec command string is done and it is
safe to issue more commands.

I'm not quite sure what revision would help most here -- a cross reference,
moving that content, duplicating that content. Offhand, I'm inclined to move
it to the PQexec() documentation with some kind of reference back from its
original location. Thoughts?

Thanks,
nm