Opened 17 years ago
Closed 17 years ago
#7087 closed (fixed)
QuerySet.dates bug in Oracle
Reported by: | Erin Kelly | Owned by: | Malcolm Tredinnick |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | queryset-refactor |
Severity: | Keywords: | ||
Cc: | Triage Stage: | Unreviewed | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
The DateQuery.results_iter method doesn't account for the possibility of extra selects and just returns the first column from the results. When the query is subscripted in Oracle, that ends up being the row number.
Attachments (3)
Change History (16)
by , 17 years ago
Attachment: | qsrf_dates_oracle.diff added |
---|
follow-up: 4 comment:1 by , 17 years ago
by , 17 years ago
Attachment: | orcl_revert_resolve_columns.diff added |
---|
follow-up: 5 comment:2 by , 17 years ago
Cc: | added |
---|
Using my patch I was able to use the dates
method to pull dates from a test model w/a DateTimeField
just fine -- but I did not run the full test suite. My GIS tests now all pass as it fixes a bug due to my use of extra SQL to retrieve GML representations of geometries.
comment:3 by , 17 years ago
Cc: | removed |
---|---|
Owner: | changed from | to
follow-up: 7 comment:4 by , 17 years ago
Replying to jbronn:
Actually extra select SQL is broken everywhere -- I think your revision to my previous patch (by overridding
set_limits
andclear_limits
) made the hacks toresolve_columns
unnecessary.
Hrm, I don't think that it did. resolve_columns will still receive the extra selects at the beginning of the row and will still need to account for that, since the extra selects don't have any corresponding entries at the beginning of the fields list.
Can you clarify what extra selects are broken? After applying my patch here and the one in #7057, the only test cases that are still failing for me are the values_list queries in the lookups test.
follow-up: 6 comment:5 by , 17 years ago
Replying to jbronn:
Using my patch I was able to use the
dates
method to pull dates from a test model w/aDateTimeField
just fine -- but I did not run the full test suite. My GIS tests now all pass as it fixes a bug due to my use of extra SQL to retrieve GML representations of geometries.
Try the queries
test, if you would. The specific failure that motivated this ticket was this one:
File "/home/ikelly/projects/django.qs-rf/tests/regressiontests/queries/models.py", line ?, in regressiontests.queries.models.__test__.API_TESTS Failed example: Item.objects.dates('created', 'day')[0] Expected: datetime.datetime(2007, 12, 19, 0, 0) Got: 1
comment:6 by , 17 years ago
Replying to ikelly:
Try the
queries
test, if you would. The specific failure that motivated this ticket was this one:
My bad, I ran my dates test accidentally using trunk -- I get the same erroneous response w/qs-rf.
follow-up: 8 comment:7 by , 17 years ago
Can you clarify what extra selects are broken? After applying my patch here and the one in #7057, the only test cases that are still failing for me are the values_list queries in the lookups test.
Yes GeoQuerySet
has a gml
method that attaches a GML representation (XML) of the geometry, which is returned as a CLOB. Essentially, here's what I do:
qs.extra(select={'gml' : 'SDO_UTIL.TO_GMLGEOMETRY("AU_AUSTRALIACITY"."POINT")'})
Without my patch I get the following:
In [2]: qs = AustraliaCity.objects.all().gml() In [3]: qs[0].gml Out[3]: <cx_Oracle.LOB object at 0x011FFAE8> In [4]: qs[0].gml.read() Out[4]: '<gml:Point srsName="SDO:4326" xmlns:gml="http://www.opengis.net/gml"><gml:coordinates decimal="." cs="," ts=" ">150.90199999999998681232 6824292540550232,-34.42450000000000187583282240666449069977 </gml:coordinates></gml:Point>'
This is because in resolve_columns
, the values
list is initialized like so:
values = list(row[:index_start])
as such, any extra selection fields don't benefit from the extra processing -- I think this would include any custom selects on DateField
and TextField
.
New ticket, perhaps?
comment:8 by , 17 years ago
Replying to jbronn:
as such, any extra selection fields don't benefit from the extra processing -- I think this would include any custom selects on
DateField
andTextField
.
New ticket, perhaps?
Ah, yes. We can't do all of the resolve_columns processing without a field object, but we can and should convert dates and clobs -- especially clobs, since the handle won't necessarily be available to read from later on. The change would also be consistent with what we already do in trunk.
comment:9 by , 17 years ago
I'm in the process of writing a (hopefully) comprehensive patch for #7088 and I'll include some extra processing of the extra_select for dates and clobs in that. Will close this one in a moment, since the date bug Ian found is actually valid for all backends and I've just written a test to make sure it stays fixed.
comment:10 by , 17 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:11 by , 17 years ago
by , 17 years ago
Attachment: | orcl_convert_values.diff added |
---|
Missing import fix for convert_values
.
comment:12 by , 17 years ago
Resolution: | fixed |
---|---|
Status: | closed → reopened |
Imports are needed for convert_values
to work, see attached patch.
comment:13 by , 17 years ago
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
Fixed in [7475]. Thanks, Justin.
Actually extra select SQL is broken everywhere -- I think your revision to my previous patch (by overridding
set_limits
andclear_limits
) made the hacks toresolve_columns
unnecessary.