Re: [HACKERS] [PATCH] Send numeric version to clients
On 30 August 2016 at 00:37, Robert Haas wrote: > Long story short, I kind of agree that it might have been better to > expose server_version_num rather than server_version in the beginning, > but I'm not sure that it really helps anybody now, especially given > our decision to simplify the version number format going forward. OK, that's that then. We can fix this properly when the fabled v4 protocol moves into the real world, and keep hacking around it in the mean time. No point restating my disagreement for the 1000th time, done is done and better things for us all to spend our time on. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Send numeric version to clients
On 30 Aug 2016 9:07 AM, "Dave Cramer" wrote: > > > On 29 August 2016 at 15:42, Tom Lane wrote: >> >> Kevin Grittner writes: >> > Regarding Java, for anything above the driver itself the >> > JDBC API says the DatabaseMetaData class must implement these >> > methods: >> > ... >> > That *should* make it just a problem for the driver itself. That >> > seems simple enough until you check what those methods have been >> > returning so far. >> >> Seems like we just make getDatabaseMinorVersion() return zero for >> any major >= 10. That might not have been the best thing to do in >> a green field, but given the precedent ... >> >> regards, tom lane >> >> > seems to me that it should report 10 for the major and whatever comes after the . for the minor ? IMO it just needs to be consistent with the numeric version we report elsewhere - PG_VERSION_NUM, server_version_num etc. > > Dave Cramer > > da...@postgresintl.com > www.postgresintl.com >
Re: [HACKERS] [PATCH] Send numeric version to clients
On 29 August 2016 at 15:42, Tom Lane wrote: > Kevin Grittner writes: > > Regarding Java, for anything above the driver itself the > > JDBC API says the DatabaseMetaData class must implement these > > methods: > > ... > > That *should* make it just a problem for the driver itself. That > > seems simple enough until you check what those methods have been > > returning so far. > > Seems like we just make getDatabaseMinorVersion() return zero for > any major >= 10. That might not have been the best thing to do in > a green field, but given the precedent ... > > regards, tom lane > > > seems to me that it should report 10 for the major and whatever comes after the . for the minor ? Dave Cramer da...@postgresintl.com www.postgresintl.com
Re: [HACKERS] [PATCH] Send numeric version to clients
Kevin Grittner writes: > Regarding Java, for anything above the driver itself the > JDBC API says the DatabaseMetaData class must implement these > methods: > ... > That *should* make it just a problem for the driver itself. That > seems simple enough until you check what those methods have been > returning so far. Seems like we just make getDatabaseMinorVersion() return zero for any major >= 10. That might not have been the best thing to do in a green field, but given the precedent ... regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Send numeric version to clients
On Mon, Aug 29, 2016 at 11:37 AM, Robert Haas wrote:
> Note that these are all one-liners, and I bet the same is true in
> mostly other languages. Even in notoriously verbose languages like
> Java or Cobol or ADA it can't be very hard.
Regarding Java, for anything above the driver itself the
JDBC API says the DatabaseMetaData class must implement these
methods:
int getDatabaseMajorVersion()
Retrieves the major version number of the underlying database.
int getDatabaseMinorVersion()
Retrieves the minor version number of the underlying database.
String getDatabaseProductVersion()
Retrieves the version number of this database product.
That *should* make it just a problem for the driver itself. That
seems simple enough until you check what those methods have been
returning so far. A somewhat minimal program to return these
values:
import java.sql.*;
public final class PrintVersion
{
public static void main(String[] args) throws Exception
{
Class.forName("org.postgresql.Driver");
Connection con =
DriverManager.getConnection("jdbc:postgresql://localhost/test?user=kgrittn");
DatabaseMetaData dbmd = con.getMetaData();
System.out.println(dbmd.getDatabaseMajorVersion() + " " +
dbmd.getDatabaseMinorVersion());
con.close();
}
}
... outputs this:
9 6
I'm not sure what the right thing to do is here.
--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Send numeric version to clients
On Mon, Aug 29, 2016 at 6:37 PM, Robert Haas wrote: > On Mon, Aug 29, 2016 at 7:12 PM, Tom Lane wrote: > > Craig Ringer writes: > >> 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. > > > > I think that this would merely create an attractive nuisance: people > > would be very likely to omit the "fallback" code and thereby build > > clients that fail for no very good reason on pre-v10 servers. As a > > demonstration that that's not a hypothetical risk, I assert that > > that's exactly what you propose telling them to do: > > > >> 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. > > You know, I've kind of been on Craig's side of this running dispute up > until now, but I have to admit that this seems like an awfully good > argument. The fact is that nobody's going to be able to rely on > server_version_num until 9.6 is long gone - which doesn't mean late > 2021, when official community support will end, but several years > after that, when most everyone has actually stopped using it. Of > course, by that time, assuming we don't backpedal on our decision to > go with this new versioning scheme, three part version numbers will be > equally dead and gone, and it's actually a lot easier to extract the > major version number from the new format than the old. For example, > you can just apply atoi() to it: > > if (atoi(server_version) < 12) > fprintf(stderr, "server is ancient, some features will not work\n"); > > That wouldn't be nearly good enough with three part version numbers > because you need the second component, as well. But all that's going > away now. If you need a port to some other language: > > In Perl, you can test int($server_version). > In Javascript, you can test parseInt(server_version). > In Python, it's a bit harder. But int(re.sub(r'[^\d+]+', '', > server_version)) seems to work. > FWIW, a slightly cleaner but still somewhat meh way would be int(float(server_version)). No need to mess around with regexps and extra imports. Long story short, I kind of agree that it might have been better to > expose server_version_num rather than server_version in the beginning, > but I'm not sure that it really helps anybody now, especially given > our decision to simplify the version number format going forward. > Yeah, that's a strong point. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
Re: [HACKERS] [PATCH] Send numeric version to clients
On Mon, Aug 29, 2016 at 7:12 PM, Tom Lane wrote: > Craig Ringer writes: >> 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. > > I think that this would merely create an attractive nuisance: people > would be very likely to omit the "fallback" code and thereby build > clients that fail for no very good reason on pre-v10 servers. As a > demonstration that that's not a hypothetical risk, I assert that > that's exactly what you propose telling them to do: > >> 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. You know, I've kind of been on Craig's side of this running dispute up until now, but I have to admit that this seems like an awfully good argument. The fact is that nobody's going to be able to rely on server_version_num until 9.6 is long gone - which doesn't mean late 2021, when official community support will end, but several years after that, when most everyone has actually stopped using it. Of course, by that time, assuming we don't backpedal on our decision to go with this new versioning scheme, three part version numbers will be equally dead and gone, and it's actually a lot easier to extract the major version number from the new format than the old. For example, you can just apply atoi() to it: if (atoi(server_version) < 12) fprintf(stderr, "server is ancient, some features will not work\n"); That wouldn't be nearly good enough with three part version numbers because you need the second component, as well. But all that's going away now. If you need a port to some other language: In Perl, you can test int($server_version). In Javascript, you can test parseInt(server_version). In Python, it's a bit harder. But int(re.sub(r'[^\d+]+', '', server_version)) seems to work. In Ruby, server_version.to_i seems to do the trick. Note that these are all one-liners, and I bet the same is true in mostly other languages. Even in notoriously verbose languages like Java or Cobol or ADA it can't be very hard.[1] If you want the major and minor version numbers, you might need to write a subroutine for that containing several lines of code ... but if you're testing for the presence or absence of a feature, that's irrelevant 99% of the time, and in any event, it's probably 2-3 lines of code in most. Note that the C code that implements the version of PQserverVersion() that handles both old and new style version number is 33 lines of code, counting variable declarations, comments, and whitespace. And approximately half of that is for compatibility with the old format. Long story short, I kind of agree that it might have been better to expose server_version_num rather than server_version in the beginning, but I'm not sure that it really helps anybody now, especially given our decision to simplify the version number format going forward. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company [1] I expect someone to argue (at great length?) that Java should not be categorized as a notoriously verbose language. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Send numeric version to clients
Craig Ringer writes: > 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. I think that this would merely create an attractive nuisance: people would be very likely to omit the "fallback" code and thereby build clients that fail for no very good reason on pre-v10 servers. As a demonstration that that's not a hypothetical risk, I assert that that's exactly what you propose telling them to do: > 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. Sure, it'd be great if we'd done it like this to start with. But that ship sailed long ago, and redefining how clients ought to determine server version at this point is just a bad idea. I'm also fairly dubious that this is a large problem; surely most C-coded clients use libpq, for instance, and we already solved this for them in PQserverVersion. Or if they aren't using PQserverVersion, why not? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] [PATCH] Send numeric version to clients
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/01d2014c$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 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); Parameters reported as of the current release include server_version, + server_version_num, server_encoding, client_encoding, application_name, @@ -1647,9 +1648,10 @@ const char *PQparameterStatus(const PGconn *conn, const char *paramName); standard_conforming_strings was not reported by releases before 8.1; IntervalStyle was not reported by releases before 8.4; - application_name was not reported by releases before 9.0.) + application_name was not reported by releases before 9.0; + server_version_num was not reported by releas
