On Thu, 6 Jun 2024 at 03:03, Robert Haas <robertmhaas@gmail.com> wrote:
> This makes some sense to me. I don't think that I believe
> max_protocol_version=latest is future-proof: just because I want to
> opt into this round of new features doesn't mean I feel the same way
> about the next round. But it may still be a good idea.
I think for most people the only reason not to opt-in to improvements
(even if they are small) is if those improvements break something
else. Once the NegotiateProtocolVersion message is implemented
everywhere in the ecosystem, nothing should break when going from e.g.
3.2 to 3.4. So for the majority of the people I think
max_protocol_version=latest is what they'll want to use once the
ecosystem has caught up. Of course there will be people that want
tight control, but they can set max_protocol_version=3.2 instead.
> I suppose the semantics are that we try to connect with the version
> specified by max_protocol_version and, if we get downgraded by the
> server, we abort if we end up below min_protocol_version.
Correct
> I like those
> semantics, and I think I also like having both parameters, but I'm not
> 100% sure of the naming. It's a funny use of "max" and "min", because
> the max is really what we're trying to do and the min is what we end
> up with, and those terms don't necessarily bring those ideas to mind.
> I don't have a better idea off-hand, though.
I borrowed this terminology from the the ssl_min_protocol_version and
ssl_max_protocol_version connection options that we already have.
Those basically have the same semantics as what I'm proposing here,
but for the TLS protocol version instead of the Postgres protocol
version. I'm also not a huge fan of the min_protocol_version and
max_protocol_version names, but staying consistent with existing
options seems quite nice.
Looking at ssl_max_protocol_version closer though, to stay really
consistent I'd have to change "latest" to be renamed to empty string
(i.e. there is no max_protocol_version). I think I might prefer
staying consistent over introducing an imho slightly clearer name.
Another way to stay consistent would of course be also adding "latest"
as an option to ssl_max_protocol_version? What do you think?
I'll look into adding min_protocol_version to the patchset soonish.
Some review of the existing code in the first few patches would
definitely be appreciated.