Re: libpq support for NegotiateProtocolVersion

On 13.10.22 23:00, Nathan Bossart wrote:
> On Thu, Oct 13, 2022 at 10:33:01AM +0200, Peter Eisentraut wrote:
>> +	if (their_version != conn->pversion)
> 
> Shouldn't this be 'their_version < conn->pversion'?  If the server supports
> a later protocol than what is requested but not all the requested protocol
> extensions, I think libpq would still report "protocol version not
> supported."

Ok, changed.

>> +		appendPQExpBuffer(&conn->errorMessage,
>> +						  libpq_gettext("protocol version not supported by server: client uses %d.%d, server supports %d.%d\n"),
>> +						  PG_PROTOCOL_MAJOR(conn->pversion), PG_PROTOCOL_MINOR(conn->pversion),
>> +						  PG_PROTOCOL_MAJOR(their_version), PG_PROTOCOL_MINOR(their_version));
> 
> Should this match the error in postmaster.c and provide the range of
> versions the server supports?  The FATAL in postmaster.c is for the major
> version, but I believe the same information is relevant when a
> NegotiateProtocolVersion message is sent.
> 
> 		ereport(FATAL,
> 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> 				 errmsg("unsupported frontend protocol %u.%u: server supports %u.0 to %u.%u",

If you increase the libpq minor protocol version and connect to an older 
server, you would get an error like "server supports 3.0 to 3.0", which 
is probably a bit confusing.  I changed it to "up to 3.0" to convey that 
it could be a range.

>> +	else
>> +		appendPQExpBuffer(&conn->errorMessage,
>> +						  libpq_gettext("protocol extension not supported by server: %s\n"), buf.data);
> 
> nitpick: s/extension/extensions

Ok, added proper plural support.

> What if neither the protocol version nor the requested extensions are
> supported?  Right now, I think only the unsupported protocol version is
> supported in that case, but presumably we could report both pretty easily.

Ok, I just appended both error messages in that case.