Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#24615 closed Bug (fixed)

ordering by expression not part of SELECT is broken

Reported by: Mattia Procopio Owned by: nobody
Component: GIS Version: 1.8
Severity: Release blocker Keywords:
Cc: miroslav@… Triage Stage: Ready for checkin
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I was ordering my queryset by distance after calling .distance() on it, it was working well with django 1.7 but after upgrading to django 1.8 I am now getting this error:

File "/home/matt/repos/uwncom/lib/python2.7/site-packages/django/db/models/query.py", line 162, in __iter__
    self._fetch_all()
  File "/home/matt/repos/uwncom/lib/python2.7/site-packages/django/db/models/query.py", line 965, in _fetch_all
    self._result_cache = list(self.iterator())
  File "/home/matt/repos/uwncom/lib/python2.7/site-packages/django/db/models/query.py", line 1217, in iterator
    for row in compiler.results_iter():
  File "/home/matt/repos/uwncom/lib/python2.7/site-packages/django/db/models/sql/compiler.py", line 783, in results_iter
    results = self.execute_sql(MULTI)
  File "/home/matt/repos/uwncom/lib/python2.7/site-packages/django/db/models/sql/compiler.py", line 818, in execute_sql
    sql, params = self.as_sql()
  File "/home/matt/repos/uwncom/lib/python2.7/site-packages/django/db/models/sql/compiler.py", line 367, in as_sql
    extra_select, order_by, group_by = self.pre_sql_setup()
  File "/home/matt/repos/uwncom/lib/python2.7/site-packages/django/db/models/sql/compiler.py", line 49, in pre_sql_setup
    order_by = self.get_order_by()
  File "/home/matt/repos/uwncom/lib/python2.7/site-packages/django/db/models/sql/compiler.py", line 276, in get_order_by
    field, self.query.get_meta(), default_order=asc))
  File "/home/matt/repos/uwncom/lib/python2.7/site-packages/django/db/models/sql/compiler.py", line 552, in find_ordering_name
    if field.rel and path and opts.ordering and name != field.attname:
AttributeError: 'DistanceField' object has no attribute 'rel'

Is there anything I missed upgrading to 1.8?

Attachments (1)

ticket_24615.diff (1.7 KB ) - added by Miroslav Shubernetskiy 9 years ago.
reproduction test case

Download all attachments as: .zip

Change History (14)

comment:1 by Tim Graham, 9 years ago

It seems likely to be a bug. Could you write a regression test for Django's test suite or at least tell us how we can reproduce the error?

comment:2 by Mattia Procopio, 9 years ago

After digging a little bit more into this I discovered this is not strictly related to ordering, in fact normal ordering still works:

In [1]: from django.contrib.gis.geos import Point

In [2]: qs = GeoData.objects.all()

In [3]: nqs = qs.distance(Point(3, 3)).order_by('distance')

In [4]: nqs[0].distance
Out[5]: Distance(m=157177.768212)

but when iterating the queryset ordered by distance I get the error:

In [1]: from django.contrib.gis.geos import Point

In [2]: qs = GeoData.objects.all()

In [3]: nqs = qs.distance(Point(3, 3)).order_by('distance')

In [4]: vlqs = nqs.values_list('wisp', flat=True)

In [5]: id_list = [id for id in vlqs]
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-5-7fa41993bc21> in <module>()
----> 1 id_list = [id for id in vlqs]

/home/matt/repos/uwncom/lib/python2.7/site-packages/django/db/models/query.pyc in __iter__(self)
    160                - Responsible for turning the rows into model objects.
    161         """
--> 162         self._fetch_all()
    163         return iter(self._result_cache)
    164 

/home/matt/repos/uwncom/lib/python2.7/site-packages/django/db/models/query.pyc in _fetch_all(self)
    963     def _fetch_all(self):
    964         if self._result_cache is None:
--> 965             self._result_cache = list(self.iterator())
    966         if self._prefetch_related_lookups and not self._prefetch_done:
    967             self._prefetch_related_objects()

/home/matt/repos/uwncom/lib/python2.7/site-packages/django/db/models/query.pyc in iterator(self)
   1215         compiler = self.query.get_compiler(self.db)
   1216         if self.flat and len(self._fields) == 1:
-> 1217             for row in compiler.results_iter():
   1218                 yield row[0]
   1219         elif not self.query.extra_select and not self.query.annotation_select:

/home/matt/repos/uwncom/lib/python2.7/site-packages/django/db/models/sql/compiler.pyc in results_iter(self, results)
    781         converters = None
    782         if results is None:
--> 783             results = self.execute_sql(MULTI)
    784         fields = [s[0] for s in self.select[0:self.col_count]]
    785         converters = self.get_converters(fields)

/home/matt/repos/uwncom/lib/python2.7/site-packages/django/db/models/sql/compiler.pyc in execute_sql(self, result_type)
    816             result_type = NO_RESULTS
    817         try:
--> 818             sql, params = self.as_sql()
    819             if not sql:
    820                 raise EmptyResultSet

/home/matt/repos/uwncom/lib/python2.7/site-packages/django/db/models/sql/compiler.pyc in as_sql(self, with_limits, with_col_aliases, subquery)
    365         refcounts_before = self.query.alias_refcount.copy()
    366         try:
--> 367             extra_select, order_by, group_by = self.pre_sql_setup()
    368             if with_limits and self.query.low_mark == self.query.high_mark:
    369                 return '', ()

/home/matt/repos/uwncom/lib/python2.7/site-packages/django/db/models/sql/compiler.pyc in pre_sql_setup(self)
     47         """
     48         self.setup_query()
---> 49         order_by = self.get_order_by()
     50         extra_select = self.get_extra_select(order_by, self.select)
     51         group_by = self.get_group_by(self.select + extra_select, order_by)

/home/matt/repos/uwncom/lib/python2.7/site-packages/django/db/models/sql/compiler.pyc in get_order_by(self)
    274                 # '-field1__field2__field', etc.
    275                 order_by.extend(self.find_ordering_name(
--> 276                     field, self.query.get_meta(), default_order=asc))
    277             else:
    278                 if col not in self.query.extra_select:

/home/matt/repos/uwncom/lib/python2.7/site-packages/django/db/models/sql/compiler.pyc in find_ordering_name(self, name, opts, alias, default_order, already_seen)
    550         # append the default ordering for that model unless the attribute name
    551         # of the field is specified.
--> 552         if field.rel and path and opts.ordering and name != field.attname:
    553             # Firstly, avoid infinite loops.
    554             if not already_seen:

AttributeError: 'DistanceField' object has no attribute 'rel'

the model I use is very simple:

import uuid

from django.contrib.gis.db import models


class GeoData(models.Model):
    id = UUIDField(primary_key=True, editable=False, default=uuid.uuid4)
    coverage = models.MultiPolygonField(null=True, blank=True)

    objects = models.GeoManager()

    def __unicode__(self):
        return "%s" % self.coverage  # pragma: no cover

Creating a multipolygon:

g = GeoData(coverage=MultiPolygon(Polygon( ((1, 1), (1, 2), (2, 2), (1, 1)) )) )
Last edited 9 years ago by Mattia Procopio (previous) (diff)

comment:3 by Miroslav Shubernetskiy, 9 years ago

Cc: miroslav@… added
Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

Was able to reproduce the same traceback in Djagno1.8. Django1.7 this passes. Also Django1.9 (master branch) traceback is different. Attached is the patch with the test which reproduces the issue.

Version 0, edited 9 years ago by Miroslav Shubernetskiy (next)

by Miroslav Shubernetskiy, 9 years ago

Attachment: ticket_24615.diff added

reproduction test case

comment:4 by Claude Paroz, 9 years ago

Bisected the issue to 0c7633178fa9410f102e4708cef979b873bccb76
Anssi, does the traceback give you a hint about a possible cause/resolution?

comment:5 by Claude Paroz, 9 years ago

Also note if we simply add a rel = None attribute to geometry BaseField, the same find_ordering_name method fails a bit later with:

  File "/home/claude/virtualenvs/djangogit/local/lib/python2.7/site-packages/django/db/models/sql/compiler.py", line 261, in get_order_by
    default_order=asc))
  File "/home/claude/virtualenvs/djangogit/local/lib/python2.7/site-packages/django/db/models/sql/compiler.py", line 540, in find_ordering_name
    return [(t.get_col(alias), order, False) for t in targets]
AttributeError: 'RawSQL' object has no attribute 'get_col'

comment:6 by Anssi Kääriäinen, 9 years ago

I'm a bit busy right now. I'll take a look by Thursday.

comment:7 by Anssi Kääriäinen, 9 years ago

Triage Stage: AcceptedReady for checkin

The problem turns out to be that order_by references to expressions which are masked out of the SELECT clause (by .values() for example) weren't resolved correctly.

comment:8 by Anssi Kääriäinen, 9 years ago

Patch needs improvement: set
Triage Stage: Ready for checkinAccepted

comment:9 by Mattia Procopio, 9 years ago

Summary: ordering queryset by distance is brokenordering by expression not part of SELECT is broken

comment:10 by Anssi Kääriäinen, 9 years ago

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

comment:11 by Claude Paroz <claude@…>, 9 years ago

Resolution: fixed
Status: newclosed

In fb5c7748:

Fixed #24615 -- ordering by expression not part of SELECT

Fixed queries where an expression was used in order_by() but the
expression wasn't in the query's select clause (for example the
expression could be masked by .values() call)

Thanks to Trac alias MattBlack85 for the report.

comment:12 by Claude Paroz <claude@…>, 9 years ago

In 70ff455:

[1.8.x] Fixed #24615 -- ordering by expression not part of SELECT

Fixed queries where an expression was used in order_by() but the
expression wasn't in the query's select clause (for example the
expression could be masked by .values() call)

Thanks to Trac alias MattBlack85 for the report.
Backport of fb5c7748da from master.

comment:13 by Mattia Procopio, 9 years ago

Awesome, thanks guys!

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