QuerySet.values_list returns extra fields in Oracle

[Erin Kelly]

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.

[Malcolm Tredinnick]

(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

[Malcolm Tredinnick]

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.

[Erin Kelly]

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.

[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?

[Erin Kelly]

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