Opened 16 years ago

Closed 16 years ago

#7088 closed (fixed)

QuerySet.values_list returns extra fields in Oracle

Reported by: Erin Kelly Owned by: Malcolm Tredinnick
Component: Database layer (models, ORM) Version: dev
Severity: Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

values_list queries are failing in Oracle with results like this one:

Failed example:
    Article.objects.values_list('headline')
Expected:
    [(u'Article 5',), (u'Article 6',), (u'Article 4',), (u'Article 2',), (u'Article 3',), (u'Article 7',), (u'Article 1',)]
Got:
    [[u'Article 5', u'', None], [u'Article 6', u'', None], [u'Article 4', u'', None], [u'Article 2', u'', None], [u'Article 3', u'', None], [u'Article 7', u'', None], [u'Article 1', u'', None]]

It seems that Query.results_iter is getting the list of fields to pass to resolve_columns from self.model._meta.fields. For basic queries, this is fine, but it's wrong for values and values_list queries, and it's incomplete for select_related queries (this last appears to be a bug in trunk as well).

To fix this, I think that the Query object needs to maintain a list of field objects inside or alongside the Query.select and Query.related_select_cols lists.

Change History (6)

comment:1 by Malcolm Tredinnick, 16 years ago

(In [7469]) queryset-refactor: Make sure the right list of fields is passed to the
Query.resolve_columns() method for those backends that provide it (e.g. Oracle).

Refs #7088

comment:2 by Malcolm Tredinnick, 16 years ago

Triage Stage: UnreviewedAccepted

Something to think about: After this change and [7470], I'm becoming tempted to change the map() call in resolve_columns() to a zip() so that any other errors don't just fail silently. As far as I can work out, we should be passing in enough fields for everything but the extra select columns at the front.

comment:3 by Alex Gaynor, 16 years ago

Version: queryset-refactorSVN

in reply to:  2 comment:4 by Erin Kelly, 16 years ago

Replying to mtredinnick:

Something to think about: After this change and [7470], I'm becoming tempted to change the map() call in resolve_columns() to a zip() so that any other errors don't just fail silently. As far as I can work out, we should be passing in enough fields for everything but the extra select columns at the front.

It's not clear to me that if the results were truncated due to missing fields, that would necessarily cause an error. In fact, between [7438] and [7443], the results were getting truncated, and it was just producing bogus results rather than erroring.

I'm ambivalent as to whether the map gets replaced with zip (although note that it will eventually need to be changed in some way, since I hear that map(None) is going away in Python 3), but if we want some error-checking here, I think it should just be in the form of an explicit assertion on the relative lengths of the two sequences.

comment:5 by Malcolm Tredinnick, 16 years ago

Owner: changed from nobody to Malcolm Tredinnick

Ian: I had left this open until you had a chance to confirm the fix actually fixed stuff (ignore the refactoring comment, because that was a throwaway thought for now). Based on the fact that you're filing yet more bugs for me, does this mean we've fixed this issue and can close the ticket?

comment:6 by Erin Kelly, 16 years ago

Resolution: fixed
Status: newclosed

Yes, this particular issue appears to be resolved. I'll go ahead and close the ticket.

Note: See TracTickets for help on using tickets.
Back to Top