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.