QuerySet.values() and values_list() for compound queries fails with annotation.

[elliott-omosheye]

Added failing test to demonstrate

    def test_union_values_subquery(self):
        items = Item.objects.filter(creator=OuterRef("pk"))
        item_authors = Author.objects.annotate(is_creator=Exists(items)).order_by()
        reports = Report.objects.filter(creator=OuterRef("pk"))
        report_authors = Author.objects.annotate(is_creator=Exists(reports)).order_by()
        all_authors = item_authors.union(report_authors).order_by()
        self.assertEqual(list(all_authors.values_list("is_creator", flat=True)), [True, True, True, True])

silently messes up the data on values/values_list() with an field

FAIL: test_union_values_subquery (queries.tests.Queries1Tests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/local/lib/python2.7/unittest/case.py", line 329, in run
    testMethod()
  File "/usr/src/widget_app/tests/queries/tests.py", line 95, in test_union_values_subquery
    self.assertEqual(list(all_authors.values_list("is_creator", flat=True)), [True, True, True, True])
  File "/usr/local/lib/python2.7/unittest/case.py", line 513, in assertEqual
    assertion_func(first, second, msg=msg)
  File "/usr/local/lib/python2.7/unittest/case.py", line 742, in assertListEqual
    self.assertSequenceEqual(list1, list2, msg, seq_type=list)
  File "/usr/local/lib/python2.7/unittest/case.py", line 724, in assertSequenceEqual
    self.fail(msg)
  File "/usr/local/lib/python2.7/unittest/case.py", line 410, in fail
    raise self.failureException(msg)
AssertionError: Lists differ: [True, True, 2, 2, 3, 4, 4] != [True, True, True, True]

First differing element 2:
2
True

First list contains 3 additional elements.
First extra element 4:
3

- [True, True, 2, 2, 3, 4, 4]
+ [True, True, True, True]

throws error on empty values()/values_list()

Traceback (most recent call last):
  File "/usr/local/lib/python2.7/unittest/case.py", line 329, in run
    testMethod()
  File "/usr/src/widget_app/tests/queries/tests.py", line 95, in test_union_values_subquery
    self.assertEqual(list(all_authors.values()), [True, True, True, True])
  File "/usr/local/lib/python2.7/site-packages/django/db/models/query.py", line 250, in __iter__
    self._fetch_all()
  File "/usr/local/lib/python2.7/site-packages/django/db/models/query.py", line 1118, in _fetch_all
    self._result_cache = list(self._iterable_class(self))
  File "/usr/local/lib/python2.7/site-packages/django/db/models/query.py", line 106, in __iter__
    for row in compiler.results_iter(chunked_fetch=self.chunked_fetch):
  File "/usr/local/lib/python2.7/site-packages/django/db/models/sql/compiler.py", line 847, in results_iter
    row = self.apply_converters(row, converters)
  File "/usr/local/lib/python2.7/site-packages/django/db/models/sql/compiler.py", line 830, in apply_converters
    value = row[pos]
IndexError: list index out of range

The funny thing is that the produced SQL query is perfectly valid, so it isn't a case of me asking the ORM to do something especially funky. In any case if this is an unsupported action then the ORM should tell you and not produce runtime errors or silently return bad data.

[Simon Charette]

Would it be possible to provide your simplified model definition and data setup procedure as well as it makes it really hard to reproduce without them.

[elliott-omosheye]

Replying to Simon Charette:

Would it be possible to provide your simplified model definition and data setup procedure as well as it makes it really hard to reproduce without them.

Sorry for not being clearer, I am using the model definitions from queries.tests.Queries1Tests ​https://github.com/django/django/blob/stable/1.11.x/tests/queries/tests.py

[Sergey Fedoseev]

Probably duplicate of #28553.

[Mariusz Felisiak]

We ​decided to treat it as a separate issue.

[David Wobrock]

I didn't find the time to push on this. Removing myself from the ticket.

[David Sanders]

If it helps, while checking if #34945 was a duplicate of this I was able to boil down the failure to this simple example:

class Foo(Model):
    name = CharField()

class Bar(Model):
    name = CharField()

qs = (
    Foo.objects.annotate(alias=F("name"))
    .union(Bar.objects.annotate(alias=F("name")))
    .values("alias")
)
print(qs)

gives

<QuerySet [{'alias': 1}, {'alias': 1}]>

[ontowhee]

Replying to David Sanders:

Thanks for the simple example!

I'm not too knowledgeable about the ORM, but I'm trying to work through this problem. Is the problem within annotate, or somewhere else, or a combination of annotate and something else?

Using the simple example, I tried taking the union of Foo and Bar without any annotation, and I apply .values('name') to the result

Foo.objects.create(name='hello')
Bar.objects.create(name='goodbye')

qs = Foo.objects.all().union(Bar.objects.all()).values('name')
print(qs)

and it gives

<QuerySet [{'name': 'goodbye'}, {'name': 'hello'}]>

Which is correct and expected. However, when I remove .values('name')

qs = Foo.objects.all().union(Bar.objects.all())
print(qs)

it then gives

<QuerySet [<Foo: Foo object (1)>, <Foo: Foo object (1)>]>

Is this expected, or is this something to look into?

[Sarah Boyce <42296566+sarahboyce@…>]

In 65ad4ade:

Refs #28900 -- Made SELECT respect the order specified by values(*selected).

Previously the order was always extra_fields + model_fields + annotations with
respective local ordering inferred from the insertion order of *selected.

This commits introduces a new Query.selected propery that keeps tracks of the
global select order as specified by on values assignment. This is crucial
feature to allow the combination of queries mixing annotations and table
references.

It also allows the removal of the re-ordering shenanigans perform by
ValuesListIterable in order to re-map the tuples returned from the database
backend to the order specified by values_list() as they'll be in the right
order at query compilation time.

Refs #28553 as the initially reported issue that was only partially fixed
for annotations by d6b6e5d0fd4e6b6d0183b4cf6e4bd4f9afc7bf67.

Thanks Mariusz Felisiak and Sarah Boyce for review.

[Sarah Boyce <42296566+sarahboyce@…>]

In 6d22096:

Fixed #28900 -- Propagated all selected fields to combinator queries.

Previously, only the selected column aliases would be propagated and
annotations were ignored.

[Sarah Boyce <42296566+sarahboyce@…>]

In 0e65abd2:

Refs #28900 -- Made Query.has_select_fields a computed property.

This should ensure it never drifts from Query.selected while maintaining
backward compatibility.