Re: libpq COPY handling

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: libpq COPY handling
Date: 2013-04-25 17:56:53
Message-ID: 13826.1366912613@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> Noah Misch pointed out something interesting to me:
> /*
> * PQputCopyEnd - send EOF indication to the backend during COPY IN
> *
> * After calling this, use PQgetResult() to check command completion status.
> *
> * Returns 1 if successful, 0 if data could not be sent (only possible
> * in nonblock mode), or -1 if an error occurs.
> */

> The comment alleges that 0 is a possible return value, but the only
> return statements in the code for that function return literal values
> of either 1 or -1. I'm not sure whether that's a bug in the code or
> the documentation.

Hm. pqFlush has a three-way return value which PQputCopyEnd is only
checking as a boolean. Probably the intent was to return "data not
sent" if pqFlush reports that.

However, the documentation in libpq.sgml is a bit bogus too, because it
counsels trying the PQputCopyEnd() call again, which will not work
(since we already changed the asyncStatus). We could make that say "a
zero result is informational, you might want to try PQflush() later".
The trouble with this, though, is that any existing callers that were
coded to the old spec would now be broken.

It might be better to consider that the code is correct and fix the
documentation. I notice that the other places in fe-exec.c that call
pqFlush() generally say "In nonblock mode, don't complain if we're unable
to send it all", which is pretty much the spirit of what this is doing
though it lacks that comment.

> Also, I noticed that there are a few places in fe-protocol3.c that
> seem not to know about COPY-BOTH mode. I'm not sure that any of these
> are actually bugs right now given the current very limited use of
> COPY-BOTH mode, but I'm wondering whether it wouldn't be better to
> minimize the chance of future surprises. Patch attached.

+1 for these changes, anyway.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2013-04-25 18:50:21 Re: [ADMIN] Simultaneous index creates on different schemas cause deadlock?
Previous Message Timothy Garnett 2013-04-25 17:56:22 Re: Allowing parallel pg_restore from pipe