Opened 16 years ago

Closed 16 years ago

Last modified 13 years ago

#10842 closed (fixed)

Django 1.0.2 generates AttributeError with PostgreSql 8.4beta1

Reported by: Horacio G. de Oro Owned by: nobody
Component: Database layer (models, ORM) Version: 1.0
Severity: Keywords: postgresql
Cc: hgdeoro@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

I get this exception with Django 1.0.2 + PostgrsSql 8.4 beta1 (compiled from source):

  File "/path/to/project/Django-1.0.2/django/db/backends/__init__.py", line 56, in cursor
    cursor = self._cursor(settings)
  File "/path/to/project/Django-1.0.2/django/db/backends/postgresql_psycopg2/base.py", line 92, in _cursor
    self.__class__._version = get_version(cursor)
  File "/path/to/project/Django-1.0.2/django/db/backends/postgresql/version.py", line 16, in get_version
    major, minor = VERSION_RE.search(version).groups()
AttributeError: 'NoneType' object has no attribute 'groups'

The problem is the RE used on django/db/backends/postgresql/version.py (django-1.0.2):

VERSION_RE = re.compile(r'PostgreSQL (\d+)\.(\d+)\.')

When asked for the version, PostgreSql 8.4beta1 returns:
PostgreSQL 8.4beta1 on i686-pc-linux-gnu, compiled by GCC gcc (Ubuntu 4.3.2-1ubuntu12) 4.3.2, 32-bit

The RE used in Django 1.1 works OK:

VERSION_RE = re.compile(r'\S+ (\d+)\.(\d+)')

(the '\S+' is used to support EnterpriseDB, see ticket #8737).

I think django/db/backends/postgresql/operations.py (django-1.0.2) should be modifyed too to make it compatible with EnterpriseDB.

Both files (version.py and operations.py) could be patched with fix_re_django-1.0.2.diff.
I've also attached fix_re_django-1.1-dev.diff that changes operations.py to support EnterpriseDB.

Other tickets (already closed) related to this are: #6433 #9931 #9953

I've tested that the patches are working against PostgreSQL 8.3 (Ubuntu 8.10) and PostgreSQL 8.4beta1 (from sources) but haven't check that those changes works well with EnterpriseDB.

Thanks!

Horacio

Related post: http://groups.google.com/group/django-developers/browse_thread/thread/7826f83429487a07

Attachments (4)

fix_re_django-1.0.2.diff (1.4 KB ) - added by Horacio G. de Oro 16 years ago.
fixes for django 1.0.2
fix_re_django-1.1-dev.diff (1.0 KB ) - added by Horacio G. de Oro 16 years ago.
fixes for django 1.1-dev
fix_unify_pgsql_re-option_a-django-1.1-dev.diff (3.3 KB ) - added by Horacio G. de Oro 16 years ago.
fix_unify_pgsql_re-option_b-django-1.1-dev.diff (6.1 KB ) - added by Horacio G. de Oro 16 years ago.

Download all attachments as: .zip

Change History (11)

by Horacio G. de Oro, 16 years ago

Attachment: fix_re_django-1.0.2.diff added

fixes for django 1.0.2

by Horacio G. de Oro, 16 years ago

Attachment: fix_re_django-1.1-dev.diff added

fixes for django 1.1-dev

comment:1 by Karen Tracey, 16 years ago

milestone: 1.1
Triage Stage: UnreviewedAccepted

comment:2 by Malcolm Tredinnick, 16 years ago

Patch needs improvement: set

This patch needs improvement for a couple of reasons:

  1. It's identifying that a big part of the problem is two regular expressions doing the same thing. So we should use the same code paths both times.
  2. The comment before the regular expression in version.py shows that EnterpriseDB (sic) doesn't return a micro version number and that could be problematic where the code in operations.py is comparing to the third component in the _postgres_version attribute (that won't work for "Enterprise" even now, but since we're fixing this, let's fix it properly).

comment:3 by Horacio G. de Oro, 16 years ago

Cc: hgdeoro@… added

I've uploaded two patches that could help with this ticket, showing 2 ways to fix it:

fix_unify_pgsql_re-option_a-django-1.1-dev.diff:

  • unify RE and change get_version() to take a "precision" parameters; with precision=2, get_version() returns the 2 first numbers of the PostgreSql version, with precision=3, returns the 3 numbers (needed in one place).

fix_unify_pgsql_re-option_b-django-1.1-dev.diff (it is ready to be applied)

  • unify RE and change get_version() always returns the 3 numbers
  • fix an error while checking for PostgreSql >= 8.1
  • changes docs on get_version(), since now we have 3 version numbers (taken from http://www.postgresql.org/support/versioning, there they call major version to the first 2 numbers, and minor version to the third)

The problem is that in some cases the 3rd number of the version would not exists (ej: 'PostgreSql 8.4beta1'), in this cases get_version() returns None in the 3rd element, that's why I've chenged the comparison, for instance:

if self._version < (8, 0):

to:

if self._version[0:2] < (8, 0):

since self._version could be: (8,1,None)

If the 1rs patch is better, I could finish them, but IMHO I think the 2nd is better, and seems to solve the problem with EnterpriseDB that doesn't have the 3rd number of the version. I've no time to test with EnterpriseDB, I'll try to do it this week.

by Horacio G. de Oro, 16 years ago

by Horacio G. de Oro, 16 years ago

comment:4 by Malcolm Tredinnick, 16 years ago

The second approach is preferable, but I've been wondering if we can encapsulate the version comparison a bit more. Into a function that tells us whether the current version is before or after what we're looking for.

The motivation is that we really want to make sure that 8.4 beta1 evaluates as "before" 8.4.0 and 8.4-enterprise and so on. We're likely to be tweaking things a bit over time and pure lexicographical comparison isn't going to fix things completely. So let's make a function that is passed the version we are comparing for as a triple (e.g. (8, 4, 0)) and returns -1, 0, 1 if we are before, equal to or after that version. Alternatively, it could always check for "before" (or "after" -- implementer's choice, but we don't need both versions) the version passed in and return True or False. Then we can put function in one location and everything else passes in the minimum version they want and operate appropriately. The comparison version works better with the aggregate-functionality test we have (excluding some 8.2.x versions), but the boolean version could work with that as well.

Since 8.4beta1 is still not a final release, we've got time to fix this properly and I'd like to do so. Appreciate the time you've put in on the patches so far. It's getting closer. The second patch looks pretty good at the moment.

comment:5 by Russell Keith-Magee, 16 years ago

With respect to the 1.1 deadline, there are two issues here:

  1. Making 8.4beta1 (and other similarly labeled versions) parse ok
  2. Improving the version comparison capabilities.


(1) is a must have for v1.1; (2) is certainly nice to have, but we can live without. I'm about to commit a fix for (1); I've opened #11065 to track (2).

comment:6 by Russell Keith-Magee, 16 years ago

Resolution: fixed
Status: newclosed

(In [10730]) Fixed #10842 -- Corrected parsing of version numbers for PostgreSQL 8.4beta series. Thanks to hgdeoro for the report and fix.

comment:7 by Jacob, 13 years ago

milestone: 1.1

Milestone 1.1 deleted

Note: See TracTickets for help on using tickets.
Back to Top