Opened 4 years ago

Closed 3 years ago

Last modified 3 years ago

#25316 closed Bug (fixed)

Can't use order_by and values(_list) together after annotating

Reported by: Adrian Wielgosik Owned by: varun naganathan
Component: Database layer (models, ORM) Version: 1.8
Severity: Normal Keywords: annotate, order_by
Cc: josh.smeaton@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Example:

# model

from django.db import models
class Table(models.Model):
    value = models.IntegerField()

# tests

from django.test import TestCase
from testapp.models import Table
from django.db.models import IntegerField, Case, Value, When
class Test1(TestCase):
    def test_1(self):
        query = Table.objects.annotate(
            other=Case(
                When(value=11, then=Value(1)),
                output_field=IntegerField()
            ),
        ).order_by('other', 'value').values('id')

        print query

The error is:

AttributeError: 'WhereNode' object has no attribute 'resolve_expression'

Attachments (1)

25312-test.diff (739 bytes) - added by Tim Graham 4 years ago.

Download all attachments as: .zip

Change History (13)

comment:1 Changed 4 years ago by Tim Graham

Triage Stage: UnreviewedAccepted

Attached is a test to reproduce the crash (at 956df84a613d4b9a92c979e46557243d288282c8).

Changed 4 years ago by Tim Graham

Attachment: 25312-test.diff added

comment:2 Changed 4 years ago by Josh Smeaton

Cc: josh.smeaton@… added

This looks related to #25307

comment:3 Changed 4 years ago by Lucas Moeskops

I think this is not an issue.

The problem is that the example is annotating a field and then sorts on this field. To do this, the field needs to exist still in the select. It is however thrown away by values, so it can't sort on it.

The example will work correctly if the annotated field is added to the values list as well. It will also work correctly if no order on the annotated field is applied.

If this were to work, the resulting query would be considerably more complex, involving a different final select than the select to apply the ordering on.

What could be improved though is the error message. This is clear when no annotation is used. Maybe something like "Can't deselect annotated value" or "Can't order on unselected annotated field other"?

comment:4 Changed 4 years ago by Anssi Kääriäinen

We do add "unselected" fields to the end of select list for some cases already. It is perhaps possible to do so here, too.

comment:5 Changed 3 years ago by varun naganathan

I don't think the problem is because the field is not available in the select.
When I use an inbuilt function like "Count" to annotate instead of the "When" and "then" clause,the error doesnt seem to appear.
If the field used to sort not being available in select was the problem,even use of "annotate" with "Count" would give the same error.
Edit:
On top of that even if "When" and "Then" is used with the field not being used in the annotation but not included in the "fields",still no error occurs.
I'm thus persuaded to feel that the "When" and "Then" statements are causing the error.
Edit 2:
Finally I'm facing errors only if the annotated field("other" in this case) is used along with the order_by parameters.All other cases seem to work.I will try working and fixing this as soon as possible.

Last edited 3 years ago by varun naganathan (previous) (diff)

comment:6 Changed 3 years ago by varun naganathan

Owner: changed from nobody to varun naganathan
Status: newassigned

comment:7 Changed 3 years ago by varun naganathan

Please review PR
https://github.com/django/django/pull/5852
I'm new to contributing to django.Sorry if I wasn't supposed to self assign the bug to me.

Last edited 3 years ago by varun naganathan (previous) (diff)

comment:8 Changed 3 years ago by Tim Graham

Has patch: set

comment:9 Changed 3 years ago by Tim Graham

Triage Stage: AcceptedReady for checkin

comment:10 Changed 3 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

In 3eba9638:

Fixed #25316 -- Fixed a crash with order_by() and values() after annotate().

comment:11 Changed 3 years ago by Tim Graham <timograham@…>

In f6b4893:

[1.8.x] Fixed #25316 -- Fixed a crash with order_by() and values() after annotate().

Backport of 3eba9638ee69138c73efb1d1c1d1b806ddafc6cf from master

comment:12 Changed 3 years ago by Tim Graham <timograham@…>

In 5770c23:

[1.9.x] Fixed #25316 -- Fixed a crash with order_by() and values() after annotate().

Backport of 3eba9638ee69138c73efb1d1c1d1b806ddafc6cf from master

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