On Tue, Nov 3, 2009 at 6:46 PM, Russell Keith-Magee <[email protected]> wrote: > > 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 %-) >
Russ, I figured that I would diff what I had an opportunity to test on the configurations I have available. I'll try to figure out more later, but for now this has made other stuff slip, so it will be a while. So I'll check the ticket from time to time, and if I see that you've done it, I'll breath a sigh of relief. Otherwise, if I get ahead enough to give it some time, I will. Bill --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
