Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

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
  • Loading branch information...
commit 6dd2b5468fa275d53aa60fdcaff8c28bdc5e9c25 1 parent 3b64871
Malcolm Tredinnick authored July 04, 2008
21  django/db/models/sql/query.py
@@ -1104,19 +1104,7 @@ def add_filter(self, filter_expr, connector=AND, negate=False, trim=False,
1104 1104
                 # that's harmless.
1105 1105
                 self.promote_alias(table)
1106 1106
 
1107  
-        # To save memory and copying time, convert the value from the Python
1108  
-        # object to the actual value used in the SQL query.
1109  
-        if field:
1110  
-            params = field.get_db_prep_lookup(lookup_type, value)
1111  
-        else:
1112  
-            params = Field().get_db_prep_lookup(lookup_type, value)
1113  
-        if isinstance(value, datetime.datetime):
1114  
-            annotation = datetime.datetime
1115  
-        else:
1116  
-            annotation = bool(value)
1117  
-
1118  
-        self.where.add((alias, col, field.db_type(), lookup_type, annotation,
1119  
-            params), connector)
  1107
+        self.where.add((alias, col, field, lookup_type, value), connector)
1120 1108
 
1121 1109
         if negate:
1122 1110
             for alias in join_list:
@@ -1126,8 +1114,8 @@ def add_filter(self, filter_expr, connector=AND, negate=False, trim=False,
1126 1114
                     for alias in join_list:
1127 1115
                         if self.alias_map[alias][JOIN_TYPE] == self.LOUTER:
1128 1116
                             j_col = self.alias_map[alias][RHS_JOIN_COL]
1129  
-                            entry = Node([(alias, j_col, None, 'isnull', True,
1130  
-                                    [True])])
  1117
+                            entry = self.where_class()
  1118
+                            entry.add((alias, j_col, None, 'isnull', True), AND)
1131 1119
                             entry.negate()
1132 1120
                             self.where.add(entry, AND)
1133 1121
                             break
@@ -1135,7 +1123,8 @@ def add_filter(self, filter_expr, connector=AND, negate=False, trim=False,
1135 1123
                     # Leaky abstraction artifact: We have to specifically
1136 1124
                     # exclude the "foo__in=[]" case from this handling, because
1137 1125
                     # it's short-circuited in the Where class.
1138  
-                    entry = Node([(alias, col, None, 'isnull', True, [True])])
  1126
+                    entry = self.where_class()
  1127
+                    entry.add((alias, col, None, 'isnull', True), AND)
1139 1128
                     entry.negate()
1140 1129
                     self.where.add(entry, AND)
1141 1130
 
12  django/db/models/sql/subqueries.py
@@ -49,7 +49,7 @@ def delete_batch_related(self, pk_list):
49 49
                 for offset in range(0, len(pk_list), GET_ITERATOR_CHUNK_SIZE):
50 50
                     where = self.where_class()
51 51
                     where.add((None, related.field.m2m_reverse_name(),
52  
-                            related.field.db_type(), 'in', True,
  52
+                            related.field, 'in',
53 53
                             pk_list[offset : offset+GET_ITERATOR_CHUNK_SIZE]),
54 54
                             AND)
55 55
                     self.do_query(related.field.m2m_db_table(), where)
@@ -59,11 +59,11 @@ def delete_batch_related(self, pk_list):
59 59
             if isinstance(f, generic.GenericRelation):
60 60
                 from django.contrib.contenttypes.models import ContentType
61 61
                 field = f.rel.to._meta.get_field(f.content_type_field_name)
62  
-                w1.add((None, field.column, field.db_type(), 'exact', True,
63  
-                        [ContentType.objects.get_for_model(cls).id]), AND)
  62
+                w1.add((None, field.column, field, 'exact',
  63
+                        ContentType.objects.get_for_model(cls).id), AND)
64 64
             for offset in range(0, len(pk_list), GET_ITERATOR_CHUNK_SIZE):
65 65
                 where = self.where_class()
66  
-                where.add((None, f.m2m_column_name(), f.db_type(), 'in', True,
  66
+                where.add((None, f.m2m_column_name(), f, 'in',
67 67
                         pk_list[offset : offset + GET_ITERATOR_CHUNK_SIZE]),
68 68
                         AND)
69 69
                 if w1:
@@ -81,7 +81,7 @@ def delete_batch(self, pk_list):
81 81
         for offset in range(0, len(pk_list), GET_ITERATOR_CHUNK_SIZE):
82 82
             where = self.where_class()
83 83
             field = self.model._meta.pk
84  
-            where.add((None, field.column, field.db_type(), 'in', True,
  84
+            where.add((None, field.column, field, 'in',
85 85
                     pk_list[offset : offset + GET_ITERATOR_CHUNK_SIZE]), AND)
86 86
             self.do_query(self.model._meta.db_table, where)
87 87
 
@@ -204,7 +204,7 @@ def clear_related(self, related_field, pk_list):
204 204
         for offset in range(0, len(pk_list), GET_ITERATOR_CHUNK_SIZE):
205 205
             self.where = self.where_class()
206 206
             f = self.model._meta.pk
207  
-            self.where.add((None, f.column, f.db_type(), 'in', True,
  207
+            self.where.add((None, f.column, f, 'in',
208 208
                     pk_list[offset : offset + GET_ITERATOR_CHUNK_SIZE]),
209 209
                     AND)
210 210
             self.values = [(related_field.column, None, '%s')]
71  django/db/models/sql/where.py
@@ -27,7 +27,36 @@ class WhereNode(tree.Node):
27 27
     """
28 28
     default = AND
29 29
 
30  
-    def as_sql(self, node=None, qn=None):
  30
+    def add(self, data, connector):
  31
+        """
  32
+        Add a node to the where-tree. If the data is a list or tuple, it is
  33
+        expected to be of the form (alias, col_name, field_obj, lookup_type,
  34
+        value), which is then slightly munged before being stored (to avoid
  35
+        storing any reference to field objects). Otherwise, the 'data' is
  36
+        stored unchanged and can be anything with an 'as_sql()' method.
  37
+        """
  38
+        if not isinstance(data, (list, tuple)):
  39
+            super(WhereNode, self).add(data, connector)
  40
+            return
  41
+
  42
+        alias, col, field, lookup_type, value = data
  43
+        if field:
  44
+            params = field.get_db_prep_lookup(lookup_type, value)
  45
+            db_type = field.db_type()
  46
+        else:
  47
+            # This is possible when we add a comparison to NULL sometimes (we
  48
+            # don't really need to waste time looking up the associated field
  49
+            # object).
  50
+            params = Field().get_db_prep_lookup(lookup_type, value)
  51
+            db_type = None
  52
+        if isinstance(value, datetime.datetime):
  53
+            annotation = datetime.datetime
  54
+        else:
  55
+            annotation = bool(value)
  56
+        super(WhereNode, self).add((alias, col, db_type, lookup_type,
  57
+                annotation, params), connector)
  58
+
  59
+    def as_sql(self, qn=None):
31 60
         """
32 61
         Returns the SQL version of the where clause and the value to be
33 62
         substituted in. Returns None, None if this node is empty.
@@ -36,60 +65,56 @@ def as_sql(self, node=None, qn=None):
36 65
         (generally not needed except by the internal implementation for
37 66
         recursion).
38 67
         """
39  
-        if node is None:
40  
-            node = self
41 68
         if not qn:
42 69
             qn = connection.ops.quote_name
43  
-        if not node.children:
  70
+        if not self.children:
44 71
             return None, []
45 72
         result = []
46 73
         result_params = []
47 74
         empty = True
48  
-        for child in node.children:
  75
+        for child in self.children:
49 76
             try:
50 77
                 if hasattr(child, 'as_sql'):
51 78
                     sql, params = child.as_sql(qn=qn)
52  
-                    format = '(%s)'
53  
-                elif isinstance(child, tree.Node):
54  
-                    sql, params = self.as_sql(child, qn)
55  
-                    if child.negated:
56  
-                        format = 'NOT (%s)'
57  
-                    elif len(child.children) == 1:
58  
-                        format = '%s'
59  
-                    else:
60  
-                        format = '(%s)'
61 79
                 else:
  80
+                    # A leaf node in the tree.
62 81
                     sql, params = self.make_atom(child, qn)
63  
-                    format = '%s'
64 82
             except EmptyResultSet:
65  
-                if node.connector == AND and not node.negated:
  83
+                if self.connector == AND and not self.negated:
66 84
                     # We can bail out early in this particular case (only).
67 85
                     raise
68  
-                elif node.negated:
  86
+                elif self.negated:
69 87
                     empty = False
70 88
                 continue
71 89
             except FullResultSet:
72 90
                 if self.connector == OR:
73  
-                    if node.negated:
  91
+                    if self.negated:
74 92
                         empty = True
75 93
                         break
76 94
                     # We match everything. No need for any constraints.
77 95
                     return '', []
78  
-                if node.negated:
  96
+                if self.negated:
79 97
                     empty = True
80 98
                 continue
81 99
             empty = False
82 100
             if sql:
83  
-                result.append(format % sql)
  101
+                result.append(sql)
84 102
                 result_params.extend(params)
85 103
         if empty:
86 104
             raise EmptyResultSet
87  
-        conn = ' %s ' % node.connector
88  
-        return conn.join(result), result_params
  105
+
  106
+        conn = ' %s ' % self.connector
  107
+        sql_string = conn.join(result)
  108
+        if sql_string:
  109
+            if self.negated:
  110
+                sql_string = 'NOT (%s)' % sql_string
  111
+            elif len(self.children) != 1:
  112
+                sql_string = '(%s)' % sql_string
  113
+        return sql_string, result_params
89 114
 
90 115
     def make_atom(self, child, qn):
91 116
         """
92  
-        Turn a tuple (table_alias, field_name, db_type, lookup_type,
  117
+        Turn a tuple (table_alias, column_name, db_type, lookup_type,
93 118
         value_annot, params) into valid SQL.
94 119
 
95 120
         Returns the string for the SQL fragment and the parameters to use for
31  django/utils/tree.py
@@ -29,6 +29,22 @@ def __init__(self, children=None, connector=None, negated=False):
29 29
         self.subtree_parents = []
30 30
         self.negated = negated
31 31
 
  32
+    # We need this because of django.db.models.query_utils.Q. Q. __init__() is
  33
+    # problematic, but it is a natural Node subclass in all other respects.
  34
+    def _new_instance(cls, children=None, connector=None, negated=False):
  35
+        """
  36
+        This is called to create a new instance of this class when we need new
  37
+        Nodes (or subclasses) in the internal code in this class. Normally, it
  38
+        just shadows __init__(). However, subclasses with an __init__ signature
  39
+        that is not an extension of Node.__init__ might need to implement this
  40
+        method to allow a Node to create a new instance of them (if they have
  41
+        any extra setting up to do).
  42
+        """
  43
+        obj = Node(children, connector, negated)
  44
+        obj.__class__ = cls
  45
+        return obj
  46
+    _new_instance = classmethod(_new_instance)
  47
+
32 48
     def __str__(self):
33 49
         if self.negated:
34 50
             return '(NOT (%s: %s))' % (self.connector, ', '.join([str(c) for c
@@ -82,7 +98,8 @@ def add(self, node, conn_type):
82 98
             else:
83 99
                 self.children.append(node)
84 100
         else:
85  
-            obj = Node(self.children, self.connector, self.negated)
  101
+            obj = self._new_instance(self.children, self.connector,
  102
+                    self.negated)
86 103
             self.connector = conn_type
87 104
             self.children = [obj, node]
88 105
 
@@ -96,7 +113,8 @@ def negate(self):
96 113
         Interpreting the meaning of this negate is up to client code. This
97 114
         method is useful for implementing "not" arrangements.
98 115
         """
99  
-        self.children = [Node(self.children, self.connector, not self.negated)]
  116
+        self.children = [self._new_instance(self.children, self.connector,
  117
+                not self.negated)]
100 118
         self.connector = self.default
101 119
 
102 120
     def start_subtree(self, conn_type):
@@ -108,12 +126,13 @@ def start_subtree(self, conn_type):
108 126
         if len(self.children) == 1:
109 127
             self.connector = conn_type
110 128
         elif self.connector != conn_type:
111  
-            self.children = [Node(self.children, self.connector, self.negated)]
  129
+            self.children = [self._new_instance(self.children, self.connector,
  130
+                    self.negated)]
112 131
             self.connector = conn_type
113 132
             self.negated = False
114 133
 
115  
-        self.subtree_parents.append(Node(self.children, self.connector,
116  
-                self.negated))
  134
+        self.subtree_parents.append(self.__class__(self.children,
  135
+                self.connector, self.negated))
117 136
         self.connector = self.default
118 137
         self.negated = False
119 138
         self.children = []
@@ -126,7 +145,7 @@ def end_subtree(self):
126 145
         the current instances state to be the parent.
127 146
         """
128 147
         obj = self.subtree_parents.pop()
129  
-        node = Node(self.children, self.connector)
  148
+        node = self.__class__(self.children, self.connector)
130 149
         self.connector = obj.connector
131 150
         self.negated = obj.negated
132 151
         self.children = obj.children

0 notes on commit 6dd2b54

Please sign in to comment.
Something went wrong with that request. Please try again.