Skip to content

Further issues with new major/minor/fix level parsing #256

Closed

Description

@cpaelzer
  • asyncpg version: 0.13
  • PostgreSQL version: 10.1
  • Python version: 2.7.14-4 / 3.6.4-1
  • Platform: Linux Debian sid / Linux Ubuntu Bionic
  • Do you use pgbouncer?: No
  • Did you install asyncpg with pip?: No (package repository)

In Issue #250 was a fix for Debian branding in version 10.1
But I think it has slight issues and there are more issues with 10.1, not related to the branding but the change of major/minor/fixed version handling.

First of all for the change in #251 :
("PostgreSQL 10.1 (Debian 10.1-3)", (10, 1, 0, 'final', 0),),
I ran into a test fail with this:
E AssertionError: (10, 0, 1, 'final', 0) != ServerVersion(major=10, minor=1, micro=0, releaselevel='final', serial=0)
Following the argument in the comment above the code I think it should instead be:
("PostgreSQL 10.1 (Debian 10.1-3)", (10, 0, 1, 'final', 0),),

Further on I ran into a fail with async def test_server_version_01:

  self.assertEqual(version[:3], (ver_maj, ver_min, ver_fix))

E AssertionError: Tuples differ: (10, 1, 0) != (10, 0, 1)

Again - broken due to Postgresql 10's second number means micro.
That means 10.1 now is:
SELECT current_setting('server_version_num'); => 100001
While 9.5.6 for example was 90506
asyncpg/connection.py:161 get_server_version already does it right, as postgres is doing the mapping.
But test_server_version_01 needs to learn the changed behavior.

That would be solved by:

         version_num = await self.con.fetchval("SELECT current_setting($1)",
                                               'server_version_num', column=0)
         ver_maj = int(version_num[:-4])
-        ver_min = int(version_num[-4:-2])
-        ver_fix = int(version_num[-2:])
+        if ver_maj < 10:
+            ver_min = int(version_num[-4:-2])
+            ver_fix = int(version_num[-2:])
+        else:
+            ver_min = int(version_num[-2:])
+            ver_fix = 0
 
         self.assertEqual(version[:3], (ver_maj, ver_min, ver_fix))

Activity

  1. vitaly-burovoy commented on Feb 6, 2018

    @vitaly-burovoy
    Contributor

    Your reported version is 0.13.0, but a fix is present for 0.14.0 by the commit:
    1fa12fe

  2. cpaelzer commented on Feb 6, 2018

    @cpaelzer
    Author

    Hmm I checked latest head if the code looks the same way, maybe it was just fixed in a different place.
    ... checking the commit ...
    @vitaly-burovoy you are right, that looks good - Thanks!
    I'll replace my changes with this (on top of 0.13) and test again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions