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