#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)
Change History (11)
by , 16 years ago
Attachment: | fix_re_django-1.0.2.diff added |
---|
comment:1 by , 16 years ago
milestone: | → 1.1 |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 16 years ago
Patch needs improvement: | set |
---|
This patch needs improvement for a couple of reasons:
- 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.
- 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 inoperations.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 , 16 years ago
Cc: | 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 , 16 years ago
Attachment: | fix_unify_pgsql_re-option_a-django-1.1-dev.diff added |
---|
by , 16 years ago
Attachment: | fix_unify_pgsql_re-option_b-django-1.1-dev.diff added |
---|
comment:4 by , 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 , 16 years ago
With respect to the 1.1 deadline, there are two issues here:
- Making 8.4beta1 (and other similarly labeled versions) parse ok
- 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 , 16 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
fixes for django 1.0.2