Opened 15 years ago
Closed 11 years ago
#11722 closed Bug (fixed)
Query lookups that reference an F() expression produce invalid sql
Reported by: | Owned by: | ||
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Normal | Keywords: | F() expression query sql |
Cc: | hongshuning@…, mumino | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
When using an iexact lookup, the sql generated is invalid. For example
MyObject.objects.filter(col1__iexact=F('col2'))
throws a syntax error. Here's an example of the SQL generated:
UPPER("app_myobject"."col1"::text) = UPPER() "app_myobject"."col2"
It's actually not working correctly for any lookup type that involves an expression, but only iexact produces sql that breaks because of the UPPER() function call. An exact lookup produces:
UPPER("app_myobject"."col1"::text) = "app_myobject"."col2"
Notice the extra space between '=' and '"app_myobject"."col2".
I'm using Django trunk, and Postgresql 8.3.
Attachments (3)
Change History (26)
by , 15 years ago
Attachment: | incorrect_fix.diff added |
---|
comment:1 by , 15 years ago
I don't have an Oracle installation handy, but this error also happens in sqlite3, but not in MySQL. Here is the query produced on sqlite3:
SELECT "app_myobject"."id", "app_myobject"."col1", "app_myobject"."col2" FROM "app_myobject" WHERE "app_myobject"."col1" LIKE ESCAPE \'\\\' "app_myobject"."col2"
comment:2 by , 15 years ago
Has patch: | set |
---|---|
Patch needs improvement: | set |
by , 15 years ago
Attachment: | possible_fix.patch added |
---|
comment:3 by , 15 years ago
I've confirmed that this issue still exists in r12085. I've attached another patch that I still assume isn't a proper solution, but it's simpler than my original wild guess (and trac parses it correctly now, as well). I will continue looking at this bug, but as I think my patch shows, I don't fully understand all the internals yet.
comment:4 by , 15 years ago
Needs tests: | set |
---|
I fully encourage you to poke around and try and find a solution to this problem - but if you're going to provide a patch, you need to provide a test case as well. Test cases aren't optional - they're how you prove to me (and the rest of the triage and core team) that your patch actually fixes the problem you suggest.
comment:5 by , 15 years ago
Right you are. Creating the regression test revealed some problems with my patch. I'll re-evaluate how I'm approaching this and get a patch + test up soon. Thanks for the gentle nudge.
comment:6 by , 15 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:7 by , 15 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:9 by , 14 years ago
Owner: | removed |
---|---|
Status: | assigned → new |
Removing myself from this ticket due to lack of time/progress. I still plan on looking at it when time is available!
comment:10 by , 14 years ago
Cc: | added |
---|
I have the save problem when I use startswith lookup.
After appling the patch, the problem isn't gone.
I'm using V1.2.1
comment:11 by , 14 years ago
Severity: | → Normal |
---|---|
Type: | → Bug |
comment:12 by , 14 years ago
Easy pickings: | unset |
---|
#15914 is a duplicate. A patch with a very simple test case exhibiting the problem is attached to that ticket.
comment:13 by , 14 years ago
Thanks, I'm attaching the diff with the test from that ticket here. Test patch credit goes to lrekucki
comment:15 by , 12 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:16 by , 12 years ago
Resolution: | fixed |
---|---|
Status: | closed → new |
Could you describe how you confirmed this problem was fixed? Do you know which commit fixed it?
comment:18 by , 11 years ago
invalid sql generation happens for iendswith, istartswith, icontains, iexact for postgreql. similar problems arise in SQLite. I think other backends have same problem too.
this pull request has a fix. https://github.com/django/django/pull/1945
on the other hand, F objects has another issue related with here.
https://code.djangoproject.com/ticket/16731
because of it istartswith, icontains, iendswith still don't produce correct SQL. but they produce valid SQL now.
comment:19 by , 11 years ago
Cc: | added |
---|
comment:20 by , 11 years ago
Needs tests: | unset |
---|---|
Patch needs improvement: | unset |
comment:21 by , 11 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
Confirmed the regression test works as expected and passes on all backends.
comment:22 by , 11 years ago
This one will be fixed as a side effect of lookup refactor, see: https://github.com/akaariai/django/commit/193cd097ca8f2cc6a911e57b8e3fb726f96ee6a6
comment:23 by , 11 years ago
Owner: | set to |
---|---|
Resolution: | → fixed |
Status: | new → closed |
A patch that illustrates a fix for this case, but is not a permanent fix