Opened 16 years ago
Closed 12 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 , 16 years ago
| Attachment: | incorrect_fix.diff added |
|---|
comment:1 by , 16 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 , 16 years ago
| Has patch: | set |
|---|---|
| Patch needs improvement: | set |
by , 16 years ago
| Attachment: | possible_fix.patch added |
|---|
comment:3 by , 16 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 , 16 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 , 16 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 , 16 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
comment:7 by , 16 years ago
| Triage Stage: | Unreviewed → Accepted |
|---|
comment:9 by , 15 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 , 15 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 , 15 years ago
| Severity: | → Normal |
|---|---|
| Type: | → Bug |
comment:12 by , 15 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 , 15 years ago
Thanks, I'm attaching the diff with the test from that ticket here. Test patch credit goes to lrekucki
comment:15 by , 13 years ago
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
comment:16 by , 13 years ago
| Resolution: | fixed |
|---|---|
| Status: | closed → new |
Could you described how you confirmed this problem was fixed? Do you know which commit fixed it?
comment:18 by , 12 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 , 12 years ago
| Cc: | added |
|---|
comment:20 by , 12 years ago
| Needs tests: | unset |
|---|---|
| Patch needs improvement: | unset |
comment:21 by , 12 years ago
| Triage Stage: | Accepted → Ready for checkin |
|---|
Confirmed the regression test works as expected and passes on all backends.
comment:22 by , 12 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 , 12 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