Commit 6dd2b546 authored by Malcolm Tredinnick's avatar Malcolm Tredinnick
Browse files

Redo the changes in [7773] in a better way.

This removes some of the leaky abstraction problems (lifting WhereNode
internals into the Query class) from that commit and makes it possible for
extensions to WhereNode to have access to the field instances. It's also
backwards-compatible with pre-[7773] code, which is also better.


git-svn-id: http://code.djangoproject.com/svn/django/trunk@7835 bcc190cf-cafb-0310-a4f2-bffc1f526a37
parent 3b648713
Loading
Loading
Loading
Loading
+5 −16
Original line number Diff line number Diff line
@@ -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)

+6 −6
Original line number Diff line number Diff line
@@ -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')]
+48 −23
Original line number Diff line number Diff line
@@ -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
+25 −6
Original line number Diff line number Diff line
@@ -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