Re: [PATCH] server_version_num should be GUC_REPORT

On 9 January 2015 at 22:53, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Craig Ringer <craig(at)2ndquadrant(dot)com> writes:
> > While looking into client code that relies on parsing server_version
> > instead of checking server_version_num, I was surprised to discover that
> > server_version_num isn't sent to the client by the server as part of the
> > standard set of parameters reported post-auth.
>
> Why should it be? server_version is what's documented to be sent.
>

Because it makes sense to send it - it's obviously useful for clients, and
the same rationale that justifies adding server_version_num as a GUC
("parsing text versions is annoying, slow, and flakey") would also be a
good reason to send it to clients on first connection.

If the protocol were being designed now, frankly I think it'd make sense to
send *only* server_version_num and let the client query for the full text
version if it wants it. Bit late for that though.

Anyway, apply this and server_version_num *is* documented to be sent ;-)

>
> > The attached patch marks server_version_num GUC_REPORT and documents
> > that it's reported to the client automatically.
>
> I think this is just a waste of network bandwidth. No client-side code
> could safely depend on its being available for many years yet, therefore
> they're going to keep using server_version.
>

By the same logic, isn't server_version_num its self useless, since no
client can rely on it being present so it'll just use server_version?

server_version_num was added in 8.2. It's now present in all even remotely
interesting PostgreSQL versions and can be relied upon to be present. Back
in 2006 you argued against its addition using much the same rationale you
are doing for this now:
http://www.postgresql.org/message-id/21507.1154223850@sss.pgh.pa.us . Time
flies; now it's useful and can be reasonably relied upon.

In the mean time a client can use server_version_num if sent, and fall back
to server_version otherwise. Just like you already have to do if querying
it via current_setting(...). This means that at least when connecting to
newer servers clients no longer have to do any stupid dances around parsing
"9.5beta1", "9.4.0mycustompatchedPg", etc.

Having this sent by GUC_REPORT would be very helpful for PgJDBC. It'd let
the driver avoid some round trips while being smarter about handling of
custom version strings.

The server's GUC_REPORT output and other startup content is 331 bytes of
payload according to Wireshark. Sending server_version_num will add, what,
26 bytes? That's not completely negligible, but compared to the *real*
costs paid in round-trips it's pretty tiny.

I expected this to be uncontroversial to the point where it'd just be
committed on the spot. As that is not the case I will add it to the next
commitfest.

--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services