Обсуждение: pgsql: Fix an ancient oversight in libpq's handling of V3-protocol COPY

Log Message:
-----------
Fix an ancient oversight in libpq's handling of V3-protocol COPY OUT mode:
we need to be able to swallow NOTICE messages, and potentially also
ParameterStatus messages (although the latter would be a bit weird),
without exiting COPY OUT state.  Fix it, and adjust the protocol documentation
to emphasize the need for this.  Per off-list report from Alexander Galler.

Modified Files:
--------------
    pgsql/doc/src/sgml:
        protocol.sgml (r1.70 -> r1.71)
        (http://developer.postgresql.org/cvsweb.cgi/pgsql/doc/src/sgml/protocol.sgml?r1=1.70&r2=1.71)
    pgsql/src/interfaces/libpq:
        fe-protocol3.c (r1.31 -> r1.32)
        (http://developer.postgresql.org/cvsweb.cgi/pgsql/src/interfaces/libpq/fe-protocol3.c?r1=1.31&r2=1.32)

On Mon, 2008-01-14 at 18:46 +0000, Tom Lane wrote:
> Log Message:
> -----------
> Fix an ancient oversight in libpq's handling of V3-protocol COPY OUT mode:
> we need to be able to swallow NOTICE messages, and potentially also
> ParameterStatus messages (although the latter would be a bit weird),
> without exiting COPY OUT state.  Fix it, and adjust the protocol documentation
> to emphasize the need for this.  Per off-list report from Alexander Galler.

My reading of this is that previously a PQgetCopyData() operation would
fail mysteriously because of

i) a reload of postgresql.conf, following the setting of any of the
following parameters client_encoding, DateStyle, TimeZone, and
standard_conforming_strings.

ii) any previous LISTEN command on the session running the COPY

Would that be correct? Did I miss anything?

--
  Simon Riggs
  2ndQuadrant  http://www.2ndQuadrant.com

Simon Riggs wrote:
> On Mon, 2008-01-14 at 18:46 +0000, Tom Lane wrote:
> > Log Message:
> > -----------
> > Fix an ancient oversight in libpq's handling of V3-protocol COPY OUT mode:
> > we need to be able to swallow NOTICE messages, and potentially also
> > ParameterStatus messages (although the latter would be a bit weird),
> > without exiting COPY OUT state.  Fix it, and adjust the protocol documentation
> > to emphasize the need for this.  Per off-list report from Alexander Galler.
>
> My reading of this is that previously a PQgetCopyData() operation would
> fail mysteriously because of
>
> i) a reload of postgresql.conf, following the setting of any of the
> following parameters client_encoding, DateStyle, TimeZone, and
> standard_conforming_strings.

Hmm, aren't ParameterStatus messages sent just before Ready For Query?

> ii) any previous LISTEN command on the session running the COPY

I don't think LISTEN is involved.  It's NOTICE that's a problem, not
NOTIFY.

--
Alvaro Herrera                                http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

On Tue, 2008-01-15 at 18:22 -0300, Alvaro Herrera wrote:
> Simon Riggs wrote:
> > On Mon, 2008-01-14 at 18:46 +0000, Tom Lane wrote:
> > > Log Message:
> > > -----------
> > > Fix an ancient oversight in libpq's handling of V3-protocol COPY OUT mode:
> > > we need to be able to swallow NOTICE messages, and potentially also
> > > ParameterStatus messages (although the latter would be a bit weird),
> > > without exiting COPY OUT state.  Fix it, and adjust the protocol documentation
> > > to emphasize the need for this.  Per off-list report from Alexander Galler.
> >
> > My reading of this is that previously a PQgetCopyData() operation would
> > fail mysteriously because of
> >
> > i) a reload of postgresql.conf, following the setting of any of the
> > following parameters client_encoding, DateStyle, TimeZone, and
> > standard_conforming_strings.
>
> Hmm, aren't ParameterStatus messages sent just before Ready For Query?

The docs explain they can also be sent in the above case.

> > ii) any previous LISTEN command on the session running the COPY
>
> I don't think LISTEN is involved.  It's NOTICE that's a problem, not
> NOTIFY.

That's what the docs say, but Tom's patch also adds lines to handle
NOTIFY, which is what prompted the question.

--
  Simon Riggs
  2ndQuadrant  http://www.2ndQuadrant.com

Alvaro Herrera <alvherre@commandprompt.com> writes:
> Hmm, aren't ParameterStatus messages sent just before Ready For Query?

No, they're sent immediately (see ReportGUCOption).  I had some ideas
in the back of my head about postponing such sends until just before
Ready For Query, with the idea of avoiding work for repeated changes
to the same variable; but it's not done yet.

            regards, tom lane

Simon Riggs <simon@2ndquadrant.com> writes:
> That's what the docs say, but Tom's patch also adds lines to handle
> NOTIFY, which is what prompted the question.

I don't believe that code can get executed given the current backend
design.  I just put it in because the protocol spec says (and always
has said)

Note: At present, NotificationResponse can only be sent outside a
transaction, and thus it will not occur in the middle of a
command-response series, though it might occur just before
ReadyForQuery. It is unwise to design frontend logic that assumes that,
however. Good practice is to be able to accept NotificationResponse at
any point in the protocol.


            regards, tom lane