Created
January 21, 2015 06:33
-
-
Save loic/0d40b6bc16d70fcf88c8 to your computer and use it in GitHub Desktop.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
commit ac7828487fbb574e098bdf0cb9f727b05f35d240 | |
Author: Loic Bistuer <[email protected]> | |
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 | |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment