Edgewall Software

Opened 6 years ago

Closed 6 years ago

#13032 closed defect (fixed)

Postgres server version reported incorrectly for psql >= 10 — at Version 6

Reported by: Ryan J Ollos Owned by: Ryan J Ollos
Priority: normal Milestone: 1.3.3
Component: general Version:
Severity: normal Keywords:
Cc: Branch:
Release Notes:

Fixed PostgreSQL server version reported incorrectly for version ≥ 10.

API Changes:
Internal Changes:

Description (last modified by 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

    diff --git a/trac/db/postgres_backend.py b/trac/db/postgres_backend.py
    index c2e94becc..2647f9b9d 100644
    a b def _quote(identifier):  
    9999def _version_tuple(ver):
    100100
    101101    if ver:
    102         # Extract 9.1.23 from 90123.
    103102        def get_digit(version, n):
    104103            return version / 10 ** (2 * n) % 100
    105         return get_digit(ver, 2), get_digit(ver, 1), get_digit(ver, 0)
     104        first_digit = get_digit(ver, 2)
     105        if first_digit >= 10:
     106            # Extract (10, 4) from 100004.
     107            return first_digit, get_digit(ver, 0)
     108        else:
     109            # Extract (9, 1, 23) from 90123.
     110            return first_digit, get_digit(ver, 1), get_digit(ver, 0)
    106111
    107112
    108113def _version_string(ver):

A few other minor changes may also be needed.

Change History (6)

comment:1 by Ryan J Ollos, 6 years ago

Description: modified (diff)

comment:2 by anonymous, 6 years ago

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

in reply to:  2 comment:3 by Jun Omae, 6 years ago

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

    diff --git a/trac/db/postgres_backend.py b/trac/db/postgres_backend.py
    index c2e94becc..bb3922f32 100644
    a b class PostgreSQLConnector(Component):  
    146146        cnx = PostgreSQLConnection(path, log, user, password, host, port,
    147147                                   params)
    148148        if not self.required:
     149            server_ver = _version_string(cnx.server_version)
     150            client_ver = _version_string(self._client_version)
    149151            if cnx.server_version < min_postgresql_version:
    150152                error = _(
    151153                    "PostgreSQL version is %(version)s. Minimum required "
    152                     "version is %(min_version)s.",
    153                     version='%d.%d.%d' % cnx.server_version,
    154                     min_version='%d.%d.%d' % min_postgresql_version)
     154                    "version is %(min_version)s.", version=server_ver,
     155                    min_version=_version_string(min_postgresql_version))
    155156                raise TracError(error)
    156             self._postgresql_version = \
    157                 'server: %s, client: %s' \
    158                 % (_version_string(cnx.server_version),
    159                    _version_string(self._client_version))
     157            self._postgresql_version = 'server: %s, client: %s' % \
     158                                       (server_ver, client_ver)
    160159            self.required = True
    161160        return cnx
    162161

comment:4 by anonymous, 6 years ago

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

comment:5 by Jun Omae, 6 years ago

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

  • trac/db/postgres_backend.py

    diff --git a/trac/db/postgres_backend.py b/trac/db/postgres_backend.py
    index c2e94becc..67491704e 100644
    a b def _quote(identifier):  
    9797
    9898
    9999def _version_tuple(ver):
    100 
    101100    if ver:
    102         # Extract 9.1.23 from 90123.
    103         def get_digit(version, n):
    104             return version / 10 ** (2 * n) % 100
    105         return get_digit(ver, 2), get_digit(ver, 1), get_digit(ver, 0)
     101        major, minor = divmod(ver, 10000)
     102        if major >= 10:
     103            # Extract 10.4 from 100004.
     104            return major, minor
     105        else:
     106            # Extract 9.1.23 from 90123.
     107            minor, patch = divmod(minor, 100)
     108            return major, minor, patch
    106109
    107110
    108111def _version_string(ver):
  • trac/db/tests/postgres_test.py

    diff --git a/trac/db/tests/postgres_test.py b/trac/db/tests/postgres_test.py
    index 88199d1d0..619cad948 100644
    a b from trac.db.api import DatabaseManager  
    1919from trac.db.schema import Table, Column, Index
    2020from trac.test import EnvironmentStub, get_dburi
    2121try:
    22     from trac.db.postgres_backend import PostgreSQLConnector, assemble_pg_dsn
     22    from trac.db.postgres_backend import (PostgreSQLConnector, _version_tuple,
     23                                          assemble_pg_dsn)
    2324except pkg_resources.DistributionNotFound:
    24     PostgreSQLConnector = assemble_pg_dsn = None
     25    PostgreSQLConnector = _version_tuple = assemble_pg_dsn = None
    2526
    2627
    2728class PostgresTableCreationSQLTest(unittest.TestCase):
    class PostgresTableCreationSQLTest(unittest.TestCase):  
    142143            ["dbname='/trac'", r"password='pa\\\'ss'", "user='user'"],
    143144            sorted(assemble_pg_dsn('/trac', 'user', r"pa\'ss").split(' ')))
    144145
     146    def test_version_tuple(self):
     147        self.assertEqual((9, 0, 0), _version_tuple(90000))
     148        self.assertEqual((9, 1, 23), _version_tuple(90123))
     149        self.assertEqual((9, 98, 97), _version_tuple(99897))
     150        self.assertEqual((10, 0), _version_tuple(100000))
     151        self.assertEqual((10, 4), _version_tuple(100004))
     152        self.assertEqual((10, 9999), _version_tuple(109999))
     153        self.assertEqual((11, 0), _version_tuple(110000))
     154
    145155
    146156class PostgresTableAlterationSQLTest(unittest.TestCase):
    147157    def setUp(self):

comment:6 by Ryan J Ollos, 6 years ago

Release Notes: modified (diff)
Resolution: fixed
Status: assignedclosed

Thanks for suggestions. Committed in r16623.

Note: See TracTickets for help on using tickets.