diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py index 6eccaf997ee..7944d0358fa 100644 --- a/django/db/models/sql/query.py +++ b/django/db/models/sql/query.py @@ -1104,19 +1104,7 @@ class Query(object): # that's harmless. self.promote_alias(table) - # 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) + self.where.add((alias, col, field, lookup_type, value), connector) if negate: for alias in join_list: @@ -1126,8 +1114,8 @@ class Query(object): 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, - [True])]) + entry = self.where_class() + entry.add((alias, j_col, None, 'isnull', True), AND) entry.negate() self.where.add(entry, AND) break @@ -1135,7 +1123,8 @@ class Query(object): # 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, None, 'isnull', True, [True])]) + entry = self.where_class() + entry.add((alias, col, None, 'isnull', True), AND) entry.negate() self.where.add(entry, AND) diff --git a/django/db/models/sql/subqueries.py b/django/db/models/sql/subqueries.py index d8c5b074eca..0bb741d7067 100644 --- a/django/db/models/sql/subqueries.py +++ b/django/db/models/sql/subqueries.py @@ -49,7 +49,7 @@ class DeleteQuery(Query): 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.db_type(), 'in', True, + related.field, 'in', pk_list[offset : offset+GET_ITERATOR_CHUNK_SIZE]), AND) self.do_query(related.field.m2m_db_table(), where) @@ -59,11 +59,11 @@ class DeleteQuery(Query): 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.db_type(), 'exact', True, - [ContentType.objects.get_for_model(cls).id]), AND) + w1.add((None, field.column, field, 'exact', + 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.db_type(), 'in', True, + where.add((None, f.m2m_column_name(), f, 'in', pk_list[offset : offset + GET_ITERATOR_CHUNK_SIZE]), AND) if w1: @@ -81,7 +81,7 @@ class DeleteQuery(Query): 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.db_type(), 'in', True, + where.add((None, field.column, field, 'in', pk_list[offset : offset + GET_ITERATOR_CHUNK_SIZE]), AND) 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): self.where = self.where_class() f = self.model._meta.pk - self.where.add((None, f.column, f.db_type(), 'in', True, + self.where.add((None, f.column, f, 'in', pk_list[offset : offset + GET_ITERATOR_CHUNK_SIZE]), AND) self.values = [(related_field.column, None, '%s')] diff --git a/django/db/models/sql/where.py b/django/db/models/sql/where.py index bf45bceb4ba..18e4bf2f7e9 100644 --- a/django/db/models/sql/where.py +++ b/django/db/models/sql/where.py @@ -27,7 +27,36 @@ class WhereNode(tree.Node): """ default = AND - def as_sql(self, node=None, qn=None): + def add(self, data, connector): + """ + Add a node to the where-tree. If the data is a list or tuple, it is + expected to be of the form (alias, col_name, field_obj, lookup_type, + value), which is then slightly munged before being stored (to avoid + storing any reference to field objects). Otherwise, the 'data' is + stored unchanged and can be anything with an 'as_sql()' method. + """ + if not isinstance(data, (list, tuple)): + super(WhereNode, self).add(data, connector) + return + + alias, col, field, lookup_type, value = data + if field: + params = field.get_db_prep_lookup(lookup_type, value) + db_type = field.db_type() + else: + # This is possible when we add a comparison to NULL sometimes (we + # don't really need to waste time looking up the associated field + # object). + params = Field().get_db_prep_lookup(lookup_type, value) + db_type = None + if isinstance(value, datetime.datetime): + annotation = datetime.datetime + else: + annotation = bool(value) + super(WhereNode, self).add((alias, col, db_type, lookup_type, + annotation, params), connector) + + def as_sql(self, qn=None): """ Returns the SQL version of the where clause and the value to be substituted in. Returns None, None if this node is empty. @@ -36,60 +65,56 @@ class WhereNode(tree.Node): (generally not needed except by the internal implementation for recursion). """ - if node is None: - node = self if not qn: qn = connection.ops.quote_name - if not node.children: + if not self.children: return None, [] result = [] result_params = [] empty = True - for child in node.children: + for child in self.children: try: if hasattr(child, 'as_sql'): sql, params = child.as_sql(qn=qn) - format = '(%s)' - elif isinstance(child, tree.Node): - sql, params = self.as_sql(child, qn) - if child.negated: - format = 'NOT (%s)' - elif len(child.children) == 1: - format = '%s' - else: - format = '(%s)' else: + # A leaf node in the tree. sql, params = self.make_atom(child, qn) - format = '%s' except EmptyResultSet: - if node.connector == AND and not node.negated: + if self.connector == AND and not self.negated: # We can bail out early in this particular case (only). raise - elif node.negated: + elif self.negated: empty = False continue except FullResultSet: if self.connector == OR: - if node.negated: + if self.negated: empty = True break # We match everything. No need for any constraints. return '', [] - if node.negated: + if self.negated: empty = True continue empty = False if sql: - result.append(format % sql) + result.append(sql) result_params.extend(params) if empty: raise EmptyResultSet - conn = ' %s ' % node.connector - return conn.join(result), result_params + + conn = ' %s ' % self.connector + sql_string = conn.join(result) + if sql_string: + if self.negated: + sql_string = 'NOT (%s)' % sql_string + elif len(self.children) != 1: + sql_string = '(%s)' % sql_string + return sql_string, result_params def make_atom(self, child, qn): """ - Turn a tuple (table_alias, field_name, db_type, lookup_type, + Turn a tuple (table_alias, column_name, db_type, lookup_type, value_annot, params) into valid SQL. Returns the string for the SQL fragment and the parameters to use for diff --git a/django/utils/tree.py b/django/utils/tree.py index a62a4ae6c3a..5dd9f567994 100644 --- a/django/utils/tree.py +++ b/django/utils/tree.py @@ -29,6 +29,22 @@ class Node(object): self.subtree_parents = [] self.negated = negated + # We need this because of django.db.models.query_utils.Q. Q. __init__() is + # problematic, but it is a natural Node subclass in all other respects. + def _new_instance(cls, children=None, connector=None, negated=False): + """ + This is called to create a new instance of this class when we need new + Nodes (or subclasses) in the internal code in this class. Normally, it + just shadows __init__(). However, subclasses with an __init__ signature + that is not an extension of Node.__init__ might need to implement this + method to allow a Node to create a new instance of them (if they have + any extra setting up to do). + """ + obj = Node(children, connector, negated) + obj.__class__ = cls + return obj + _new_instance = classmethod(_new_instance) + def __str__(self): if self.negated: return '(NOT (%s: %s))' % (self.connector, ', '.join([str(c) for c @@ -82,7 +98,8 @@ class Node(object): else: self.children.append(node) else: - obj = Node(self.children, self.connector, self.negated) + obj = self._new_instance(self.children, self.connector, + self.negated) self.connector = conn_type self.children = [obj, node] @@ -96,7 +113,8 @@ class Node(object): Interpreting the meaning of this negate is up to client code. This method is useful for implementing "not" arrangements. """ - self.children = [Node(self.children, self.connector, not self.negated)] + self.children = [self._new_instance(self.children, self.connector, + not self.negated)] self.connector = self.default def start_subtree(self, conn_type): @@ -108,12 +126,13 @@ class Node(object): if len(self.children) == 1: self.connector = conn_type elif self.connector != conn_type: - self.children = [Node(self.children, self.connector, self.negated)] + self.children = [self._new_instance(self.children, self.connector, + self.negated)] self.connector = conn_type self.negated = False - self.subtree_parents.append(Node(self.children, self.connector, - self.negated)) + self.subtree_parents.append(self.__class__(self.children, + self.connector, self.negated)) self.connector = self.default self.negated = False self.children = [] @@ -126,7 +145,7 @@ class Node(object): the current instances state to be the parent. """ obj = self.subtree_parents.pop() - node = Node(self.children, self.connector) + node = self.__class__(self.children, self.connector) self.connector = obj.connector self.negated = obj.negated self.children = obj.children