Context Navigation

Back to Ticket #17025

Ticket #17025: wherenode_refactor.diff

File wherenode_refactor.diff, 68.3 KB (added by Anssi Kääriäinen, 13 years ago)

django/db/models/query.py

diff --git a/django/db/models/query.py b/django/db/models/query.py
index be42d02..3070915 100644

                class QuerySet(object):
         Prepare the query for computing a result that contains aggregate annotations.
         """
         opts = self.model._meta
         if self.query.group_by is None:
+        if not self.query.group_by:
             field_names = [f.attname for f in opts.fields]
             self.query.add_fields(field_names, False)
             self.query.set_group_by()
+            self.query.group_by = True
     def _prepare(self):
         return self
-…
+               class ValuesQuerySet(QuerySet):
         if self._fields:
             self.extra_names = []
+            # We collect the aggregate names here from the backing query's
+            # aggregates. These are those aggregates that will be in the
+            # values list. We set the list here to [], so that in the end
+            # of this method we can check "is None" and set the aggregate
+            # mask of the backing query to empty indicating that we aren't
+            # actually interested in any fields. Naturally if we collect
+            # some aggregates, then the aggregate mask will be set to that.
             self.aggregate_names = []
             if not self.query.extra and not self.query.aggregates:
                 # Short cut - if there are no extra or aggregates, then
-…
+               class ValuesQuerySet(QuerySet):
             else:
                 self.query.default_cols = False
                 self.field_names = []
+                # OK, we have a list of fields - now we split them into
+                # fields which are aggregates, those which are from extra
+                # and normal fields. Why?
                 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
-…
+               class ValuesQuerySet(QuerySet):
             self.field_names = [f.attname for f in self.model._meta.fields]
             self.aggregate_names = None
+        # Why can't we just keep the values we are interested in, pass that
+        # into compiler, and let it do the final pruning?
         self.query.select = []
         if self.extra_names is not None:
             self.query.set_extra_mask(self.extra_names)
         self.query.add_fields(self.field_names, True)
+        # Ok, if we are called without fields, this means we do keep the
+        # aggregates.
         if self.aggregate_names is not None:
             self.query.set_aggregate_mask(self.aggregate_names)
-…
+               class ValuesQuerySet(QuerySet):
         """
         Prepare the query for computing a result that contains aggregate annotations.
         """
+        self.query.set_group_by()
+        # This super call will add all the fields in the model into the query,
+        # or do nothing if group_by is set. We call it, but it will not do
+        # anything.
+        self.query.group_by = True
+        super(ValuesQuerySet, self)._setup_aggregate_query(aggregates)
+        # Set the new additional aggregates into the aggregate mask.
         if self.aggregate_names is not None:
             self.aggregate_names.extend(aggregates)
             self.query.set_aggregate_mask(self.aggregate_names)
+        super(ValuesQuerySet, self)._setup_aggregate_query(aggregates)
     def _as_sql(self, connection):
         """

django/db/models/query_utils.py

diff --git a/django/db/models/query_utils.py b/django/db/models/query_utils.py
index a56ab5c..10e532d 100644

                class Q(tree.Node):
     def __init__(self, *args, **kwargs):
         super(Q, self).__init__(children=list(args) + kwargs.items())
+    def _new_instance(cls, children=None, connector=None, negated=False):
+        obj = tree.Node(children, connector, negated)
+        obj.__class__ = cls
+        return obj
+    _new_instance = classmethod(_new_instance)
     def _combine(self, other, conn):
         if not isinstance(other, Q):
             raise TypeError(other)
         obj = type(self)()
+        obj.add(self, conn)
+        obj.add(other, conn)
+        obj.connector = conn
+        if len(self) == 1 and not self.negated:
+            obj.add(self.children[0], conn)
+        else:
+            obj.add(self, conn)
+        if len(other) == 1 and not other.negated:
+            obj.add(other.children[0], conn)
+        else:
+            obj.add(other, conn)
         return obj
     def __or__(self, other):
-…
+               class Q(tree.Node):
         return self._combine(other, self.AND)
     def __invert__(self):
+        obj = type(self)()
+        obj.add(self, self.AND)
+        obj = self.clone()
         obj.negate()
         return obj

django/db/models/sql/aggregates.py

diff --git a/django/db/models/sql/aggregates.py b/django/db/models/sql/aggregates.py
index 207bc0c..aef8483 100644

-              a
+import copy
 """
 Classes to represent the default SQL aggregate functions
 """
-…
+               class Aggregate(object):
         self.field = tmp
+    def clone(self):
+        clone = copy.copy(self)
+        clone.col = self.col[:]
+        return clone
     def relabel_aliases(self, change_map):
         if isinstance(self.col, (list, tuple)):
             self.col = (change_map.get(self.col[0], self.col[0]), self.col[1])

django/db/models/sql/compiler.py

diff --git a/django/db/models/sql/compiler.py b/django/db/models/sql/compiler.py
index 6bf7de2..174cfd1 100644

                class SQLCompiler(object):
         self.quote_cache[name] = r
         return r
+    def where_to_sql(self):
+        """
+        This method is responsible for:
+           - Removing always True / always False parts of the tree
+           - Splitting the tree into having and where
+           - Getting the group by columns from the having part of the query
+           - And finally turning the remaining trees into SQL and params
+        Returns 3-tuple of the form:
+           ((where, w_params), (having, h_params), having_group_by)
+        Where the having_group_by is a set of SQL snippets to add into the
+        group by, for example ["T1".some_field]
+        """
+        # Prune the tree. If we are left with a tree that matches nothing
+        # this EmptyResultSet will be risen.
+        where = self.query.where.clone_internal()
+        where.final_prune(self.quote_name_unless_alias, self.connection)
+        if where.match_nothing:
+            raise EmptyResultSet
+        if self.query.aggregates:
+            having = self.query.where_class()
+            where.split_aggregates(having)
+            where.prune_tree(); having.prune_tree()
+            group_by = set(); having.get_group_by(group_by)
+            return (where.as_sql(), having.as_sql(), group_by)
+        else:
+            return (where and where.as_sql() or ('', []),  ('', []), set())
     def as_sql(self, with_limits=True, with_col_aliases=False):
         """
         Creates the SQL for this query. Returns the SQL string and list of
-…
+               class SQLCompiler(object):
         from_, f_params = self.get_from_clause()
         qn = self.quote_name_unless_alias
+        where, w_params = self.query.where.as_sql(qn=qn, connection=self.connection)
+        having, h_params = self.query.having.as_sql(qn=qn, connection=self.connection)
+        where_tpl, having_tpl, having_group_by = self.where_to_sql()
+        having, h_params = having_tpl
+        where, w_params = where_tpl
         params = []
         for val in self.query.extra_select.itervalues():
             params.extend(val[1])
-…
+               class SQLCompiler(object):
             result.append('WHERE %s' % where)
             params.extend(w_params)
         grouping, gb_params = self.get_grouping()
         if grouping:
+        grouping, gb_params = self.get_grouping(having_group_by)
+        if self.query.group_by:
             if ordering:
                 # If the backend can't group by PK (i.e., any database
                 # other than MySQL), then any fields mentioned in the
-…
+               class SQLCompiler(object):
                             gb_params.extend(col_params)
             else:
                 ordering = self.connection.ops.force_no_ordering()
+            result.append('GROUP BY %s' % ', '.join(grouping))
+            params.extend(gb_params)
+            if grouping:
+                result.append('GROUP BY %s' % ', '.join(grouping))
+                params.extend(gb_params)
         if having:
             result.append('HAVING %s' % having)
             params.extend(h_params)
+        if ordering:
+        # This is a hack: we rely on the ordering for GROUP BY. Subqueries do
+        # not use ordering, so instead of clearing the ordering, subqueries
+        # flag the query as not using the ordering there is defined. This is
+        # sure to bite us, and should be fixed. The real fix might be that
+        # relying on doing .order_by() to get the wanted GROUP BY might just
+        # need to be deprecated. Or maybe we should have a variable
+        # ordering_group_by, making it explicit that we collect the order_by
+        # GROUP BY clauses in different scope than the actual order by. But
+        # that just sounds hacky. Or maybe just resurrect the query.group_by
+        # set.
+        if ordering and self.query.use_ordering:
             result.append('ORDER BY %s' % ', '.join(ordering))
         if with_limits:
-…
+               class SQLCompiler(object):
         """
         obj = self.query.clone()
         if obj.low_mark == 0 and obj.high_mark is None:
+            # If there is no slicing in use, then we can safely drop all ordering
+            obj.clear_ordering(True)
+            # If there is no slicing in use, then we can safely drop all
+            # ordering.
+            # TODO: We rely on ordering to determine the GROUP BY clause.
+            # So we keep the ordering, but tell the compiler not to append
+            # it to the query, just to group by it. Refactor.
+            obj.use_ordering = False
+            obj.order_by = [f for f in self.query.order_by if f not in self.query.aggregates]
+            # We essentially defined a group_by variable above. It seems clear
+            # that we need a custom group_by variable, which we can then use
+            # properly. This was in the original code.
         obj.bump_prefix()
         return obj.get_compiler(connection=self.connection).as_sql()
-…
+               class SQLCompiler(object):
                 first = False
         return result, []
     def get_grouping(self):
+    def get_grouping(self, where_group_by):
         """
         Returns a tuple representing the SQL elements in the "group by" clause.
         """
+        if not self.query.group_by:
+             return [], []
         qn = self.quote_name_unless_alias
         result, params = [], []
         if self.query.group_by is not None:
             if (len(self.query.model._meta.fields) == len(self.query.select) and
                 self.connection.features.allows_group_by_pk):
                 self.query.group_by = [
+        group_by = where_group_by
+        if (len(self.query.model._meta.fields) == len(self.query.select) and
+            self.connection.features.allows_group_by_pk):
+                group_by = set([
                     (self.query.model._meta.db_table, self.query.model._meta.pk.column)
+                ]
+            group_by = self.query.group_by or []
+            extra_selects = []
+            for extra_select, extra_params in self.query.extra_select.itervalues():
+                extra_selects.append(extra_select)
+                params.extend(extra_params)
+            cols = (group_by + self.query.select +
+                self.query.related_select_cols + extra_selects)
+            seen = set()
+            for col in cols:
+                if col in seen:
+                    continue
+                seen.add(col)
+                if isinstance(col, (list, tuple)):
+                    result.append('%s.%s' % (qn(col[0]), qn(col[1])))
+                elif hasattr(col, 'as_sql'):
+                    result.append(col.as_sql(qn, self.connection))
+                else:
+                    result.append('(%s)' % str(col))
+                ])
+        extra_selects = []
+        for extra_select, extra_params in self.query.extra_select.itervalues():
+            extra_selects.append(extra_select)
+            params.extend(extra_params)
+        cols = group_by.union(self.query.select +
+            self.query.related_select_cols + extra_selects)
+        for col in cols:
+            if isinstance(col, (list, tuple)):
+                result.append('%s.%s' % (qn(col[0]), qn(col[1])))
+            elif hasattr(col, 'as_sql'):
+                result.append(col.as_sql(qn, self.connection))
+            else:
+                result.append('(%s)' % str(col))
         return result, params
     def fill_related_selections(self, opts=None, root_alias=None, cur_depth=1,
-…
+               class SQLDeleteCompiler(SQLCompiler):
                 "Can only delete from one table at a time."
         qn = self.quote_name_unless_alias
         result = ['DELETE FROM %s' % qn(self.query.tables[0])]
+        where, params = self.query.where.as_sql(qn=qn, connection=self.connection)
+        where_tpl, _, _ = self.where_to_sql()
+        where, params = where_tpl
         result.append('WHERE %s' % where)
         return ' '.join(result), tuple(params)
-…
+               class SQLUpdateCompiler(SQLCompiler):
         if not values:
             return '', ()
         result.append(', '.join(values))
+        where, params = self.query.where.as_sql(qn=qn, connection=self.connection)
+        where_tpl, _, _ = self.where_to_sql()
+        where, params = where_tpl
         if where:
             result.append('WHERE %s' % where)
         return ' '.join(result), tuple(update_params + params)

django/db/models/sql/datastructures.py

diff --git a/django/db/models/sql/datastructures.py b/django/db/models/sql/datastructures.py
index 92d64e1..b8e06da 100644

                the SQL domain.
 class EmptyResultSet(Exception):
     pass
-class FullResultSet(Exception):
-    pass
 class MultiJoin(Exception):
     """
     Used by join construction code to indicate the point at which a

django/db/models/sql/query.py

diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py
index 61fd2be..453df65 100644

                from django.db.models.sql import aggregates as base_aggregates_module
 from django.db.models.sql.constants import *
 from django.db.models.sql.datastructures import EmptyResultSet, Empty, MultiJoin
 from django.db.models.sql.expressions import SQLEvaluator
 from django.db.models.sql.where import (WhereNode, Constraint, EverythingNode,
     ExtraWhere, AND, OR)
+from django.db.models.sql.where import (WhereNode, Constraint, ExtraWhere,
+    AND, OR)
 from django.core.exceptions import FieldError
 __all__ = ['Query', 'RawQuery']
-…
+               class RawQuery(object):
         return RawQuery(self.sql, using, params=self.params)
     def convert_values(self, value, field, connection):
+        """Convert the database-returned value into a type that is consistent
+        """
+        Convert the database-returned value into a type that is consistent
         across database backends.
         By default, this defers to the underlying backend operations, but
-…
+               class RawQuery(object):
         self.cursor = connections[self.using].cursor()
         self.cursor.execute(self.sql, self.params)
 class Query(object):
     """
     A single SQL query.
-…
+               class Query(object):
         self.tables = []    # Aliases in the order they are created.
         self.where = where()
         self.where_class = where
+        self.group_by = None
+        self.having = where()
+        self.use_ordering = True
         self.order_by = []
         self.low_mark, self.high_mark = 0, None  # Used for offset/limit
         self.distinct = False
-…
+               class Query(object):
         self.select_related = False
         self.related_select_cols = []
+        # SQL aggregate-related attributes
+        # Here is some random rambling about aggregates. First, the current
+        # implementation is pretty darned hard to understand. There is little
+        # to no documentation, and there is usage of these variables all over
+        # the place.
+        #
+        # So, first, here is a list of what we will need for successful
+        # aggregate queries. First we will naturally need the actual
+        # aggregates, these are stored in self.aggregates, and this is
+        # pretty much clear thing.
+        #
+        # Next, we will need the fields to group by with. This we shouldn't
+        # keep record of, as the set of fields to group by is the wanted
+        # select fields, having fields, order fields, and extra select fields.
+        # We can and do compute these when the query gets executed. They will
+        # come as a side product when preparing other parts of the query for
+        # execution.
+        # Still, we need the having clause. This is under control now, as
+        # we have gotten rid of the query.having. We split the query.where
+        # into having and where based on the actual need.
+        # The current implementation of blindly adding the fields to the query
+        # is a bit dangerous - it leads to potential multijoins which will
+        # result in duplicate rows for aggregation. This is a hard problem to
+        # solve correctly.
+        # So, what do these variables below represent? aggregates is clear,
+        # it represents the aggregates in the query. Next comes group_by, this
+        # is a variable that represents if we should do a GROUP BY at all.
+        # Then we have self.aggregate_select_mask. This is the fields actually
+        # present in the query. Why self.aggregates do not get changed when we
+        # change the aggregates actually in the query is unknown. The actual
+        # fields in the query can be accessed through self.aggregate_select,
+        # which is a property showing only the fields in the aggregate select
+        # mask.
+        # To make things more complicated, db/query.py keeps its own variable
+        # aggregate_names. It seems this is collected from the aggregate_select
+        # property, and then used to add fields to the query. This is just
+        # speculation, I do not understand completely what it does.
         self.aggregates = SortedDict() # Maps alias -> SQL aggregate function
+        self.group_by = False
         self.aggregate_select_mask = None
         self._aggregate_select_cache = None
-…
+               class Query(object):
         obj.dupe_avoidance = self.dupe_avoidance.copy()
         obj.select = self.select[:]
         obj.tables = self.tables[:]
+        obj.where = copy.deepcopy(self.where, memo=memo)
+        # We do not need to clone the leaf nodes - they are immutable until
+        # the query is executed, or relabel_alias is called. In either case
+        # we will take care of the copying where needed. This can be a major
+        # speed optimization when the where three has a lot of leaf nodes.
+        obj.where = self.where.clone_internal()
         obj.where_class = self.where_class
-        if self.group_by is None:
-            obj.group_by = None
-        else:
-            obj.group_by = self.group_by[:]
-        obj.having = copy.deepcopy(self.having, memo=memo)
         obj.order_by = self.order_by[:]
+        obj.use_ordering = self.use_ordering
         obj.low_mark, obj.high_mark = self.low_mark, self.high_mark
         obj.distinct = self.distinct
         obj.select_for_update = self.select_for_update
         obj.select_for_update_nowait = self.select_for_update_nowait
         obj.select_related = self.select_related
         obj.related_select_cols = []
+        obj.aggregates = copy.deepcopy(self.aggregates, memo=memo)
+        if self.aggregates:
+            obj.aggregates = copy.deepcopy(self.aggregates, memo=memo)
+        else:
+            obj.aggregates = SortedDict()
         if self.aggregate_select_mask is None:
             obj.aggregate_select_mask = None
         else:
-…
+               class Query(object):
         # It will get re-populated in the cloned queryset the next time it's
         # used.
         obj._aggregate_select_cache = None
+        obj.group_by = self.group_by
         obj.max_depth = self.max_depth
         obj.extra = self.extra.copy()
         if self.extra_select_mask is None:
-…
+               class Query(object):
             obj._extra_select_cache = self._extra_select_cache.copy()
         obj.extra_tables = self.extra_tables
         obj.extra_order_by = self.extra_order_by
         obj.deferred_loading = copy.deepcopy(self.deferred_loading, memo=memo)
+        obj.deferred_loading = self.deferred_loading[0].copy(), self.deferred_loading[1]
         if self.filter_is_sticky and self.used_aliases:
             obj.used_aliases = self.used_aliases.copy()
         else:
-…
+               class Query(object):
         # If there is a group by clause, aggregating does not add useful
         # information but retrieves only the first row. Aggregate
         # over the subquery instead.
         if self.group_by is not None:
+        if self.group_by:
             from django.db.models.sql.subqueries import AggregateQuery
             query = AggregateQuery(self.model)
-…
+               class Query(object):
                 obj.add_subquery(subquery, using=using)
             except EmptyResultSet:
                 # add_subquery evaluates the query, if it's an EmptyResultSet
+                # then there are can be no results, and therefore there the
+                # count is obviously 0
+                # then there can be no results. Therefore the count is 0.
                 return 0
         obj.add_count_column()
-…
+               class Query(object):
                 if self.alias_refcount.get(alias) or rhs.alias_refcount.get(alias):
                     self.promote_alias(alias, True)
+        # Now relabel a copy of the rhs where-clause and add it to the current
+        # one.
+        if rhs.where:
+            w = copy.deepcopy(rhs.where)
+            w.relabel_aliases(change_map)
+            if not self.where:
+                # Since 'self' matches everything, add an explicit "include
+                # everything" where-constraint so that connections between the
+                # where clauses won't exclude valid results.
+                self.where.add(EverythingNode(), AND)
+        elif self.where:
+            # rhs has an empty where clause.
+            w = self.where_class()
+            w.add(EverythingNode(), AND)
+        if connector == OR and (not self.where or not rhs.where):
+            # One of the two sides matches everything and the connector is OR.
+            # This means the new where condition must match everything.
+            self.where = self.where_class()
         else:
+            w = self.where_class()
+        self.where.add(w, connector)
+            rhs_where = rhs.where.clone()
+            rhs_where.relabel_aliases(change_map)
+            self.where = self.where_class([self.where, rhs_where], connector)
+            # the root node's connector must always be AND
+            if self.where.connector == OR:
+                self.where = self.where_class([self.where])
+        self.where.prune_tree()
         # Selection columns and extra extensions are those provided by 'rhs'.
         self.select = []
-…
+               class Query(object):
         assert set(change_map.keys()).intersection(set(change_map.values())) == set()
         # 1. Update references in "select" (normal columns plus aliases),
         # "group by", "where" and "having".
+        # "group by" and  "where"
         self.where.relabel_aliases(change_map)
+        self.having.relabel_aliases(change_map)
+        for columns in [self.select, self.group_by or []]:
+        for columns in [self.select]:
             for pos, col in enumerate(columns):
                 if isinstance(col, (list, tuple)):
                     old_alias = col[0]
-…
+               class Query(object):
         The 'exceptions' parameter is a container that holds alias names which
         should not be changed.
         """
+        # We must make sure the leaf nodes of the where tree will be cloned,
+        # as they will be relabeled.
+        self.where = self.where.clone()
         current = ord(self.alias_prefix)
         assert current < ord('Z')
         prefix = chr(current + 1)
-…
+               class Query(object):
                 self.unref_alias(alias)
         self.included_inherited_models = {}
-    def need_force_having(self, q_object):
-        """
-        Returns whether or not all elements of this q_object need to be put
-        together in the HAVING clause.
-        """
-        for child in q_object.children:
-            if isinstance(child, Node):
-                if self.need_force_having(child):
-                    return True
-            else:
-                if child[0].split(LOOKUP_SEP)[0] in self.aggregates:
-                    return True
-        return False
     def add_aggregate(self, aggregate, model, alias, is_summary):
         """
         Adds a single aggregate expression to the Query
-…
+               class Query(object):
                     aggregate.name, field_name, field_name))
         elif ((len(field_list) > 1) or
             (field_list[0] not in [i.name for i in opts.fields]) or
             self.group_by is None or
+            not self.group_by or
             not is_summary):
             # If:
             #   - the field descriptor has more than one part (foo__bar), or
-…
+               class Query(object):
         # Add the aggregate to the query
         aggregate.add_to_query(self, alias, col=col, source=source, is_summary=is_summary)
+    def add_where_leaf(self, data, negated=False):
+        leaf_class = self.where.leaf_class()
+        self.where.add(leaf_class(data, negated), AND)
     def add_filter(self, filter_expr, connector=AND, negate=False, trim=False,
             can_reuse=None, process_extras=True, force_having=False):
+            can_reuse=None, process_extras=True):
         """
         Add a single filter to the query. The 'filter_expr' is a pair:
         (filter_string, value). E.g. ('name__contains', 'fred')
-…
+               class Query(object):
         # By default, this is a WHERE clause. If an aggregate is referenced
         # in the value, the filter will be promoted to a HAVING
-        having_clause = False
         # Interpret '__exact=None' as the sql 'is NULL'; otherwise, reject all
         # uses of None as a query value.
-…
+               class Query(object):
         elif hasattr(value, 'evaluate'):
             # If value is a query expression, evaluate it
             value = SQLEvaluator(value, self)
-            having_clause = value.contains_aggregate
         for alias, aggregate in self.aggregates.items():
             if alias in (parts[0], LOOKUP_SEP.join(parts)):
+                entry = self.where_class()
+                entry.add((aggregate, lookup_type, value), AND)
+                if negate:
+                    entry.negate()
+                self.having.add(entry, connector)
+                self.add_where_leaf((aggregate, lookup_type, value))
                 return
         opts = self.get_meta()
-…
+               class Query(object):
             self.promote_alias_chain(join_it, join_promote)
             self.promote_alias_chain(table_it, table_promote or join_promote)
+        if having_clause or force_having:
+            if (alias, col) not in self.group_by:
+                self.group_by.append((alias, col))
+            self.having.add((Constraint(alias, col, field), lookup_type, value),
+                connector)
+        else:
+            self.where.add((Constraint(alias, col, field), lookup_type, value),
+                connector)
+        self.add_where_leaf((Constraint(alias, col, field), lookup_type, value))
         if negate:
             self.promote_alias_chain(join_list)
-…
+               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 = self.where_class()
+                            entry.add(
+                            self.add_where_leaf(
                                 (Constraint(alias, j_col, None), 'isnull', True),
                                 AND
+                                negated=True
+                            )
-                            entry.negate()
-                            self.where.add(entry, AND)
                             break
                 if not (lookup_type == 'in'
                             and not hasattr(value, 'as_sql')
-…
+               class Query(object):
                     # exclude the "foo__in=[]" case from this handling, because
                     # it's short-circuited in the Where class.
                     # We also need to handle the case where a subquery is provided
                     self.where.add((Constraint(alias, col, None), 'isnull', False), AND)
+                    self.add_where_leaf((Constraint(alias, col, None), 'isnull', False))
         if can_reuse is not None:
             can_reuse.update(join_list)
-…
+               class Query(object):
                 self.add_filter(filter, negate=negate, can_reuse=can_reuse,
                         process_extras=False)
     def add_q(self, q_object, used_aliases=None, force_having=False):
+    def add_q(self, q_object):
         """
         Adds a Q-object to the current filter.
         Can also be used to add anything that has an 'add_to_query()' method.
+        In case add_to_query path is not executed, this method's main purpose
+        is to walk the q_object's internal nodes and manage the state of the
+        self.where. Leaf nodes will be handled by add_filter.
+        The self.where tree is managed by pushing new nodes to the tree. This
+        way self.where is always at the right node when add_filter adds items
+        to it.
+        We need to start a new subtree when:
+           - The connector of the q_object is different than the connector of
+             the where tree.
+           - The q_object is negated.
+        After call of this function with q_object=~Q(pk=1)&~Q(Q(pk=3)|Q(pk=2))
+        we should have the following tree:
+                      AND
+                     /   \
+                    NOT  NOT
+                     |     \
+                    pk=1   OR
+                          /  \
+                        pk=3 pk=2
+        This method will call recursively itself for those childrens of the
+        q_object which are Q-objs, and call add_filter for the leaf nodes.
+        We will add all filters to self.where. When the query is executed, the
+        tree is splitted into where and having clauses.
         """
+        if used_aliases is None:
             used_aliases = self.used_aliases
+        # Complex custom objects are responsible for adding themselves.
         if hasattr(q_object, 'add_to_query'):
+            # Complex custom objects are responsible for adding themselves.
+            q_object.add_to_query(self, used_aliases)
+        else:
+            if self.where and q_object.connector != AND and len(q_object) > 1:
+                self.where.start_subtree(AND)
+                subtree = True
+            q_object.add_to_query(self, self.used_aliases)
+            return
+        # Start subtree if needed. At the end we check if anything got added
+        # into the subtrees. If not, prune em.
+        connector = q_object.connector
+        subtree_parent = None
+        if self.where.connector <> connector or q_object.negated:
+            subtree = self.where_class(connector=connector)
+            subtree_parent = self.where
+            self.where.add(subtree, self.where.connector)
+            self.where = subtree
+        if q_object.negated:
+            self.where.negate()
+        # Aliases that were newly added or not used at all need to
+        # be promoted to outer joins if they are nullable relations.
+        # (they shouldn't turn the whole conditional into the empty
+        # set just because they don't match anything). Take the
+        # before snapshot of the aliases.
+        if connector == OR:
+            refcounts_before = self.alias_refcount.copy()
+        for child in q_object.children:
+            if isinstance(child, Node):
+                self.add_q(child)
             else:
+                subtree = False
+            connector = AND
+            if q_object.connector == OR and not force_having:
+                force_having = self.need_force_having(q_object)
+            for child in q_object.children:
+                if connector == OR:
+                    refcounts_before = self.alias_refcount.copy()
+                if force_having:
+                    self.having.start_subtree(connector)
+                else:
+                    self.where.start_subtree(connector)
+                if isinstance(child, Node):
+                    self.add_q(child, used_aliases, force_having=force_having)
+                else:
+                    self.add_filter(child, connector, q_object.negated,
+                            can_reuse=used_aliases, force_having=force_having)
+                if force_having:
+                    self.having.end_subtree()
+                else:
+                    self.where.end_subtree()
+                if connector == OR:
+                    # Aliases that were newly added or not used at all need to
+                    # be promoted to outer joins if they are nullable relations.
+                    # (they shouldn't turn the whole conditional into the empty
+                    # set just because they don't match anything).
+                    self.promote_unused_aliases(refcounts_before, used_aliases)
+                connector = q_object.connector
+            if q_object.negated:
+                self.where.negate()
+            if subtree:
+                self.where.end_subtree()
+        if self.filter_is_sticky:
+            self.used_aliases = used_aliases
+                self.add_filter(child, connector, q_object.negated,
+                        can_reuse=self.used_aliases)
+        if connector == OR:
+            self.promote_unused_aliases(refcounts_before, self.used_aliases)
+        if subtree_parent:
+            self.where = subtree_parent
+        self.where.prune_tree()
     def setup_joins(self, names, opts, alias, dupe_multis, allow_many=True,
             allow_explicit_fk=False, can_reuse=None, negate=False,
-…
+               class Query(object):
         column (used for any 'where' constraint), the final 'opts' value and the
         list of tables joined.
         """
         joins = [alias]
         last = [0]
         dupe_set = set()
-…
+               class Query(object):
         # database from tripping over IN (...,NULL,...) selects and returning
         # nothing
         alias, col = query.select[0]
         query.where.add((Constraint(alias, col, None), 'isnull', False), AND)
+        query.add_where_leaf((Constraint(alias, col, None), 'isnull', False))
         self.add_filter(('%s__in' % prefix, query), negate=True, trim=True,
                 can_reuse=can_reuse)
-…
+               class Query(object):
         if force_empty:
             self.default_ordering = False
-    def set_group_by(self):
-        """
-        Expands the GROUP BY clause required by the query.
-        This will usually be the set of all non-aggregate fields in the
-        return data. If the database backend supports grouping by the
-        primary key, and the query would be equivalent, the optimization
-        will be made automatically.
-        """
-        self.group_by = []
-        for sel in self.select:
-            self.group_by.append(sel)
     def add_count_column(self):
         """
         Converts the query to do count(...) or count(distinct(pk)) in order to
-…
+               class Query(object):
         # Clear out the select cache to reflect the new unmasked aggregates.
         self.aggregates = {None: count}
         self.set_aggregate_mask(None)
         self.group_by = None
+        self.group_by = False
     def add_select_related(self, fields):
         """
-…
+               class Query(object):
             # This is order preserving, since self.extra_select is a SortedDict.
             self.extra.update(select_pairs)
         if where or params:
             self.where.add(ExtraWhere(where, params), AND)
+            self.add_where_leaf(ExtraWhere(where, params))
         if tables:
             self.extra_tables += tuple(tables)
         if order_by:
-…
+               class Query(object):
         target[model] = set([f.name for f in fields])
     def set_aggregate_mask(self, names):
+        if 'n_authors' in self.aggregate_select and names is None:
+            import ipdb; ipdb.set_trace()
         "Set the mask of aggregates that will actually be returned by the SELECT"
         if names is None:
             self.aggregate_select_mask = None

django/db/models/sql/subqueries.py

diff --git a/django/db/models/sql/subqueries.py b/django/db/models/sql/subqueries.py
index 1b03647..3a7774c 100644

                class DeleteQuery(Query):
             field = self.model._meta.pk
         for offset in range(0, len(pk_list), GET_ITERATOR_CHUNK_SIZE):
             where = self.where_class()
+            where.add((Constraint(None, field.column, field), 'in',
+                    pk_list[offset : offset + GET_ITERATOR_CHUNK_SIZE]), AND)
+            leaf = where.leaf_class()
+            where.add(leaf((Constraint(None, field.column, field), 'in',
+                    pk_list[offset : offset + GET_ITERATOR_CHUNK_SIZE])), AND)
             self.do_query(self.model._meta.db_table, where, using=using)
 class UpdateQuery(Query):
-…
+               class UpdateQuery(Query):
         self.add_update_values(values)
         for offset in range(0, len(pk_list), GET_ITERATOR_CHUNK_SIZE):
             self.where = self.where_class()
+            self.where.add((Constraint(None, pk_field.column, pk_field), 'in',
+                    pk_list[offset : offset + GET_ITERATOR_CHUNK_SIZE]),
+                    AND)
+            self.add_where_leaf((Constraint(None, pk_field.column, pk_field), 'in',
+                    pk_list[offset : offset + GET_ITERATOR_CHUNK_SIZE]))
             self.get_compiler(using).execute_sql(None)
     def add_update_values(self, values):

django/db/models/sql/where.py

diff --git a/django/db/models/sql/where.py b/django/db/models/sql/where.py
index 3e9dbf0..ea26f62 100644

                from itertools import repeat
 from django.utils import tree
 from django.db.models.fields import Field
 from datastructures import EmptyResultSet, FullResultSet
+from django.db.models.sql.aggregates import Aggregate
 # Connection types
 AND = 'AND'
 OR = 'OR'
 class EmptyShortCircuit(Exception):
+class WhereLeaf(object):
     """
+    Internal exception used to indicate that a "matches nothing" node should be
+    added to the where-clause.
+    """
+    pass
+    Represents a leaf node in a where tree. Contains single constraint,
+    and knows how to turn it into sql and params.
+class WhereNode(tree.Node):
+    This implements many of the WhereNode's methods. Here the methods
+    will do the terminal work, while WhereNode's methods will be mostly
+    recursive in nature.
     """
-    Used to represent the SQL where-clause.
-    The class is tied to the Query class that created it (in order to create
-    the correct SQL).
+    The children in this tree are usually either Q-like objects or lists of
+    [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
+    # Fast and pretty way to test if the node is a leaf node.
+    is_leaf = True
+    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 (obj, lookup_type, value), where obj is
+        a Constraint object, and is then slightly munged before being stored
+        (to avoid storing any reference to field objects). Otherwise, the 'data'
+        is stored unchanged and can be any class with an 'as_sql()' method.
+        """
+    def __init__(self, data, negated=False):
+        self.sql = ''
+        self.negated = negated
+        self.params = []
+        self.match_all = False
+        self.match_nothing = False
         if not isinstance(data, (list, tuple)):
+            super(WhereNode, self).add(data, connector)
+            return
+        obj, lookup_type, value = data
+        if hasattr(value, '__iter__') and hasattr(value, 'next'):
+            # Consume any generators immediately, so that we can determine
+            # emptiness and transform any non-empty values correctly.
+            value = list(value)
+        # The "annotation" parameter is used to pass auxilliary information
+        # about the value(s) to the query construction. Specifically, datetime
+        # and empty values need special handling. Other types could be used
+        # here in the future (using Python types is suggested for consistency).
+        if isinstance(value, datetime.datetime):
+            annotation = datetime.datetime
+        elif hasattr(value, 'value_annotation'):
+            annotation = value.value_annotation
+            self.data = data
         else:
+            annotation = bool(value)
+        if hasattr(obj, "prepare"):
+            value = obj.prepare(lookup_type, value)
+            super(WhereNode, self).add((obj, lookup_type, annotation, value),
+                connector)
+            return
+        super(WhereNode, self).add((obj, lookup_type, annotation, value),
+                connector)
+            # Preprocess the data
+            obj, lookup_type, value = data
+            if hasattr(value, '__iter__') and hasattr(value, 'next'):
+                # Consume any generators immediately, so that we can determine
+                # emptiness and transform any non-empty values correctly.
+                value = list(value)
+            # The "annotation" parameter is used to pass auxilliary information
+            # about the value(s) to the query construction. Specifically, datetime
+            # and empty values need special handling. Other types could be used
+            # here in the future (using Python types is suggested for consistency).
+            if isinstance(value, datetime.datetime):
+                annotation = datetime.datetime
+            elif hasattr(value, 'value_annotation'):
+                annotation = value.value_annotation
+            else:
+                annotation = bool(value)
+    def as_sql(self, qn, connection):
+        """
+        Returns the SQL version of the where clause and the value to be
+        substituted in. Returns None, None if this node is empty.
+            if hasattr(obj, "prepare"):
+                value = obj.prepare(lookup_type, value)
+            self.data = (obj, lookup_type, annotation, value)
+        If 'node' is provided, that is the root of the SQL generation
+        (generally not needed except by the internal implementation for
+        recursion).
+        """
+        if not self.children:
+            return None, []
+        result = []
+        result_params = []
+        empty = True
+        for child in self.children:
+            try:
+                if hasattr(child, 'as_sql'):
+                    sql, params = child.as_sql(qn=qn, connection=connection)
+                else:
+                    # A leaf node in the tree.
+                    sql, params = self.make_atom(child, qn, connection)
+            except EmptyResultSet:
+                if self.connector == AND and not self.negated:
+                    # We can bail out early in this particular case (only).
+                    raise
+                elif self.negated:
+                    empty = False
+                continue
+            except FullResultSet:
+                if self.connector == OR:
+                    if self.negated:
+                        empty = True
+                        break
+                    # We match everything. No need for any constraints.
+                    return '', []
+                if self.negated:
+                    empty = True
+                continue
+            empty = False
+            if sql:
+                result.append(sql)
+                result_params.extend(params)
+        if empty:
+            raise EmptyResultSet
+    def create_sql(self, qn, connection):
+        if hasattr(self.data, 'as_sql'):
+            self.sql, self.params = self.data.as_sql(qn, connection)
+        else:
+            self.sql, self.params = self.make_atom(qn, connection)
+        if self.negated and self.sql:
+            self.sql = 'NOT ' + self.sql
+        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 as_sql(self):
+        return self.sql, self.params
     def make_atom(self, child, qn, connection):
+    def make_atom(self, qn, connection):
         """
         Turn a tuple (table_alias, column_name, db_type, lookup_type,
         value_annot, params) into valid SQL.
-…
+               class WhereNode(tree.Node):
         Returns the string for the SQL fragment and the parameters to use for
         it.
         """
         lvalue, lookup_type, value_annot, params_or_value = child
+        lvalue, lookup_type, value_annot, params_or_value = self.data
         if hasattr(lvalue, 'process'):
+            from django.db.models.base import ObjectDoesNotExist
             try:
                 lvalue, params = lvalue.process(lookup_type, params_or_value, connection)
+            except EmptyShortCircuit:
+                raise EmptyResultSet
+            except ObjectDoesNotExist:
+                self.set_sql_matches_nothing()
+                return '', []
         else:
             params = Field().get_db_prep_lookup(lookup_type, params_or_value,
                 connection=connection, prepared=True)
-…
+               class WhereNode(tree.Node):
         if lookup_type == 'in':
             if not value_annot:
+                raise EmptyResultSet
+                self.set_sql_matches_nothing()
+                return '', []
             if extra:
                 return ('%s IN %s' % (field_sql, extra), params)
             max_in_list_size = connection.ops.max_in_list_size()
-…
+               class WhereNode(tree.Node):
             return connection.ops.regex_lookup(lookup_type) % (field_sql, cast_sql), params
         raise TypeError('Invalid lookup_type: %r' % lookup_type)
+    def set_sql_matches_nothing(self):
+        if self.negated:
+            self.match_everything = True
+        else:
+            self.match_nothing = True
+    def subtree_contains_aggregate(self):
+        """
+        The leaf node contains aggregate if it has an aggregate in it, or it
+        contains a subquery which contains an aggregate as a value.
+        """
+        return (isinstance(self.data[0], Aggregate) or
+                   (len(self.data) == 4 and
+                    hasattr(self.data[3], 'contains_aggregate') and
+                    self.data[3].contains_aggregate))
     def sql_for_columns(self, data, qn, connection):
         """
         Returns the SQL fragment used for the left-hand side of a column
-…
+               class WhereNode(tree.Node):
             lhs = qn(name)
         return connection.ops.field_cast_sql(db_type) % lhs
+    def relabel_aliases(self, change_map, node=None):
+    def relabel_aliases(self, change_map):
+        if hasattr(self.data, 'relabel_aliases'):
+            self.data.relabel_aliases(change_map)
+        elif isinstance(self.data[0], (list, tuple)):
+            elt = list(self.data[0])
+            if elt[0] in change_map:
+                elt[0] = change_map[elt[0]]
+                self.data = (tuple(elt),) + self.data[1:]
+        else:
+            self.data[0].relabel_aliases(change_map)
+            # Check if the query value also requires relabelling
+            if hasattr(self.data[3], 'relabel_aliases'):
+                self.data[3].relabel_aliases(change_map)
+    def get_group_by(self, group_by):
+        if isinstance(self.data, tuple) and not isinstance(self.data[0], Aggregate):
+            group_by.add((self.data[0].alias, self.data[0].col))
+    def clone(self):
         """
+        Relabels the alias values of any children. 'change_map' is a dictionary
+        mapping old (current) alias values to the new values.
+        TODO: It is unfortunate that the data can be all sorts of things. It
+        would be a good idea to make the Constraint a bit larger class, so
+        that it could hold also the lookup type and value. Then we would
+        always have something implementing similar interface in Data.
         """
+        if not node:
+            node = self
+        for pos, child in enumerate(node.children):
+            if hasattr(child, 'relabel_aliases'):
+                child.relabel_aliases(change_map)
+            elif isinstance(child, tree.Node):
+                self.relabel_aliases(change_map, child)
+            elif isinstance(child, (list, tuple)):
+                if isinstance(child[0], (list, tuple)):
+                    elt = list(child[0])
+                    if elt[0] in change_map:
+                        elt[0] = change_map[elt[0]]
+                        node.children[pos] = (tuple(elt),) + child[1:]
+                else:
+                    child[0].relabel_aliases(change_map)
+        clone = self.__class__(None, self.negated)
+        if hasattr(self.data, 'clone'):
+            clone.data = self.data.clone()
+        else:
+            if hasattr(self.data[3], 'clone'):
+                new_data3 = self.data[3].clone()
+            else:
+                new_data3 = self.data[3]
+            clone.data = (self.data[0].clone(), self.data[1], self.data[2], new_data3)
+        return clone
+    def negate(self):
+        self.negated = not self.negated
                 # Check if the query value also requires relabelling
                 if hasattr(child[3], 'relabel_aliases'):
                     child[3].relabel_aliases(change_map)
+    def __str__(self):
+        return "%s%s, %s, %s" % (self.negated and 'NOT: ' or '',
+                                 self.data[0], self.data[1], self.data[3])
 class EverythingNode(object):
+class WhereNode(tree.Node):
     """
+    A node that matches everything.
+    Used to represent the SQL where-clause.
+    The class is tied to the Query class that created it (in order to create
+    the correct SQL).
+    The children in this tree are usually either Q-like objects or lists of
+    [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.
     """
     def as_sql(self, qn=None, connection=None):
         raise FullResultSet
+    default = AND
+    is_leaf = False
+    def relabel_aliases(self, change_map, node=None):
+        return
+    def leaf_class(cls):
+        # Subclass hook
+        return WhereLeaf
+    leaf_class = classmethod(leaf_class)
+class NothingNode(object):
+    """
+    A node that matches nothing.
+    """
+    def as_sql(self, qn=None, connection=None):
+        raise EmptyResultSet
+    def clone_internal(self):
+        clone = self._new_instance()
+        clone.negated = self.negated; clone.connector = self.connector
+        clone.children = [c.is_leaf and c or c.clone() for c in self.children]
+        return clone
+    def final_prune(self, qn, connection):
+        """
+        This will do the final pruning of the tree, that is, removing parts
+        of the tree that must match everything / nothing.
+        Due to the fact that the only way to get to know that is calling
+        as_sql(), we will at the same time turn the leaf nodes into sql.
+        """
+        # There variables make sense only in the context of the final prune.
+        # There is no need to clone them, and there is no need to have them
+        # elsewhere. So, define them here instead of __init__.
+        self.match_all = False
+        self.match_nothing = False
+        for child in self.children[:]:
+            if child.is_leaf:
+                child.create_sql(qn, connection)
+            else:
+                child.final_prune(qn, connection)
+            if child.match_all:
+                 if self.connector == OR:
+                     self.match_all = True
+                     break
+                 self.children.remove(child)
+            if child.match_nothing:
+                 if self.connector == AND:
+                     self.match_nothing = True
+                     break
+                 self.children.remove(child)
+        else:
+            # We got through the loop without a break. Check if there are any
+            # children left. If not, this node must be a match_all node.
+            if not self.children:
+                self.match_all = True
+        if self.negated:
+            # If the node is negated, then turn the tables around.
+            self.match_all, self.match_nothing = self.match_nothing, self.match_all
+    def split_aggregates(self, having, parent=None):
+        """
+        Remove those parts of self that must go into the having clause. Part
+        must go into having if:
+          - It is connected to parent with OR and the subtree contains
+            aggregate
+          - The node is a leaf node and it contains aggregate
+        """
+        from django.conf import settings
+        if self.connector == OR:
+            if self.subtree_contains_aggregate():
+                having.add(self, AND)
+                # Note that OR cannot be the highest node in the tree, a where
+                # tree must always contain AND as root, and as such parent
+                # can't be None here.
+                parent.children.remove(self)
+        else:
+            if self.negated:
+                # TODO: I believe this might be broken. If in fact it isn't,
+                # we need a comment why it isn't so.
+                neg_node = having._new_instance(negated=True)
+                having.add(neg_node, AND)
+                having = neg_node
+            for child in self.children[:]:
+                if child.is_leaf:
+                    if child.subtree_contains_aggregate():
+                        having.add(child, AND)
+                        self.children.remove(child)
+                else:
+                    child.split_aggregates(having, self)
+    def subtree_contains_aggregate(self):
+        """
+        Returns whether or not all elements of this q_object need to be put
+        together in the HAVING clause.
+        """
+        for child in self.children:
+            if child.subtree_contains_aggregate():
+                 return True
+        return False
+    def as_sql(self):
+        """
+        Turns this tree into SQL and params. It is assumed that leaf nodes are already
+        TODO: rename, and have as_sql implement the normal as_sql(qn, connection)
+        interface.
+        """
+        if not self:
+            return '', []
+        sql_snippets, params = [], []
+        for child in self.children:
+            child_sql, child_params = child.as_sql()
+            sql_snippets.append(child_sql); params.extend(child_params)
+        conn = ' %s ' % self.connector
+        sql_string = conn.join(sql_snippets)
+        if self.negated and sql_string:
+            sql_string = 'NOT (%s)' % sql_string
+        elif len(self.children) != 1:
+            sql_string = '(%s)' % sql_string
+        return sql_string, params
+    def get_group_by(self, group_by):
+        for child in self.children:
+            child.get_group_by(group_by)
     def relabel_aliases(self, change_map, node=None):
+        return
+        """
+        Relabels the alias values of any children. 'change_map' is a dictionary
+        mapping old (current) alias values to the new values.
+        """
+        for child in self.children:
+            child.relabel_aliases(change_map)
 class ExtraWhere(object):
     def __init__(self, sqls, params):
         self.sqls = sqls
         self.params = params
+    def relabel_aliases(self, change_map):
+        return
     def as_sql(self, qn=None, connection=None):
         return " AND ".join(self.sqls), tuple(self.params or ())
+    def clone(self):
+        return self
 class Constraint(object):
     """
     An object that can be passed to WhereNode.add() and knows how to
     pre-process itself prior to including in the WhereNode.
     """
     def __init__(self, alias, col, field):
         self.alias, self.col, self.field = alias, col, field
-…
+               class Constraint(object):
     def process(self, lookup_type, value, connection):
         """
         Returns a tuple of data suitable for inclusion in a WhereNode
         instance.
+        instance. Can raise ObjectDoesNotExist
         """
+        # Because of circular imports, we need to import this here.
+        from django.db.models.base import ObjectDoesNotExist
+        try:
+            if self.field:
+                params = self.field.get_db_prep_lookup(lookup_type, value,
+                    connection=connection, prepared=True)
+                db_type = self.field.db_type(connection=connection)
+            else:
+                # This branch is used at times when we add a comparison to NULL
+                # (we don't really want to waste time looking up the associated
+                # field object at the calling location).
+                params = Field().get_db_prep_lookup(lookup_type, value,
+                    connection=connection, prepared=True)
+                db_type = None
+        except ObjectDoesNotExist:
+            raise EmptyShortCircuit
+        if self.field:
+            params = self.field.get_db_prep_lookup(lookup_type, value,
+                connection=connection, prepared=True)
+            db_type = self.field.db_type(connection=connection)
+        else:
+            # This branch is used at times when we add a comparison to NULL
+            # (we don't really want to waste time looking up the associated
+            # field object at the calling location).
+            params = Field().get_db_prep_lookup(lookup_type, value,
+                connection=connection, prepared=True)
+            db_type = None
         return (self.alias, self.col, db_type), params
     def relabel_aliases(self, change_map):
         if self.alias in change_map:
             self.alias = change_map[self.alias]
+    def clone(self):
+        return Constraint(self.alias, self.col, self.field)
+    def __str__(self):
+        return "%s.%s" % (self.alias, self.col)

django/utils/tree.py

diff --git a/django/utils/tree.py b/django/utils/tree.py
index 36b5977..f733d1b 100644

                class Node(object):
         """
         Constructs a new Node. If no connector is given, the default will be
         used.
-        Warning: You probably don't want to pass in the 'negated' parameter. It
-        is NOT the same as constructing a node and calling negate() on the
-        result.
         """
         self.children = children and children[:] or []
         self.connector = connector or self.default
         self.subtree_parents = []
+        self.parent = None
         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.
+    # The __init__ of Q has different signature, and thus _new_instance of Q
+    # does call Q's version of __init__.
     def _new_instance(cls, children=None, connector=None, negated=False):
+        return cls(children, connector, negated)
+    _new_instance = classmethod(_new_instance)
+    def clone(self):
         """
+        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).
+        Clones the internal nodes of the tree. If also_leafs is False, does
+        not copy leaf nodes. This is a useful optimization for WhereNode
+        because WhereLeaf nodes do not need copying except when relabel_aliases
+        is called.
         """
+        obj = Node(children, connector, negated)
+        obj.__class__ = cls
+        obj = self._new_instance()
+        obj.children = [
+            isinstance(c, tuple) and c or c.clone() for c in self.children
+         ]
+        obj.connector = self.connector
+        obj.negated = self.negated
         return obj
+    _new_instance = classmethod(_new_instance)
+    def __repr__(self):
+        return self.as_subtree
     def __str__(self):
         if self.negated:
-…
+               class Node(object):
         return '(%s: %s)' % (self.connector, ', '.join([str(c) for c in
                 self.children]))
+    def __deepcopy__(self, memodict):
+        """
+        Utility method used by copy.deepcopy().
+        """
+        obj = Node(connector=self.connector, negated=self.negated)
+        obj.__class__ = self.__class__
+        obj.children = copy.deepcopy(self.children, memodict)
+        obj.subtree_parents = copy.deepcopy(self.subtree_parents, memodict)
+        return obj
+    def _as_subtree(self, indent=0):
+        buf = []
+        if self.negated:
+            buf.append(" " * indent + "NOT")
+        buf.append((" " * indent) + self.connector + ":")
+        indent += 2
+        for child in self.children:
+            if isinstance(child, Node):
+                buf.append(child._as_subtree(indent=indent))
+            else:
+                buf.append((" " * indent) + str(child))
+        return "\n".join(buf)
+    as_subtree = property(_as_subtree)
     def __len__(self):
         """
-…
+               class Node(object):
     def add(self, node, conn_type):
         """
         Adds a new node to the tree. If the conn_type is the same as the root's
         current connector type, the node is added to the first level.
+        Adds a new node to the tree. If the conn_type is the same as the
+        root's current connector type, the node is added to the first level.
         Otherwise, the whole tree is pushed down one level and a new root
         connector is created, connecting the existing tree and the new node.
+        connector is created, connecting the existing tree and the added node.
         """
         if node in self.children and conn_type == self.connector:
             return
-        if len(self.children) < 2:
-            self.connector = conn_type
         if self.connector == conn_type:
+            if isinstance(node, Node) and (node.connector == conn_type or
+                    len(node) == 1):
+                self.children.extend(node.children)
+            else:
+                self.children.append(node)
+            self.children.append(node)
         else:
+            obj = self._new_instance(self.children, self.connector,
+                    self.negated)
+            self.connector = conn_type
+            self.children = [obj, node]
+            obj = self._new_instance([node], conn_type)
+            self.children.append(obj)
     def negate(self):
         """
+        Negate the sense of the root connector. This reorganises the children
+        so that the current node has a single child: a negated node containing
+        all the previous children. This slightly odd construction makes adding
+        new children behave more intuitively.
+        Interpreting the meaning of this negate is up to client code. This
+        method is useful for implementing "not" arrangements.
+        """
+        self.children = [self._new_instance(self.children, self.connector,
+                not self.negated)]
+        self.connector = self.default
+    def start_subtree(self, conn_type):
+        """
+        Sets up internal state so that new nodes are added to a subtree of the
+        current node. The conn_type specifies how the sub-tree is joined to the
+        existing children.
+        """
+        if len(self.children) == 1:
+            self.connector = conn_type
+        elif self.connector != conn_type:
+            self.children = [self._new_instance(self.children, self.connector,
+                    self.negated)]
+            self.connector = conn_type
+            self.negated = False
+        self.subtree_parents.append(self.__class__(self.children,
+                self.connector, self.negated))
+        self.connector = self.default
+        self.negated = False
+        self.children = []
+    def end_subtree(self):
+        """
+        Closes off the most recently unmatched start_subtree() call.
+        This puts the current state into a node of the parent tree and returns
+        the current instances state to be the parent.
+        """
+        obj = self.subtree_parents.pop()
+        node = self.__class__(self.children, self.connector)
+        self.connector = obj.connector
+        self.negated = obj.negated
+        self.children = obj.children
+        self.children.append(node)
+        Negate the sense of this node.
+        """
+        self.negated = not self.negated
+    def prune_tree(self):
+        """
+        Removes empty children nodes, and non-necessary intermediatry
+        nodes from this node.
+        """
+        for child in self.children[:]:
+            if not child:
+                self.children.remove(child)
+            elif not child.is_leaf:
+                child.prune_tree()
+                if len(child) == 1:
+                    # There is no need for this node.we can prune internal
+                    # nodes with just on child
+                    grandchild = child.children[0]
+                    if child.negated:
+                        grandchild.negate()
+                    self.children.remove(child)
+                    self.children.append(grandchild)
+                elif not child:
+                    self.children.remove(child)

tests/regressiontests/aggregation_regress/tests.py

diff --git a/tests/regressiontests/aggregation_regress/tests.py b/tests/regressiontests/aggregation_regress/tests.py
index acdc59a..badc1cb 100644

                class AggregationTests(TestCase):
         # Regression for #15709 - Ensure each group_by field only exists once
         # per query
         qs = Book.objects.values('publisher').annotate(max_pages=Max('pages')).order_by()
         grouping, gb_params = qs.query.get_compiler(qs.db).get_grouping()
+        grouping, gb_params = qs.query.get_compiler(qs.db).get_grouping(set())
         self.assertEqual(len(grouping), 1)
     def test_duplicate_alias(self):

tests/regressiontests/queries/tests.py

diff --git a/tests/regressiontests/queries/tests.py b/tests/regressiontests/queries/tests.py
index d8fd5bc..4f505a3 100644

                class Queries1Tests(BaseQuerysetTest):
         q = Note.objects.filter(Q(extrainfo__author=self.a1)|Q(extrainfo=xx)).query
         self.assertEqual(
             len([x[2] for x in q.alias_map.values() if x[2] == q.LOUTER and q.alias_refcount[x[1]]]),
+        )

Download in other formats:

Original Format

Issues

Context Navigation

Ticket #17025: wherenode_refactor.diff

django/db/models/query.py

django/db/models/query_utils.py

django/db/models/sql/aggregates.py

django/db/models/sql/compiler.py

django/db/models/sql/datastructures.py

django/db/models/sql/query.py

django/db/models/sql/subqueries.py

django/db/models/sql/where.py

django/utils/tree.py

tests/regressiontests/aggregation_regress/tests.py

tests/regressiontests/queries/tests.py

Download in other formats:

Django Links

Learn More

Get Involved

Get Help

Follow Us

Support Us