queryset-refactor: Fixed a few really silly errors in the Q class and negation

handling in the tree class. Discovered whilst starting to fix exclude().


git-svn-id: http://code.djangoproject.com/svn/django/branches/queryset-refactor@7169 bcc190cf-cafb-0310-a4f2-bffc1f526a37

django/db/models/query.py

@@ -2,7 +2,7 @@ import warnings
 
 from django.db import connection, transaction
 from django.db.models.fields import DateField, FieldDoesNotExist
-from django.db.models.query_utils import Q, QNot, EmptyResultSet
+from django.db.models.query_utils import Q, EmptyResultSet, not_q
 from django.db.models import signals, sql
 from django.dispatch import dispatcher
 from django.utils.datastructures import SortedDict
@@ -316,7 +316,7 @@ class _QuerySet(object):
         Returns a new QuerySet instance with NOT (args) ANDed to the existing
         set.
         """
-        return self._filter_or_exclude(QNot, *args, **kwargs)
+        return self._filter_or_exclude(not_q, *args, **kwargs)
 
     def _filter_or_exclude(self, mapper, *args, **kwargs):
         # mapper is a callable used to transform Q objects,
@@ -591,15 +591,20 @@ class EmptyQuerySet(QuerySet):
         # (it raises StopIteration immediately).
         yield iter([]).next()
 
-# QOperator, QAnd and QOr are temporarily retained for backwards compatibility.
+# QOperator, QNot, QAnd and QOr are temporarily retained for backwards
-# All the old functionality is now part of the 'Q' class.
+# compatibility. All the old functionality is now part of the 'Q' class.
 class QOperator(Q):
     def __init__(self, *args, **kwargs):
         warnings.warn('Use Q instead of QOr, QAnd or QOperation.',
                 DeprecationWarning, stacklevel=2)
+        super(QOperator, self).__init__(*args, **kwargs)
 
 QOr = QAnd = QOperator
 
+def QNot(q):
+    warnings.warn('Use ~q instead of QNot(q)', DeprecationWarning, stacklevel=2)
+    return ~q
+
 def get_cached_row(klass, row, index_start, max_depth=0, cur_depth=0,
         requested=None):
     """Helper function that recursively returns an object with cache filled"""

django/db/models/query_utils.py

@@ -4,6 +4,9 @@ Various data structures used in query construction.
 Factored out from django.db.models.query so that they can also be used by other
 modules without getting into circular import difficulties.
 """
+
+from copy import deepcopy
+
 from django.utils import tree
 
 class EmptyResultSet(Exception):
@@ -31,8 +34,9 @@ class Q(tree.Node):
     def _combine(self, other, conn):
         if not isinstance(other, Q):
             raise TypeError(other)
-        self.add(other, conn)
+        obj = deepcopy(self)
-        return self
+        obj.add(other, conn)
+        return obj
 
     def __or__(self, other):
         return self._combine(other, self.OR)
@@ -41,18 +45,10 @@ class Q(tree.Node):
         return self._combine(other, self.AND)
 
     def __invert__(self):
-        return QNot(self)
+        obj = deepcopy(self)
+        obj.negate()
+        return obj
 
-class QNot(Q):
+def not_q(q):
-    """
+    return ~q
-    Encapsulates the negation of a Q object.
-    """
-    def __init__(self, q):
-        """Creates the negation of the Q object passed in."""
-        super(QNot, self).__init__()
-        self.add(q, self.AND)
-        self.negate()
-
-    def __invert__(self):
-        return self.children[0]
 

django/db/models/sql/where.py

@@ -49,10 +49,12 @@ class WhereNode(tree.Node):
                     format = '(%s)'
                 elif isinstance(child, tree.Node):
                     sql, params = self.as_sql(child, qn)
-                    if child.negated:
+                    if len(child.children) == 1:
-                        format = 'NOT (%s)'
+                        format = '%s'
                     else:
                         format = '(%s)'
+                    if child.negated:
+                        format = 'NOT %s' % format
                 else:
                     sql, params = self.make_atom(child, qn)
                     format = '%s'

django/utils/tree.py

@@ -3,7 +3,7 @@ A class for storing a tree graph. Primarily used for filter constructs in the
 ORM.
 """
 
-import copy
+from copy import deepcopy
 
 class Node(object):
     """
@@ -15,13 +15,16 @@ class Node(object):
     # subclasses will usually override the value.
     default = 'DEFAULT'
 
-    def __init__(self, children=None, connector=None):
+    def __init__(self, children=None, connector=None, negated=False):
         self.children = children and children[:] or []
         self.connector = connector or self.default
         self.subtree_parents = []
-        self.negated = False
+        self.negated = negated
 
     def __str__(self):
+        if self.negated:
+            return '(NOT (%s: %s))' % (self.connector, ', '.join([str(c) for c
+                    in self.children]))
         return '(%s: %s)' % (self.connector, ', '.join([str(c) for c in
                 self.children]))
 
@@ -29,10 +32,10 @@ class Node(object):
         """
         Utility method used by copy.deepcopy().
         """
-        obj = self.__class__(connector=self.connector)
+        obj = Node(connector=self.connector, negated=self.negated)
-        obj.children = copy.deepcopy(self.children, memodict)
+        obj.__class__ = self.__class__
-        obj.subtree_parents = copy.deepcopy(self.subtree_parents, memodict)
+        obj.children = deepcopy(self.children, memodict)
-        obj.negated = self.negated
+        obj.subtree_parents = deepcopy(self.subtree_parents, memodict)
         return obj
 
     def __len__(self):
@@ -63,13 +66,13 @@ class Node(object):
         if len(self.children) < 2:
             self.connector = conn_type
         if self.connector == conn_type:
-            if isinstance(node, Node) and (node.connector == conn_type
+            if isinstance(node, Node) and (node.connector == conn_type or
-                    or len(node) == 1):
+                    len(node) == 1):
                 self.children.extend(node.children)
             else:
                 self.children.append(node)
         else:
-            obj = Node(self.children, self.connector)
+            obj = Node(self.children, self.connector, self.negated)
             self.connector = conn_type
             self.children = [obj, node]
 
@@ -80,8 +83,7 @@ class Node(object):
         Interpreting the meaning of this negate is up to client code. This
         method is useful for implementing "not" arrangements.
         """
-        self.children = [NegatedNode(self.children, self.connector,
+        self.children = [Node(self.children, self.connector, not self.negated)]
-                old_state=self.negated)]
         self.connector = self.default
 
     def start_subtree(self, conn_type):
@@ -93,11 +95,14 @@ 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.children = [Node(self.children, self.connector, self.negated)]
             self.connector = conn_type
+            self.negated = False
 
-        self.subtree_parents.append(Node(self.children, self.connector))
+        self.subtree_parents.append(Node(self.children, self.connector,
+                self.negated))
         self.connector = self.default
+        self.negated = False
         self.children = []
 
     def end_subtree(self):
@@ -110,15 +115,7 @@ class Node(object):
         obj = self.subtree_parents.pop()
         node = Node(self.children, self.connector)
         self.connector = obj.connector
+        self.negated = obj.negated
         self.children = obj.children
         self.children.append(node)
 
-class NegatedNode(Node):
-    """
-    A class that indicates the connector type should be negated (whatever that
-    means -- it's up to the client) when used by the client code.
-    """
-    def __init__(self, children=None, connector=None, old_state=True):
-        super(NegatedNode, self).__init__(children, connector)
-        self.negated = not old_state
-