foo.diff

commit ac7828487fbb574e098bdf0cb9f727b05f35d240
Author: Loic Bistuer <loic.bistuer@gmail.com>
Date:   Wed Jan 21 13:14:40 2015 +0700

    Cleaned up QuerySet cloning.

diff --git a/django/db/models/query.py b/django/db/models/query.py
index 6774fee..44e8fff 100644
--- a/django/db/models/query.py
+++ b/django/db/models/query.py
@@ -61,7 +61,7 @@ class QuerySet(object):
         self._for_write = False
         self._prefetch_related_lookups = []
         self._prefetch_done = False
-        self._known_related_objects = {}        # {rel_field, {pk: rel_obj}}
+        self._known_related_objects = {}  # {rel_field, {pk: rel_obj}}
 
     def as_manager(cls):
         # Address the circular dependency between `Queryset` and `Manager`.
@@ -185,7 +185,7 @@ class QuerySet(object):
             return self._result_cache[k]
 
         if isinstance(k, slice):
-            qs = self._clone()
+            clone = self._clone()
             if k.start is not None:
                 start = int(k.start)
             else:
@@ -194,12 +194,12 @@ class QuerySet(object):
                 stop = int(k.stop)
             else:
                 stop = None
-            qs.query.set_limits(start, stop)
-            return list(qs)[::k.step] if k.step else qs
+            clone.query.set_limits(start, stop)
+            return list(clone)[::k.step] if k.step else clone
 
-        qs = self._clone()
-        qs.query.set_limits(k, k + 1)
-        return list(qs)[0]
+        clone = self._clone()
+        clone.query.set_limits(k, k + 1)
+        return list(clone)[0]
 
     def __and__(self, other):
         self._merge_sanity_check(other)
@@ -463,11 +463,11 @@ class QuerySet(object):
             "field_name parameter or 'get_latest_by' in the model"
         assert self.query.can_filter(), \
             "Cannot change a query once a slice has been taken."
-        obj = self._clone()
-        obj.query.set_limits(high=1)
-        obj.query.clear_ordering(force_empty=True)
-        obj.query.add_ordering('%s%s' % (direction, order_by))
-        return obj.get()
+        clone = self._clone()
+        clone.query.set_limits(high=1)
+        clone.query.clear_ordering(force_empty=True)
+        clone.query.add_ordering('%s%s' % (direction, order_by))
+        return clone.get()
 
     def earliest(self, field_name=None):
         return self._earliest_or_latest(field_name=field_name, direction="")
@@ -512,20 +512,20 @@ class QuerySet(object):
         assert self.query.can_filter(), \
             "Cannot use 'limit' or 'offset' with delete."
 
-        del_query = self._clone()
+        clone = self._clone()
 
         # The delete is actually 2 queries - one to find related objects,
         # and one to delete. Make sure that the discovery of related
         # objects is performed on the same database as the deletion.
-        del_query._for_write = True
+        clone._for_write = True
 
         # Disable non-supported fields.
-        del_query.query.select_for_update = False
-        del_query.query.select_related = False
-        del_query.query.clear_ordering(force_empty=True)
+        clone.query.select_for_update = False
+        clone.query.select_related = False
+        clone.query.clear_ordering(force_empty=True)
 
-        collector = Collector(using=del_query.db)
-        collector.collect(del_query)
+        collector = Collector(using=clone.db)
+        collector.collect(clone)
         collector.delete()
 
         # Clear the result cache, in case this QuerySet gets reused.
@@ -594,8 +594,10 @@ class QuerySet(object):
                 params=params, translations=translations,
                 using=using)
 
-    def values(self, *fields):
-        return self._clone(klass=ValuesQuerySet, setup=True, _fields=fields)
+        clone = self._clone(klass=ValuesQuerySet)
+        clone.fields = fields
+        clone._setup_query()
+        return clone
 
     def values_list(self, *fields, **kwargs):
         flat = kwargs.pop('flat', False)
@@ -604,8 +606,12 @@ class QuerySet(object):
                     % (list(kwargs),))
         if flat and len(fields) > 1:
             raise TypeError("'flat' is not valid when values_list is called with more than one field.")
-        return self._clone(klass=ValuesListQuerySet, setup=True, flat=flat,
-                _fields=fields)
+        clone = self._clone(klass=ValuesListQuerySet)
+        clone.fields = fields
+        clone.aliased_fields = {}
+        clone.flat = flat
+        clone._setup_query()
+        return clone
 
     def dates(self, field_name, kind, order='ASC'):
         """
@@ -711,11 +717,11 @@ class QuerySet(object):
         Returns a new QuerySet instance that will select objects with a
         FOR UPDATE lock.
         """
-        obj = self._clone()
-        obj._for_write = True
-        obj.query.select_for_update = True
-        obj.query.select_for_update_nowait = nowait
-        return obj
+        clone = self._clone()
+        clone._for_write = True
+        clone.query.select_for_update = True
+        clone.query.select_for_update_nowait = nowait
+        return clone
 
     def select_related(self, *fields):
         """
@@ -726,14 +732,14 @@ class QuerySet(object):
 
         If select_related(None) is called, the list is cleared.
         """
-        obj = self._clone()
+        clone = self._clone()
         if fields == (None,):
-            obj.query.select_related = False
+            clone.query.select_related = False
         elif fields:
-            obj.query.add_select_related(fields)
+            clone.query.add_select_related(fields)
         else:
-            obj.query.select_related = True
-        return obj
+            clone.query.select_related = True
+        return clone
 
     def prefetch_related(self, *lookups):
         """
@@ -771,8 +777,8 @@ class QuerySet(object):
             annotations[arg.default_alias] = arg
         annotations.update(kwargs)
 
-        obj = self._clone()
-        names = getattr(self, '_fields', None)
+        clone = self._clone()
+        names = getattr(self, 'fields', None)
         if names is None:
             names = {f.name for f in self.model._meta.get_fields()}
 
@@ -781,16 +787,16 @@ class QuerySet(object):
             if alias in names:
                 raise ValueError("The annotation '%s' conflicts with a field on "
                                  "the model." % alias)
-            obj.query.add_annotation(annotation, alias, is_summary=False)
+            clone.query.add_annotation(annotation, alias, is_summary=False)
         # expressions need to be added to the query before we know if they contain aggregates
         added_aggregates = []
-        for alias, annotation in obj.query.annotations.items():
+        for alias, annotation in clone.query.annotations.items():
             if alias in annotations and annotation.contains_aggregate:
                 added_aggregates.append(alias)
         if added_aggregates:
-            obj._setup_aggregate_query(list(added_aggregates))
+            clone._setup_aggregate_query(list(added_aggregates))
 
-        return obj
+        return clone
 
     def order_by(self, *field_names):
         """
@@ -798,10 +804,10 @@ class QuerySet(object):
         """
         assert self.query.can_filter(), \
             "Cannot reorder a query once a slice has been taken."
-        obj = self._clone()
-        obj.query.clear_ordering(force_empty=False)
-        obj.query.add_ordering(*field_names)
-        return obj
+        clone = self._clone()
+        clone.query.clear_ordering(force_empty=False)
+        clone.query.add_ordering(*field_names)
+        return clone
 
     def distinct(self, *field_names):
         """
@@ -809,9 +815,9 @@ class QuerySet(object):
         """
         assert self.query.can_filter(), \
             "Cannot create distinct fields once a slice has been taken."
-        obj = self._clone()
-        obj.query.add_distinct_fields(*field_names)
-        return obj
+        clone = self._clone()
+        clone.query.add_distinct_fields(*field_names)
+        return clone
 
     def extra(self, select=None, where=None, params=None, tables=None,
               order_by=None, select_params=None):
@@ -926,7 +932,7 @@ class QuerySet(object):
             self.model._base_manager._insert(batch, fields=fields,
                                              using=self.db)
 
-    def _clone(self, klass=None, setup=False, **kwargs):
+    def _clone(self, klass=None, **kwargs):
         if klass is None:
             klass = self.__class__
         elif not issubclass(self.__class__, klass):
@@ -941,14 +947,12 @@ class QuerySet(object):
         query = self.query.clone()
         if self._sticky_filter:
             query.filter_is_sticky = True
-        c = klass(model=self.model, query=query, using=self._db, hints=self._hints)
-        c._for_write = self._for_write
-        c._prefetch_related_lookups = self._prefetch_related_lookups[:]
-        c._known_related_objects = self._known_related_objects
-        c.__dict__.update(kwargs)
-        if setup and hasattr(c, '_setup_query'):
-            c._setup_query()
-        return c
+        clone = klass(model=self.model, query=query, using=self._db, hints=self._hints)
+        clone._for_write = self._for_write
+        clone._prefetch_related_lookups = self._prefetch_related_lookups[:]
+        clone._known_related_objects = self._known_related_objects
+        clone.__dict__.update(kwargs)
+        return clone
 
     def _fetch_all(self):
         if self._result_cache is None:
@@ -1052,12 +1056,9 @@ class EmptyQuerySet(six.with_metaclass(InstanceCheckMeta)):
 class ValuesQuerySet(QuerySet):
     def __init__(self, *args, **kwargs):
         super(ValuesQuerySet, self).__init__(*args, **kwargs)
-        # select_related isn't supported in values(). (FIXME -#3358)
+        # ValuesQuerySet ignores select_related.
         self.query.select_related = False
 
-        # QuerySet.clone() will also set up the _fields attribute with the
-        # names of the model fields to select.
-
     def only(self, *fields):
         raise NotImplementedError("ValuesQuerySet does not implement only()")
 
@@ -1094,17 +1095,17 @@ class ValuesQuerySet(QuerySet):
             self.query.set_group_by()
         self.query.clear_deferred_loading()
         self.query.clear_select_fields()
-        if self._fields:
+        if self.fields:
             self.extra_names = []
             self.annotation_names = []
             if not self.query._extra and not self.query._annotations:
                 # Short cut - if there are no extra or annotations, then
                 # the values() clause must be just field names.
-                self.field_names = list(self._fields)
+                self.field_names = list(self.fields)
             else:
                 self.query.default_cols = False
                 self.field_names = []
-                for f in self._fields:
+                for f in self.fields:
                     # we inspect the full extra_select list since we might
                     # be adding back an extra select item that we hadn't
                     # had selected previously.
@@ -1127,21 +1128,16 @@ class ValuesQuerySet(QuerySet):
         if self.annotation_names is not None:
             self.query.set_annotation_mask(self.annotation_names)
 
-    def _clone(self, klass=None, setup=False, **kwargs):
+    def _clone(self, klass=None, **kwargs):
         """
         Cloning a ValuesQuerySet preserves the current fields.
         """
-        c = super(ValuesQuerySet, self)._clone(klass, **kwargs)
-        if not hasattr(c, '_fields'):
-            # Only clone self._fields if _fields wasn't passed into the cloning
-            # call directly.
-            c._fields = self._fields[:]
-        c.field_names = self.field_names
-        c.extra_names = self.extra_names
-        c.annotation_names = self.annotation_names
-        if setup and hasattr(c, '_setup_query'):
-            c._setup_query()
-        return c
+        clone = super(ValuesQuerySet, self)._clone(klass, **kwargs)
+        clone.fields = self.fields
+        clone.field_names = self.field_names
+        clone.extra_names = self.extra_names
+        clone.annotation_names = self.annotation_names
+        return clone
 
     def _merge_sanity_check(self, other):
         super(ValuesQuerySet, self)._merge_sanity_check(other)
@@ -1171,8 +1167,9 @@ class ValuesQuerySet(QuerySet):
         returned). This differs from QuerySet.as_sql(), where the column to
         select is set up by Django.
         """
-        if ((self._fields and len(self._fields) > 1) or
-                (not self._fields and len(self.model._meta.fields) > 1)):
+        fields = list(self.fields) + list(self.aliased_fields.values())
+        if ((fields and len(fields) > 1) or
+                (not fields and len(self.model._meta.fields) > 1)):
             raise TypeError('Cannot use a multi-field %s as a filter value.'
                     % self.__class__.__name__)
 
@@ -1186,8 +1183,9 @@ class ValuesQuerySet(QuerySet):
         Validates that we aren't trying to do a query like
         value__in=qs.values('value1', 'value2'), which isn't valid.
         """
-        if ((self._fields and len(self._fields) > 1) or
-                (not self._fields and len(self.model._meta.fields) > 1)):
+        fields = list(self.fields) + list(self.aliased_fields.values())
+        if ((fields and len(fields) > 1) or
+                (not fields and len(self.model._meta.fields) > 1)):
             raise TypeError('Cannot use a multi-field %s as a filter value.'
                     % self.__class__.__name__)
         return self
@@ -1203,7 +1201,7 @@ class ValuesQuerySet(QuerySet):
 class ValuesListQuerySet(ValuesQuerySet):
     def iterator(self):
         compiler = self.query.get_compiler(self.db)
-        if self.flat and len(self._fields) == 1:
+        if self.flat and len(self.fields) == 1:
             for row in compiler.results_iter():
                 yield row[0]
         elif not self.query.extra_select and not self.query.annotation_select:
@@ -1212,7 +1210,7 @@ class ValuesListQuerySet(ValuesQuerySet):
         else:
             # When extra(select=...) or an annotation is involved, the extra
             # cols are always at the start of the row, and we need to reorder
-            # the fields to match the order in self._fields.
+            # the fields to match the order in self.fields.
             extra_names = list(self.query.extra_select)
             field_names = self.field_names
             annotation_names = list(self.query.annotation_select)
@@ -1221,8 +1219,8 @@ class ValuesListQuerySet(ValuesQuerySet):
 
             # If a field list has been specified, use it. Otherwise, use the
             # full list of fields, including extras and annotations.
-            if self._fields:
-                fields = list(self._fields) + [f for f in annotation_names if f not in self._fields]
+            if self.fields:
+                fields = list(self.fields) + [f for f in annotation_names if f not in self.fields]
             else:
                 fields = names
 
@@ -1232,9 +1230,7 @@ class ValuesListQuerySet(ValuesQuerySet):
 
     def _clone(self, *args, **kwargs):
         clone = super(ValuesListQuerySet, self)._clone(*args, **kwargs)
-        if not hasattr(clone, "flat"):
-            # Only assign flat if the clone didn't already get it from kwargs
-            clone.flat = self.flat
+        clone.flat = self.flat
         return clone