Django

Code

Ticket #7087 (closed: fixed)

Opened 7 months ago

Last modified 7 months ago

QuerySet.dates bug in Oracle

Reported by: ikelly Assigned to: mtredinnick
Milestone: Component: Database layer (models, ORM)
Version: queryset-refactor Keywords:
Cc: Triage Stage: Unreviewed
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 0

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

qsrf_dates_oracle.diff (0.7 kB) - added by ikelly on 04/25/08 15:01:36.
orcl_revert_resolve_columns.diff (0.9 kB) - added by jbronn on 04/25/08 19:55:45.
orcl_convert_values.diff (1.1 kB) - added by jbronn on 04/26/08 17:33:34.
Missing import fix for convert_values.

Change History

04/25/08 15:01:36 changed by ikelly

  • attachment qsrf_dates_oracle.diff added.

(follow-up: ↓ 4 ) 04/25/08 19:55:30 changed by jbronn

  • needs_better_patch changed.
  • needs_tests changed.
  • needs_docs changed.

Actually extra select SQL is broken everywhere -- I think your revision to my previous patch (by overridding set_limits and clear_limits) made the hacks to resolve_columns unnecessary.

04/25/08 19:55:45 changed by jbronn

  • attachment orcl_revert_resolve_columns.diff added.

(follow-up: ↓ 5 ) 04/25/08 20:03:52 changed by jbronn

  • cc set to mtredinnick.

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.

04/25/08 20:04:09 changed by jbronn

  • cc deleted.
  • owner changed from nobody to mtredinnick.

(in reply to: ↑ 1 ; follow-up: ↓ 7 ) 04/25/08 20:11:04 changed by ikelly

Replying to jbronn:

Actually extra select SQL is broken everywhere -- I think your revision to my previous patch (by overridding set_limits and clear_limits) made the hacks to resolve_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.

(in reply to: ↑ 2 ; follow-up: ↓ 6 ) 04/25/08 20:18:35 changed by ikelly

Replying to jbronn:

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.

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

(in reply to: ↑ 5 ) 04/25/08 23:45:31 changed by jbronn

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.

(in reply to: ↑ 4 ; follow-up: ↓ 8 ) 04/26/08 01:08:14 changed by jbronn

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?

(in reply to: ↑ 7 ) 04/26/08 01:51:20 changed by ikelly

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 and TextField. 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.

04/26/08 04:04:58 changed by mtredinnick

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.

04/26/08 04:06:57 changed by mtredinnick

  • status changed from new to closed.
  • resolution set to fixed.

(In [7468]) queryset-refactor: Repaired the dates() method with extra(select=...).xi It was broken by [7340]. Patch from Ian Kelly. Fixed #7087.

04/26/08 06:21:02 changed by mtredinnick

(In [7470]) queryset-refactor: Pass any extra(select=...) columns through the value conversion function in the Oracle backend after reading the row from the database.

Refs #7087 (see comment 7 on that ticket).

04/26/08 17:33:34 changed by jbronn

  • attachment orcl_convert_values.diff added.

Missing import fix for convert_values.

04/26/08 17:34:51 changed by jbronn

  • status changed from closed to reopened.
  • resolution deleted.

Imports are needed for convert_values to work, see attached patch.

04/26/08 19:45:17 changed by anonymous

  • status changed from reopened to closed.
  • resolution set to fixed.

Fixed in [7475]. Thanks, Justin.

Add/Change #7087 (QuerySet.dates bug in Oracle)




Change Properties
Action