Re: libpq support for NegotiateProtocolVersion

On 14.11.22 19:11, Jacob Champion wrote:
>> If we want to address this, maybe this should be handled
>> in the polling loop before we pass off the input buffer to the
>> per-message-type handlers.
> 
> I thought it was supposed to be handled by this code:
> 
>>     /*
>>      * Can't process if message body isn't all here yet.
>>      */
>>     msgLength -= 4;
>>     avail = conn->inEnd - conn->inCursor;
>>     if (avail < msgLength)
>>     {
>>         /*
>>          * Before returning, try to enlarge the input buffer if
>>          * needed to hold the whole message; see notes in
>>          * pqParseInput3.
>>          */
>>         if (pqCheckInBufferSpace(conn->inCursor + (size_t) msgLength,
>>                      conn))
>>             goto error_return;
>>         /* We'll come back when there is more data */
>>         return PGRES_POLLING_READING;
>>     }
> 
> But after this block, we still treat EOF as if we need to get more data.
> If we know that the message was supposed to be fully buffered, can we
> just avoid the return to the pooling loop altogether and error out
> whenever we see EOF?

I agree this doesn't make sense together.  Digging through the history, 
this code is ancient and might have come about during the protocol 2/3 
transition.  (Protocol 2 didn't have length fields in the message IIRC.)

I think for the current code, the following would be an appropriate 
adjustment:

diff --git a/src/interfaces/libpq/fe-connect.c 
b/src/interfaces/libpq/fe-connect.c
index 746e9b4f1efc..d15fb96572d9 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -3412,8 +3412,7 @@ PQconnectPoll(PGconn *conn)
                 /* Get the type of request. */
                 if (pqGetInt((int *) &areq, 4, conn))
                 {
-                   /* We'll come back when there are more data */
-                   return PGRES_POLLING_READING;
+                   goto error_return;
                 }
                 msgLength -= 4;

And then the handling of the 'v' message in my patch would also be 
adjusted like that.