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
This commit is contained in:
Malcolm Tredinnick 2008-06-27 03:27:20 +00:00
parent c237ef625c
commit ade4a1246c
4 changed files with 40 additions and 24 deletions

View File

@ -7,6 +7,7 @@ databases). The abstraction barrier only works one way: this module has to know
all about the internals of models in order to get the information it needs. all about the internals of models in order to get the information it needs.
""" """
import datetime
from copy import deepcopy from copy import deepcopy
from django.utils.tree import Node from django.utils.tree import Node
@ -1048,7 +1049,19 @@ class Query(object):
# that's harmless. # that's harmless.
self.promote_alias(table) 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: if negate:
for alias in join_list: for alias in join_list:
@ -1058,7 +1071,8 @@ class Query(object):
for alias in join_list: for alias in join_list:
if self.alias_map[alias][JOIN_TYPE] == self.LOUTER: if self.alias_map[alias][JOIN_TYPE] == self.LOUTER:
j_col = self.alias_map[alias][RHS_JOIN_COL] 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() entry.negate()
self.where.add(entry, AND) self.where.add(entry, AND)
break break
@ -1066,7 +1080,7 @@ class Query(object):
# Leaky abstraction artifact: We have to specifically # Leaky abstraction artifact: We have to specifically
# exclude the "foo__in=[]" case from this handling, because # exclude the "foo__in=[]" case from this handling, because
# it's short-circuited in the Where class. # 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() entry.negate()
self.where.add(entry, AND) self.where.add(entry, AND)

View File

@ -49,7 +49,7 @@ class DeleteQuery(Query):
for offset in range(0, len(pk_list), GET_ITERATOR_CHUNK_SIZE): for offset in range(0, len(pk_list), GET_ITERATOR_CHUNK_SIZE):
where = self.where_class() where = self.where_class()
where.add((None, related.field.m2m_reverse_name(), 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]), pk_list[offset : offset+GET_ITERATOR_CHUNK_SIZE]),
AND) AND)
self.do_query(related.field.m2m_db_table(), where) self.do_query(related.field.m2m_db_table(), where)
@ -59,11 +59,11 @@ class DeleteQuery(Query):
if isinstance(f, generic.GenericRelation): if isinstance(f, generic.GenericRelation):
from django.contrib.contenttypes.models import ContentType from django.contrib.contenttypes.models import ContentType
field = f.rel.to._meta.get_field(f.content_type_field_name) field = f.rel.to._meta.get_field(f.content_type_field_name)
w1.add((None, field.column, field, 'exact', w1.add((None, field.column, field.db_type(), 'exact', True,
ContentType.objects.get_for_model(cls).id), AND) [ContentType.objects.get_for_model(cls).id]), AND)
for offset in range(0, len(pk_list), GET_ITERATOR_CHUNK_SIZE): for offset in range(0, len(pk_list), GET_ITERATOR_CHUNK_SIZE):
where = self.where_class() 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]), pk_list[offset : offset + GET_ITERATOR_CHUNK_SIZE]),
AND) AND)
if w1: if w1:
@ -81,7 +81,7 @@ class DeleteQuery(Query):
for offset in range(0, len(pk_list), GET_ITERATOR_CHUNK_SIZE): for offset in range(0, len(pk_list), GET_ITERATOR_CHUNK_SIZE):
where = self.where_class() where = self.where_class()
field = self.model._meta.pk 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) pk_list[offset : offset + GET_ITERATOR_CHUNK_SIZE]), AND)
self.do_query(self.model._meta.db_table, where) self.do_query(self.model._meta.db_table, where)
@ -204,7 +204,7 @@ class UpdateQuery(Query):
for offset in range(0, len(pk_list), GET_ITERATOR_CHUNK_SIZE): for offset in range(0, len(pk_list), GET_ITERATOR_CHUNK_SIZE):
self.where = self.where_class() self.where = self.where_class()
f = self.model._meta.pk 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]), pk_list[offset : offset + GET_ITERATOR_CHUNK_SIZE]),
AND) AND)
self.values = [(related_field.column, None, '%s')] self.values = [(related_field.column, None, '%s')]

View File

@ -21,8 +21,9 @@ class WhereNode(tree.Node):
the correct SQL). the correct SQL).
The children in this tree are usually either Q-like objects or lists of 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 [table_alias, field_name, db_type, lookup_type, value_annotation,
child could also be any class with as_sql() and relabel_aliases() methods. params]. However, a child could also be any class with as_sql() and
relabel_aliases() methods.
""" """
default = AND default = AND
@ -88,29 +89,25 @@ class WhereNode(tree.Node):
def make_atom(self, child, qn): def make_atom(self, child, qn):
""" """
Turn a tuple (table_alias, field_name, field_class, lookup_type, value) Turn a tuple (table_alias, field_name, db_type, lookup_type,
into valid SQL. value_annot, params) into valid SQL.
Returns the string for the SQL fragment and the parameters to use for Returns the string for the SQL fragment and the parameters to use for
it. it.
""" """
table_alias, name, field, lookup_type, value = child table_alias, name, db_type, lookup_type, value_annot, params = child
if table_alias: if table_alias:
lhs = '%s.%s' % (qn(table_alias), qn(name)) lhs = '%s.%s' % (qn(table_alias), qn(name))
else: else:
lhs = qn(name) 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 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() cast_sql = connection.ops.datetime_cast_sql()
else: else:
cast_sql = '%s' 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): if isinstance(params, QueryWrapper):
extra, params = params.data extra, params = params.data
else: else:
@ -123,11 +120,11 @@ class WhereNode(tree.Node):
connection.operators[lookup_type] % cast_sql), params) connection.operators[lookup_type] % cast_sql), params)
if lookup_type == 'in': if lookup_type == 'in':
if not value: if not value_annot:
raise EmptyResultSet raise EmptyResultSet
if extra: if extra:
return ('%s IN %s' % (field_sql, extra), params) 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) params)
elif lookup_type in ('range', 'year'): elif lookup_type in ('range', 'year'):
return ('%s BETWEEN %%s and %%s' % field_sql, params) return ('%s BETWEEN %%s and %%s' % field_sql, params)
@ -135,8 +132,8 @@ class WhereNode(tree.Node):
return ('%s = %%s' % connection.ops.date_extract_sql(lookup_type, return ('%s = %%s' % connection.ops.date_extract_sql(lookup_type,
field_sql), params) field_sql), params)
elif lookup_type == 'isnull': elif lookup_type == 'isnull':
return ('%s IS %sNULL' % (field_sql, (not value and 'NOT ' or '')), return ('%s IS %sNULL' % (field_sql,
params) (not value_annot and 'NOT ' or '')), ())
elif lookup_type == 'search': elif lookup_type == 'search':
return (connection.ops.fulltext_search_sql(field_sql), params) return (connection.ops.fulltext_search_sql(field_sql), params)
elif lookup_type in ('regex', 'iregex'): elif lookup_type in ('regex', 'iregex'):

View File

@ -3,6 +3,7 @@ Various complex queries that have been problematic in the past.
""" """
import datetime import datetime
import pickle
from django.db import models from django.db import models
from django.db.models.query import Q from django.db.models.query import Q
@ -791,5 +792,9 @@ Empty querysets can be merged with others.
>>> Note.objects.all() & Note.objects.none() >>> 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())
"""} """}