Re: Correct documentation for protocol version - Mailing list pgsql-hackers

On 2025/04/11 18:27, Dave Cramer wrote:
> 
> 
> On Fri, 11 Apr 2025 at 05:05, Fujii Masao <masao.fujii@oss.nttdata.com <mailto:masao.fujii@oss.nttdata.com>> wrote:
> 
> 
> 
>     On 2025/04/11 5:17, Dave Cramer wrote:
>      > No, you are correct.
>      >
>      > See new patch
> 
>     Thanks for updating the patch!
> 
>     -         Identifies the message as a protocol version negotiation
>     +         Identifies the message as a protocol version negotiation.
>     +         The server sends this message if the requested protocol is
>     +         not equal to the version the server supports or the client
>     +         requests protocol options that are not recognized.
>                 message.
> 
>     You added the sentence starting with "The server sends..."
>     between "negotiation" and "message", but it should be placed
>     after "message", right?
> 
>     Even though the requested version is not equal to the latest
>     version that the server supports, if it's older than
>     the latest one, the message is not sent. So how about
>     wording it like this instead:
> 
>     -------------
>     Identifies the message as a protocol version negotiation message.
>     The server sends this message when the client requests a newer
>     protocol version than the server supports, or when the client
>     includes protocol options that the server does not recognize.
>     -------------
> 
>     +         The protcol version requested by the client unless it is higher than the
>     +         latest version we support in which case the latest protocol version we support.
> 
>     Maybe rewording this for clarity and using “the server
>     instead of “we” would help. For example:
> 
>     -------------
>     The latest protocol version supported by the server if the client
>     requests a newer protocol version than the server supports.
>     The protocol version requested by the client, otherwise.
>     -------------
> 
> 
> Reworded as suggested

Thanks for updating the patch!


While checking the code in older branches, I noticed that the returned
protocol version is always the latest version supported by the server.
However, as we discussed, in master, the server may return the version
requested by the client. The change was introduced in commit 516b87502dc.
So, probably we'll need to update the documentation differently for
master and the older branches.


The patch adds a new explanation about when the NegotiateProtocolVersion
message is sent. But a similar explanation already exists in protocol.sgml:

       <term>NegotiateProtocolVersion</term>
       <listitem>
        <para>
         The server does not support the minor protocol version requested
         by the client, but does support an earlier version of the protocol;
         this message indicates the highest supported minor version.  This
         message will also be sent if the client requested unsupported protocol
         options (i.e., beginning with <literal>_pq_.</literal>) in the
         startup packet.

Given that, I'm now wondering if the new description in the patch
might be redundant.


Also, your original concern was that the phrase "Newest minor protocol version"
is inaccurate since the field contains both major and minor version numbers
(e.g., 3.2). However, based on other usage in protocol.sgml and source
comments in related code, "minor version" seems to refer to the full version
like 3.2, i.e., not just the minor part, so we might not need to reword it
after all.

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION