pgsql: Fix an ancient oversight in libpq's handling of V3-protocol COPY

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

Simon Riggs <simon@2ndquadrant.com> writes:

On Mon, 2008-01-14 at 18:46 +0000, Tom Lane wrote:

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

Neither of those events could trigger it, because neither would be
processed midstream during a COPY (in the current code, anyway).
As best I can tell:

* NOTICE messages are a risk, especially if you have a more-verbose-
than-normal client_min_messages setting.

* ParameterStatus could be a risk if a function executed during COPY
tried to change one of the above-mentioned parameters. Since COPY OUT
doesn't fire triggers, I think user-defined datatype output functions
would be the only possible candidates for that.

* LISTEN/NOTIFY isn't a risk because the backend only sends NOTIFY at
transaction end, period.

regards, tom lane

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

I wrote:

* ParameterStatus could be a risk if a function executed during COPY
tried to change one of the above-mentioned parameters. Since COPY OUT
doesn't fire triggers, I think user-defined datatype output functions
would be the only possible candidates for that.

Scratch that --- that thinking is so last-year :-(. COPY OUT from a
SELECT subquery could fire any old function. pg_dump isn't at risk
from that, but other applications would be.

regards, tom lane

On Tue, 2008-01-15 at 16:37 -0500, Tom Lane wrote:

Neither of those events could trigger it, because neither would be
processed midstream during a COPY (in the current code, anyway).

OK, thanks.

As best I can tell:

* NOTICE messages are a risk, especially if you have a more-verbose-
than-normal client_min_messages setting.

* ParameterStatus could be a risk if a function executed during COPY
tried to change one of the above-mentioned parameters. Since COPY OUT
doesn't fire triggers, I think user-defined datatype output functions
would be the only possible candidates for that.

Understood

* LISTEN/NOTIFY isn't a risk because the backend only sends NOTIFY at
transaction end, period.

OK, just seen the comment section above the case statement.

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