Opened 14 years ago

Closed 14 years ago

Last modified 12 years ago

#12137 closed (fixed)

Postgres 8.4 imposes strict typing on CharField and TextField

Reported by: Russell Keith-Magee Owned by: nobody
Component: Database layer (models, ORM) Version: 1.1
Severity: Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Consider the following model:

class MyObj(models.Model):
    CHOICES = (
        ('1', 'first choice')
        ('2', 'second choice')
    )
    choice = models.CharField(max_length=1, choices=CHOICES)

(i.e, a char field whose choices are integers, but stored as strings. Now run two queries. First, query using an integer:

MyObj.objects.filter(choice=1)

This yields the SQL:

('SELECT `myapp_myobj`.`id`, `myapp_myobj`.`choice` FROM `myapp_myobj` WHERE `myapp_myobj`.`choice` = %s ', (1,))

Now, query with an actual string:

MyObj.objects.filter(choice='1')

which yields the SQL:

('SELECT `myapp_myobj`.`id`, `myapp_myobj`.`choice` FROM `myapp_myobj` WHERE `myapp_myobj`.`choice` = %s ', ('1',))

As reported by Bill Freeman on django-users, both examples work find for all databases -- except for Postgres 8.4. Postgres 8.4 imposes strong type checking, and raises an error on the first query.

The fact that the first example (the integer lookup) passes at all is due to the good grace of the databases themselves - logically, I think Postgres 8.4 is correct in declaring this an error. After all, "1" != 1.

I think the fix is pretty simple. CharField doesn't currently have a get_db_prep_value() method, and it should.

Compare and contrast with IntegerField or BooleanField - both these fields have get_db_prep_value() methods that cast the provided value to int() and bool(). CharField (and TextField for that matter) should do the same, but with unicode(). This would force the filter value of 1 into '1', which will be passed to the backend as a string, as it should be.

Attachments (1)

mfiedls.diff (888 bytes ) - added by ke1g 14 years ago.
Strawman patch (against django 1.0.3, django/db/models/fields/init.py).

Download all attachments as: .zip

Change History (6)

comment:1 by Russell Keith-Magee, 14 years ago

Triage Stage: UnreviewedAccepted

by ke1g, 14 years ago

Attachment: mfiedls.diff added

Strawman patch (against django 1.0.3, django/db/models/fields/init.py).

comment:2 by ke1g, 14 years ago

The patch mfields.diff, above, is somewhat tested against PostgreSQL's 8.4 and 8.1.11. That is, my project now works against both versions of the database, having looked at pages in several apps (pages cms, profiles, admin, a custom app).
It is above my djangoiness to know whether it would have been better to have used "str" than "unicode" in the patch (two places).
I certainly can't promise that it doesn't break something else, but it seems "correct" to me.

comment:3 by Russell Keith-Magee, 14 years ago

Has patch: set
Needs tests: set

comment:4 by Carl Meyer, 14 years ago

Resolution: fixed
Status: newclosed

Actually a dupe of #10015, fixed in r12150.

comment:5 by Jacob, 12 years ago

milestone: 1.2

Milestone 1.2 deleted

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