Changeset 7455

Timestamp:

04/24/08 11:07:07 (2 months ago)

Author:

mtredinnick

Message:

queryset-refactor: Changed the way order_by() and distinct() interact.

When using "select distinct" all ordering columns must be part of the output
(select) columns. We were previously just throwing away ordering columns that
weren't included, but there are some cases where they are needed and it's
difficult to add them in manually. So now the default behaviour is to append
any missing columns.

This can affect the output of distinct() if complicated order_by() constructs
are used, so the documentation has been updated with an explanation of what's
going on there.

Fixed #7070.

Files:

• django/branches/queryset-refactor/django/db/models/sql/query.py (modified) (9 diffs)
• django/branches/queryset-refactor/docs/db-api.txt (modified) (4 diffs)
• django/branches/queryset-refactor/tests/modeltests/many_to_one/models.py (modified) (1 diff)
• django/branches/queryset-refactor/tests/regressiontests/queries/models.py (modified) (1 diff)

django/branches/queryset-refactor/django/db/models/sql/query.py

@@ -53 +53 @@
-        self.default_ordering = True
-        self.standard_ordering = True
+        self.ordering_aliases = []
-        self.start_meta = None
-
@@ -140 +141 @@
-        obj.default_ordering = self.default_ordering
-        obj.standard_ordering = self.standard_ordering
+        obj.ordering_aliases = []
-        obj.start_meta = self.start_meta
-        obj.select = self.select[:]
@@ -216 +218 @@
-        out_cols = self.get_columns()
-        ordering = self.get_ordering()
+
-        # This must come after 'select' and 'ordering' -- see docstring of
-        # get_from_clause() for details.
-        from_, f_params = self.get_from_clause()
+
-        where, w_params = self.where.as_sql(qn=self.quote_name_unless_alias)
-        params = list(self.extra_select_params)
@@ -225 +229 @@
-        if self.distinct:
-            result.append('DISTINCT')
-
+ + self.ordering_aliases
-
-        result.append('FROM')
@@ -466 +470 @@
-    def get_ordering(self):
-        """
-
+
+
+
-
-        Determining the ordering SQL can change the tables we need to include,
-        so this should be run *before* get_from_clause().
-        """
-        # FIXME: It's an SQL-92 requirement that all ordering columns appear as
-        # output columns in the query (in the select statement) or be ordinals.
-        # We don't enforce that here, but we should (by adding to the select
-        # columns), for portability.
-        if self.extra_order_by:
-            ordering = self.extra_order_by
@@ -486 +488 @@
-        select_aliases = self._select_aliases
-        result = []
+        ordering_aliases = []
-        if self.standard_ordering:
-            asc, desc = ORDER_DIR['ASC']
@@ -516 +519 @@
-                        self.model._meta, default_order=asc):
-                    elt = '%s.%s' % (qn(table), qn2(col))
-
-
+
+
+
-            else:
-                col, order = get_order_dir(field, asc)
-                elt = qn(col)
-
-
+
+
+
+
-        return result
-
@@ -1380 +1386 @@
-            return cursor
-        if result_type == SINGLE:
+            if self.ordering_aliases:
+                return cursor.fetchone()[:-len(results.ordering_aliases)]
-            return cursor.fetchone()
+
-        # The MULTI case.
+        if self.ordering_aliases:
+            return order_modified_iter(cursor, len(self.ordering_aliases),
+                    self.connection.features.empty_fetchmany_value)
-        return iter((lambda: cursor.fetchmany(GET_ITERATOR_CHUNK_SIZE)),
-                self.connection.features.empty_fetchmany_value)
@@ -1409 +1421 @@
-    yield iter([]).next()
-
+def order_modified_iter(cursor, trim, sentinel):
+    """
+    Yields blocks of rows from a cursor. We use this iterator in the special
+    case when extra output columns have been added to support ordering
+    requirements. We must trim those extra columns before anything else can use
+    the results, since they're only needed to make the SQL valid.
+    """
+    for rows in iter((lambda: cursor.fetchmany(GET_ITERATOR_CHUNK_SIZE)),
+            sentinel):
+        yield [r[:-trim] for r in rows]
+
-def setup_join_cache(sender):
-    """

django/branches/queryset-refactor/docs/db-api.txt

@@ -511 +511 @@
-...since the ``Blog`` model has no default ordering specified.
-
+Be cautious when ordering by fields in related models if you are also using
+``distinct()``. See the note in the `distinct()`_ section for an explanation
+of how related model ordering can change the expected results.
+
-It is permissible to specify a multi-valued field to order the results by (for
-example, a ``ManyToMany`` field). Normally this won't be a sensible thing to
@@ -560 +564 @@
-By default, a ``QuerySet`` will not eliminate duplicate rows. In practice, this
-is rarely a problem, because simple queries such as ``Blog.objects.all()``
-
-
-
-
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
-
-``values(*fields)``
@@ -628 +650 @@
-        >>> Entry.objects.values('blog_id')
-        [{'blog_id': 1}, ...]
+    * When using ``values()`` together with ``distinct()``, be aware that
+      ordering can affect the results. See the note in the `distinct()`_
+      section, above, for details.
-
-**New in Django development version:** Previously, it was not possible to pass
@@ -842 +867 @@
-call, since they are conflicting options.
-
-
-
+
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-

django/branches/queryset-refactor/tests/modeltests/many_to_one/models.py

@@ -251 +251 @@
-[<Reporter: John Smith>]
-
-
+ (Note, too, that we clear the ordering here so as not to drag the 'headline' field into the columns being used to determine uniqueness.)
->>> d = {'reporter__first_name': u'John', 'reporter__last_name': u'Smith'}
-
+order_by().
-True
-

django/branches/queryset-refactor/tests/regressiontests/queries/models.py

@@ -499 +499 @@
-[<Item: four>]
-
-
+
+
+
+
+
+
+
->>> Note.objects.values('misc').distinct().order_by('note', '-misc')
-
+, {'misc': u'foo'}
-
-Bug #4358