Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Reorganised the internals of the Where node a bit to fix some copying…

… problems.

We no longer store any reference to Django field instances or models in the
Where node. This should improve cloning speed, fix some pickling difficulties,
reduce memory usage and remove some infinite loop possibilities in odd cases.
Slightly backwards incompatible if you're writing custom filters. See the
BackwardsIncompatibleChanges wiki page for details.

Fixed #7128, #7204, #7506.


git-svn-id: http://code.djangoproject.com/svn/django/trunk@7773 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information...
commit ade4a1246c5e65ed544a990a720cf1c953ec604b 1 parent c237ef6
@malcolmt malcolmt authored
View
20 django/db/models/sql/query.py
@@ -7,6 +7,7 @@
all about the internals of models in order to get the information it needs.
"""
+import datetime
from copy import deepcopy
from django.utils.tree import Node
@@ -1048,7 +1049,19 @@ def add_filter(self, filter_expr, connector=AND, negate=False, trim=False,
# that's harmless.
self.promote_alias(table)
- self.where.add((alias, col, field, lookup_type, value), connector)
+ # To save memory and copying time, convert the value from the Python
+ # object to the actual value used in the SQL query.
+ if field:
+ params = field.get_db_prep_lookup(lookup_type, value)
+ else:
+ params = Field().get_db_prep_lookup(lookup_type, value)
+ if isinstance(value, datetime.datetime):
+ annotation = datetime.datetime
+ else:
+ annotation = bool(value)
+
+ self.where.add((alias, col, field.db_type(), lookup_type, annotation,
+ params), connector)
if negate:
for alias in join_list:
@@ -1058,7 +1071,8 @@ def add_filter(self, filter_expr, connector=AND, negate=False, trim=False,
for alias in join_list:
if self.alias_map[alias][JOIN_TYPE] == self.LOUTER:
j_col = self.alias_map[alias][RHS_JOIN_COL]
- entry = Node([(alias, j_col, None, 'isnull', True)])
+ entry = Node([(alias, j_col, None, 'isnull', True,
+ [True])])
entry.negate()
self.where.add(entry, AND)
break
@@ -1066,7 +1080,7 @@ def add_filter(self, filter_expr, connector=AND, negate=False, trim=False,
# Leaky abstraction artifact: We have to specifically
# exclude the "foo__in=[]" case from this handling, because
# it's short-circuited in the Where class.
- entry = Node([(alias, col, field, 'isnull', True)])
+ entry = Node([(alias, col, None, 'isnull', True, [True])])
entry.negate()
self.where.add(entry, AND)
View
12 django/db/models/sql/subqueries.py
@@ -49,7 +49,7 @@ def delete_batch_related(self, pk_list):
for offset in range(0, len(pk_list), GET_ITERATOR_CHUNK_SIZE):
where = self.where_class()
where.add((None, related.field.m2m_reverse_name(),
- related.field, 'in',
+ related.field.db_type(), 'in', True,
pk_list[offset : offset+GET_ITERATOR_CHUNK_SIZE]),
AND)
self.do_query(related.field.m2m_db_table(), where)
@@ -59,11 +59,11 @@ def delete_batch_related(self, pk_list):
if isinstance(f, generic.GenericRelation):
from django.contrib.contenttypes.models import ContentType
field = f.rel.to._meta.get_field(f.content_type_field_name)
- w1.add((None, field.column, field, 'exact',
- ContentType.objects.get_for_model(cls).id), AND)
+ w1.add((None, field.column, field.db_type(), 'exact', True,
+ [ContentType.objects.get_for_model(cls).id]), AND)
for offset in range(0, len(pk_list), GET_ITERATOR_CHUNK_SIZE):
where = self.where_class()
- where.add((None, f.m2m_column_name(), f, 'in',
+ where.add((None, f.m2m_column_name(), f.db_type(), 'in', True,
pk_list[offset : offset + GET_ITERATOR_CHUNK_SIZE]),
AND)
if w1:
@@ -81,7 +81,7 @@ def delete_batch(self, pk_list):
for offset in range(0, len(pk_list), GET_ITERATOR_CHUNK_SIZE):
where = self.where_class()
field = self.model._meta.pk
- where.add((None, field.column, field, 'in',
+ where.add((None, field.column, field.db_type(), 'in', True,
pk_list[offset : offset + GET_ITERATOR_CHUNK_SIZE]), AND)
self.do_query(self.model._meta.db_table, where)
@@ -204,7 +204,7 @@ def clear_related(self, related_field, pk_list):
for offset in range(0, len(pk_list), GET_ITERATOR_CHUNK_SIZE):
self.where = self.where_class()
f = self.model._meta.pk
- self.where.add((None, f.column, f, 'in',
+ self.where.add((None, f.column, f.db_type(), 'in', True,
pk_list[offset : offset + GET_ITERATOR_CHUNK_SIZE]),
AND)
self.values = [(related_field.column, None, '%s')]
View
27 django/db/models/sql/where.py
@@ -21,8 +21,9 @@ class WhereNode(tree.Node):
the correct SQL).
The children in this tree are usually either Q-like objects or lists of
- [table_alias, field_name, field_class, lookup_type, value]. However, a
- child could also be any class with as_sql() and relabel_aliases() methods.
+ [table_alias, field_name, db_type, lookup_type, value_annotation,
+ params]. However, a child could also be any class with as_sql() and
+ relabel_aliases() methods.
"""
default = AND
@@ -88,29 +89,25 @@ def as_sql(self, node=None, qn=None):
def make_atom(self, child, qn):
"""
- Turn a tuple (table_alias, field_name, field_class, lookup_type, value)
- into valid SQL.
+ Turn a tuple (table_alias, field_name, db_type, lookup_type,
+ value_annot, params) into valid SQL.
Returns the string for the SQL fragment and the parameters to use for
it.
"""
- table_alias, name, field, lookup_type, value = child
+ table_alias, name, db_type, lookup_type, value_annot, params = child
if table_alias:
lhs = '%s.%s' % (qn(table_alias), qn(name))
else:
lhs = qn(name)
- db_type = field and field.db_type() or None
+ ##db_type = field and field.db_type() or None
field_sql = connection.ops.field_cast_sql(db_type) % lhs
- if isinstance(value, datetime.datetime):
+ if value_annot is datetime.datetime:
cast_sql = connection.ops.datetime_cast_sql()
else:
cast_sql = '%s'
- if field:
- params = field.get_db_prep_lookup(lookup_type, value)
- else:
- params = Field().get_db_prep_lookup(lookup_type, value)
if isinstance(params, QueryWrapper):
extra, params = params.data
else:
@@ -123,11 +120,11 @@ def make_atom(self, child, qn):
connection.operators[lookup_type] % cast_sql), params)
if lookup_type == 'in':
- if not value:
+ if not value_annot:
raise EmptyResultSet
if extra:
return ('%s IN %s' % (field_sql, extra), params)
- return ('%s IN (%s)' % (field_sql, ', '.join(['%s'] * len(value))),
+ return ('%s IN (%s)' % (field_sql, ', '.join(['%s'] * len(params))),
params)
elif lookup_type in ('range', 'year'):
return ('%s BETWEEN %%s and %%s' % field_sql, params)
@@ -135,8 +132,8 @@ def make_atom(self, child, qn):
return ('%s = %%s' % connection.ops.date_extract_sql(lookup_type,
field_sql), params)
elif lookup_type == 'isnull':
- return ('%s IS %sNULL' % (field_sql, (not value and 'NOT ' or '')),
- params)
+ return ('%s IS %sNULL' % (field_sql,
+ (not value_annot and 'NOT ' or '')), ())
elif lookup_type == 'search':
return (connection.ops.fulltext_search_sql(field_sql), params)
elif lookup_type in ('regex', 'iregex'):
View
5 tests/regressiontests/queries/models.py
@@ -3,6 +3,7 @@
"""
import datetime
+import pickle
from django.db import models
from django.db.models.query import Q
@@ -791,5 +792,9 @@ class Child(models.Model):
>>> Note.objects.all() & Note.objects.none()
[]
+Bug #7204, #7506 -- make sure querysets with related fields can be pickled. If
+this doesn't crash, it's a Good Thing.
+>>> out = pickle.dumps(Item.objects.all())
+
"""}
Please sign in to comment.
Something went wrong with that request. Please try again.