Re: Correct documentation for protocol version

On Fri, 11 Apr 2025 at 09:39, Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>
wrote:

>
>
> On 2025/04/11 18:27, Dave Cramer wrote:
> >
> >
> > On Fri, 11 Apr 2025 at 05:05, Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com
> <mailto:masao(dot)fujii(at)oss(dot)nttdata(dot)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.
>

Well this isn't quite true since if you request 3.0 and have invalid
options it will return 3.0, which is not the highest supported minor
version.

>
> 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.
>

IMO the comments should be changed to reflect reality. If 3.2 is a minor
version what is a major version ?

Dave