On Wed, Nov 4, 2009 at 3:05 AM, Bill Freeman <[email protected]> wrote: > > On Tue, Nov 3, 2009 at 9:14 AM, Bill Freeman <[email protected]> wrote: >> On Tue, Nov 3, 2009 at 7:05 AM, Russell Keith-Magee >> <[email protected]> wrote: >>> >>> On Tue, Nov 3, 2009 at 12:19 PM, Christophe Pettus <[email protected]> >>> wrote: >>>> >>>> >>>> On Nov 2, 2009, at 2:14 PM, Bill Freeman wrote: >>>>> My presumption is that the older PostgreSQL, expecting to have to >>>>> decide whether unquoted things are strings (e.g.; "status" in the >>>>> query samples above), used to look at the context and decided that we >>>>> had meant the 8 as a string. >>>> >>>> In particular, as of PostgreSQL 8.3 (and thus 8.4), non-text types are >>>> not automatically cast to text. You can find the details in the 8.3 >>>> release notes: >>>> >>>> http://www.postgresql.org/docs/8.3/static/release-8-3.html >>>> >>>> The relevant section is E.9.2.1. >>>> >>>>> Thoughts? Workaround suggestions? >>>> >>>> I'm not completely sure why the ORM is generating SQL that compares a >>>> number with a character string in the first place; that sounds like a >>>> bug in either the ORM or the client code, to me. >>> >>> I concur. This looks like it might be a Django bug. >>> >>> If I understand the original problem correctly, it is this: >>> >>> class MyObj(models.Model): >>> CHOICES = ( >>> ('1', 'first choice') >>> ('2', 'second choice') >>> ) >>> choice = models.CharField(max_length=1, choices=CHOICES) >>> >>> 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',)) >> >> Might that have to be: >> >> 'SELECT ... `choice` = `%s`', ('1',)) >> >> because %s renders both 1 and '1' as the single character 1. >> >> But then, if I understand correctly, by the time this gets to the DB >> it's actually something like: >> SELECT ... = ? ' >> >> and I don't know how the quoting works there. Still that's the >> province of the adaptor, and all django can do is decide what is >> passed to psycopg2. I guess I'm going to learn more about the DB >> adaptor. >>> >>> 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. "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. >>> >>> I've just opened ticket #12137 to track this. I've put it on the 1.2 >>> milestone, so we will endeavour to fix it before we hit v1.2. Any >>> assistance in turning the example and suggested fix into a trunk-ready >>> patch will be gratefully accepted. >> >> Since this is impacting me right now, I'll spend some quality time >> with pdb to try and come up with a patch. >> >> News at it happens. >>> >>> Yours, >>> Russ Magee %-) >> >> Bill >> > > Well, adding the obvious get_db_prep_value() (thanks Russ) to both > CharField and TextField in django/db/models/fields/__init__.py makes > my app work on ps8.4 without any ill effects on pg8.1.11. I've tested > most of the page types that I have in both circumstances without > noticing any anomalies. > > Of course that doesn't prove that there's nobody who depends on the > old behavior. > > I basically copied the version from BooleanField, and changed the > "return bool(value)" to "return unicode(value)" (though everything > else inside the queryset's query object seems to be string rather than > unicode, so I'm not positive that "return str(value)" isn't more self > consistent). > > I'll try to provide something more formal as a response to the ticket.
Hi Bill, Thanks for taking the time to check this and make a patch. There are two things standing between this and the trunk. * Generate your diff against the root of the tree, not as a diff between two versions of a single file. The output of `svn diff`` is ideal. * Tests! Tests! Tests! A patch is great, but it needs to be tested. The regressiontests.queries tests looks to be as good a place as any. The queries test already contains a bunch of models that look like they could be used to test this (you just need to do an integer lookup on a CharField). I'm going to have to do both of these things before the fix gets into trunk, but if you can spare the time to do the leg work, the fix will end up in trunk much sooner. Yours, Russ Magee %-) --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Django users" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/django-users?hl=en -~----------~----~----~----~------~----~------~--~---
