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.

[Sarah Boyce <42296566+sarahboyce@…>]

In 40bfd7b0:

Fixed #35011, Refs #28900 -- Added tests for QuerySet.union() with multiple models and DateTimeField annotations.

Ticket was resolved by 65ad4ade74dc9208b9d686a451cd6045df0c9c3a as part of #28900.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 543e17c:

Fixed #36292 -- Fixed crash when aggregating over a group mixing transforms and references.

Regression in 65ad4ade74dc9208b9d686a451cd6045df0c9c3a.

Refs #28900

Thanks Patrick Altman for the report.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 3176904:

[5.2.x] Fixed #36292 -- Fixed crash when aggregating over a group mixing transforms and references.

Regression in 65ad4ade74dc9208b9d686a451cd6045df0c9c3a.

Refs #28900

Thanks Patrick Altman for the report.

Backport of 543e17c4405dfdac4f18759fc78b190406d14239 from main

[Mariusz Felisiak <felisiak.mariusz@…>]

In 12b771a1:

Fixed #36299 -- Prevented field selection on QuerySet.alias() after values().

Regression in 65ad4ade74dc9208b9d686a451cd6045df0c9c3a.

Refs #28900.

Thanks Jeff Iadarola for the report and tests.

Co-Authored-By: OutOfFocus4 <jeff.iadarola@…>

[Mariusz Felisiak <felisiak.mariusz@…>]

In cd1aa54f:

[5.2.x] Fixed #36299 -- Prevented field selection on QuerySet.alias() after values().

Regression in 65ad4ade74dc9208b9d686a451cd6045df0c9c3a.

Refs #28900.

Thanks Jeff Iadarola for the report and tests.

Co-Authored-By: OutOfFocus4 <jeff.iadarola@…>

Backport of 12b771a1ec4bbfe82405176f5601e6441855a303 from main

[Mariusz Felisiak <felisiak.mariusz@…>]

In 71a19a0:

Fixed #36301 -- Fixed select_for_update(of) crash when using values()/values_list().

Regression in 65ad4ade74dc9208b9d686a451cd6045df0c9c3a which allowed for
annotations to be SELECT'ed before model field references through
values()/values_list() and broke assumptions the select_for_update(of)
table infererence logic had about model fields always being first.

Refs #28900.

Thanks OutOfFocus4 for the report and Sarah for the test.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 5d2a0c5:

[5.2.x] Fixed #36301 -- Fixed select_for_update(of) crash when using values()/values_list().

Regression in 65ad4ade74dc9208b9d686a451cd6045df0c9c3a which allowed for
annotations to be SELECT'ed before model field references through
values()/values_list() and broke assumptions the select_for_update(of)
table infererence logic had about model fields always being first.

Refs #28900.

Thanks OutOfFocus4 for the report and Sarah for the test.

Backport of 71a19a0e475165dbc14c1fe02f552013ee670e4c from main

[Sarah Boyce <42296566+sarahboyce@…>]

In 21f8be7:

Fixed #36288 -- Addressed improper handling of duplicates in values_list().

Now that selected aliases are stored in sql.Query.selected: dict[str, Any]
the values_list() method must ensures that duplicate field name references are
assigned unique aliases.

Refs #28900.

Regression in 65ad4ade74dc9208b9d686a451cd6045df0c9c3a.

Thanks Claude for the report.

[Sarah Boyce <42296566+sarahboyce@…>]

In b97af5e:

[5.2.x] Fixed #36288 -- Addressed improper handling of duplicates in values_list().

Now that selected aliases are stored in sql.Query.selected: dict[str, Any]
the values_list() method must ensures that duplicate field name references are
assigned unique aliases.

Refs #28900.

Regression in 65ad4ade74dc9208b9d686a451cd6045df0c9c3a.

Thanks Claude for the report.

Backport of 21f8be76d43aa1ee5ae41c1e0a428cfea1f231c1 from main.

[Sarah Boyce <42296566+sarahboyce@…>]

In 7f6a5fbe:

[5.2.x] Fixed #36360 -- Fixed QuerySet.update() crash when referring annotations through values().

The issue was only manifesting itself when also filtering againt a related
model as that forces the usage of a subquery because SQLUpdateCompiler doesn't
support the UPDATE FROM syntax yet.

Regression in 65ad4ade74dc9208b9d686a451cd6045df0c9c3a.

Refs #28900.

Thanks Gav O'Connor for the detailed report.

Backport of 8ef4e0bd423ac3764004c73c3d1098e7a51a2945 from main.

[Sarah Boyce]

In ​8ef4e0b:

Fixed #36360 -- Fixed QuerySet.update() crash when referring annotations through values().

The issue was only manifesting itself when also filtering againt a related
model as that forces the usage of a subquery because SQLUpdateCompiler doesn't
support the UPDATE FROM syntax yet.

Regression in 65ad4ade74dc9208b9d686a451cd6045df0c9c3a.

Refs #28900.

Thanks Gav O'Connor for the detailed report.