Postgres server version reported incorrectly for psql >= 10

[Ryan J Ollos]

The About Trac page on my dev machine is showing server: 10.0.4. It should display 10.4:

$ psql -c "SELECT version();" trac tracuser
                                                    version
---------------------------------------------------------------------------------------------------------------
 PostgreSQL 10.4 on x86_64-apple-darwin17.5.0, compiled by Apple LLVM version 9.1.0 (clang-902.0.39.1), 64-bit
(1 row)

I've done only a cursory investigation, but it looks like the ​versioning policy has changed. So for version ≥ 10 we should parse 100004 to tuple (10, 4): tags/trac-1.3.2/trac/db/postgres_backend.py@:454:#L452​.

trac/db/postgres_backend.py

@@ -99 +99 @@
 def _version_tuple(ver):
 
     if ver:
-        # Extract 9.1.23 from 90123.
         def get_digit(version, n):
             return version / 10 ** (2 * n) % 100
-        return get_digit(ver, 2), get_digit(ver, 1), get_digit(ver, 0)
+        first_digit = get_digit(ver, 2)
+        if first_digit >= 10:
+            # Extract (10, 4) from 100004.
+            return first_digit, get_digit(ver, 0)
+        else:
+            # Extract (9, 1, 23) from 90123.
+            return first_digit, get_digit(ver, 1), get_digit(ver, 0)
 
 
 def _version_string(ver):

A few other minor changes may also be needed.

[anonymous]

​psycopg docs on server_version:

See also: ​libpq docs for PQserverVersion() for details.

There:

formed by multiplying the server's major version number by 10000 and adding the minor version number

---

return first_digit, get_digit(ver, 0)

In the (unlikely) case of 100100 that would parse incorrectly to tuple (10, 0) instead of (10, 100).

def get_digit(version, n, f=10000):
    return version / f ** n % f
major = get_digit(ver, 1)
if major >= 10:
    # Extract (10, 4) from 100004.
    return major, get_digit(ver, 0)
else:
    # Extract (9, 1, 23) from 90123.
    return major, get_digit(ver, 1, 100), get_digit(ver, 0, 100)

or just:

major, minor = ver / 10000, ver % 10000
if major >= 10:
    # Extract (10, 4) from 100004.    
    return major, minor
else:
    # Extract (9, 1, 23) from 90123.
    return major, minor / 100, minor % 100

[Jun Omae]

Replying to anonymous:

or just:

major, minor = ver / 10000, ver % 10000
if major >= 10:
    # Extract (10, 4) from 100004.    
    return major, minor
else:
    # Extract (9, 1, 23) from 90123.
    return major, minor / 100, minor % 100

The latter looks good to me.

Also, I think we should use _version_string() rather than directly formatting at the following:

trac/db/postgres_backend.py

@@ -146 +146 @@
         cnx = PostgreSQLConnection(path, log, user, password, host, port,
                                    params)
         if not self.required:
+            server_ver = _version_string(cnx.server_version)
+            client_ver = _version_string(self._client_version)
             if cnx.server_version < min_postgresql_version:
                 error = _(
                     "PostgreSQL version is %(version)s. Minimum required "
-                    "version is %(min_version)s.",
-                    version='%d.%d.%d' % cnx.server_version,
-                    min_version='%d.%d.%d' % min_postgresql_version)
+                    "version is %(min_version)s.", version=server_ver,
+                    min_version=_version_string(min_postgresql_version))
                 raise TracError(error)
-            self._postgresql_version = \
-                'server: %s, client: %s' \
-                % (_version_string(cnx.server_version),
-                   _version_string(self._client_version))
+            self._postgresql_version = 'server: %s, client: %s' % \
+                                       (server_ver, client_ver)
             self.required = True
         return cnx
 

[anonymous]

For Python 3 compatibility it should use from __future__ import division and // instead of /.

[Jun Omae]

We could use divmod rather than incompatible / between python 2 and 3.

trac/db/postgres_backend.py

@@ -97 +97 @@
 
 
 def _version_tuple(ver):
-
     if ver:
-        # Extract 9.1.23 from 90123.
-        def get_digit(version, n):
-            return version / 10 ** (2 * n) % 100
-        return get_digit(ver, 2), get_digit(ver, 1), get_digit(ver, 0)
+        major, minor = divmod(ver, 10000)
+        if major >= 10:
+            # Extract 10.4 from 100004.
+            return major, minor
+        else:
+            # Extract 9.1.23 from 90123.
+            minor, patch = divmod(minor, 100)
+            return major, minor, patch
 
 
 def _version_string(ver):

trac/db/tests/postgres_test.py

@@ -19 +19 @@
 from trac.db.schema import Table, Column, Index
 from trac.test import EnvironmentStub, get_dburi
 try:
-    from trac.db.postgres_backend import PostgreSQLConnector, assemble_pg_dsn
+    from trac.db.postgres_backend import (PostgreSQLConnector, _version_tuple,
+                                          assemble_pg_dsn)
 except pkg_resources.DistributionNotFound:
-    PostgreSQLConnector = assemble_pg_dsn = None
+    PostgreSQLConnector = _version_tuple = assemble_pg_dsn = None
 
 
 class PostgresTableCreationSQLTest(unittest.TestCase):
@@ -142 +143 @@
             ["dbname='/trac'", r"password='pa\\\'ss'", "user='user'"],
             sorted(assemble_pg_dsn('/trac', 'user', r"pa\'ss").split(' ')))
 
+    def test_version_tuple(self):
+        self.assertEqual((9, 0, 0), _version_tuple(90000))
+        self.assertEqual((9, 1, 23), _version_tuple(90123))
+        self.assertEqual((9, 98, 97), _version_tuple(99897))
+        self.assertEqual((10, 0), _version_tuple(100000))
+        self.assertEqual((10, 4), _version_tuple(100004))
+        self.assertEqual((10, 9999), _version_tuple(109999))
+        self.assertEqual((11, 0), _version_tuple(110000))
+
 
 class PostgresTableAlterationSQLTest(unittest.TestCase):
     def setUp(self):

[Ryan J Ollos]

Thanks for suggestions. Committed in r16623.