Hi all

It's recently been observed[1] that the 10.0 version scheme change has
affected PostGIS, which relies on parsing the server version string
and broke when faced with a string like "10.0devel" since it expected
a minor version.

In that thread Tom points out [2] that they should be using
PG_VERSION_NUM from the Makefile, or PG_CATALOG_VERSION from the
headers.

The same sort of problems apply to network clients, but network
clients don't currently get that option because we only send
server_version on the wire in the startup packet. We don't send
server_version_num.

It'll be immediately useful to have this since clients can test for
it, use it if there, and fall back to their old version parsing code
if there's no server_version_num. Version 10.0 is the perfect time to
do this since many clients will need to update their version handling
anyway, and we can just tell them to use server_version_num now.

I'm a PgJDBC committer (albeit largely inactive lately), so I'm
thoroughly familiar with at least one client, and I'd really like to
be able to have PgJDBC able to prefer server_version_num. That's how I
originally started looking at this. Also, clients relying on
server_version makes configure's --with-extra-version nearly unusable
in practice since if you build Pg 9.4.9-mypatch a bunch of clients
fall over, as I discovered when working on BDR.

I'm not convinced by the cost concerns that originally caused it not
to be made GUC_REPORT [3], or at least that they still apply today.
Since that change 10 years ago networks have changed a lot. More
significantly we've since added both IntervalStyle ([4], df7641e2, Ron
Mayer / Tom) and application_name ([5], 59ed94ad, Marko Kreen / Tom)
as GUC_REPORT params.

The startup packet is 331 bytes on my system, with a short
application_name 'psql', short username 'craig', etc. Adding
server_version_num would push it up ~26 bytes to ~357, a 7% increase
on a packet we send once at connection establishment. Given that
network performance is overwhelmingly dominated by round-trip costs
even on fast local networks this is going to be practically
undetectable. A minimal connect-and-disconnect psql session with no
SSL exchanges 14 packets of 1363 bytes (TCP level), of which 670 bytes
are server -> client. So that's a 4% increase on the network size of
the most utterly trivial conversation possible, with zero new packets
and zero new round trips.

Unsurprisingly the pgbench effects are undetectable.

Compare that to the effects of [6] pipelining work on the protocol,
where I got speedups of 300x or more by tackling round trip costs.

Can we please send server_version_num on the wire for 10.0? Patches attached.

(BTW, I noticed while prepping that patch that we have identically
duplicated docs for GUC_REPORT params in protocol.sgml and
libpq.sgml.)

[1] 
https://www.postgresql.org/message-id/000001d2014c$f84b9190$e8e2b4b0$@pcorp.us
[2] https://www.postgresql.org/message-id/1585.1472410...@sss.pgh.pa.us
[3] https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=e35ea51
[4] https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=df7641e2
[5] https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=59ed94ad
[6] https://commitfest.postgresql.org/10/634/

-- 
 Craig Ringer                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services
From 818fdc00c0f0f9d98082830c4e9fdc40e9083859 Mon Sep 17 00:00:00 2001
From: Craig Ringer <cr...@2ndquadrant.com>
Date: Mon, 29 Aug 2016 11:31:52 +0800
Subject: [PATCH 1/2] Report server_version_num alongside server_version in
 startup packet

We expose PG_VERSION_NUM in Makefiles and headers and in pg_settings,
but the equivalent server_version_num isn't sent in the startup packet
so clients must rely on parsing the server_version. Make
server_version_num GUC_REPORT so clients can use server_version_num if
present and fall back to server_version for older PostgreSQL versions.
---
 doc/src/sgml/libpq.sgml      | 6 ++++--
 doc/src/sgml/protocol.sgml   | 4 +++-
 src/backend/utils/misc/guc.c | 2 +-
 3 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 2f9350b..5428282 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -1632,6 +1632,7 @@ const char *PQparameterStatus(const PGconn *conn, const char *paramName);
       <para>
        Parameters reported as of the current release include
        <varname>server_version</>,
+       <varname>server_version_num</>,
        <varname>server_encoding</>,
        <varname>client_encoding</>,
        <varname>application_name</>,
@@ -1647,9 +1648,10 @@ const char *PQparameterStatus(const PGconn *conn, const char *paramName);
        <varname>standard_conforming_strings</> was not reported by releases
        before 8.1;
        <varname>IntervalStyle</> was not reported by releases before 8.4;
-       <varname>application_name</> was not reported by releases before 9.0.)
+       <varname>application_name</> was not reported by releases before 9.0;
+       <varname>server_version_num</> was not reported by releases before 10.0.)
        Note that
-       <varname>server_version</>,
+       <varname>server_version</>, <varname>server_version_num</>,
        <varname>server_encoding</> and
        <varname>integer_datetimes</>
        cannot change after startup.
diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml
index 68b0941..12431dc 100644
--- a/doc/src/sgml/protocol.sgml
+++ b/doc/src/sgml/protocol.sgml
@@ -1091,6 +1091,7 @@
     At present there is a hard-wired set of parameters for which
     ParameterStatus will be generated: they are
     <varname>server_version</>,
+    <varname>server_version_num</>,
     <varname>server_encoding</>,
     <varname>client_encoding</>,
     <varname>application_name</>,
@@ -1106,7 +1107,8 @@
     <varname>standard_conforming_strings</> was not reported by releases
     before 8.1;
     <varname>IntervalStyle</> was not reported by releases before 8.4;
-    <varname>application_name</> was not reported by releases before 9.0.)
+    <varname>application_name</> was not reported by releases before 9.0;
+    <varname>server_version_num</> was not reported by releases before 10.0.)
     Note that
     <varname>server_version</>,
     <varname>server_encoding</> and
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index c5178f7..36e3604 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -2767,7 +2767,7 @@ static struct config_int ConfigureNamesInt[] =
 		{"server_version_num", PGC_INTERNAL, PRESET_OPTIONS,
 			gettext_noop("Shows the server version as an integer."),
 			NULL,
-			GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE
+			GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE | GUC_REPORT
 		},
 		&server_version_num,
 		PG_VERSION_NUM, PG_VERSION_NUM, PG_VERSION_NUM,
-- 
2.5.5

From 96695cf17e4257c2b992df118cf5b0389ef98cf7 Mon Sep 17 00:00:00 2001
From: Craig Ringer <cr...@2ndquadrant.com>
Date: Mon, 29 Aug 2016 18:37:58 +0800
Subject: [PATCH 2/2] Teach libpq to prefer server_version_num if it's sent by
 server

---
 src/interfaces/libpq/fe-exec.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c
index a9ba546..e7601d7 100644
--- a/src/interfaces/libpq/fe-exec.c
+++ b/src/interfaces/libpq/fe-exec.c
@@ -975,6 +975,16 @@ pqSaveParameterStatus(PGconn *conn, const char *name, const char *value)
 		conn->std_strings = (strcmp(value, "on") == 0);
 		static_std_strings = conn->std_strings;
 	}
+	else if (strcmp(name, "server_version_num") == 0)
+	{
+		int cnt,
+			ver;
+
+		cnt = sscanf(value, "%d", &ver);
+		/* failure here seems unlikely; will fall back on server_version */
+		if (cnt > 0)
+			conn->sversion = ver;
+	}
 	else if (strcmp(name, "server_version") == 0)
 	{
 		int			cnt;
@@ -982,6 +992,10 @@ pqSaveParameterStatus(PGconn *conn, const char *name, const char *value)
 					vmin,
 					vrev;
 
+		if (conn->sversion != 0)
+			/* already determined version and it can't change after connect */
+			return;
+
 		cnt = sscanf(value, "%d.%d.%d", &vmaj, &vmin, &vrev);
 
 		if (cnt == 3)
-- 
2.5.5

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to