From 7145c8a62a38c3b30641bb84a51f7567ce9bc10e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anssi=20K=C3=A4=C3=A4ri=C3=A4inen?= Date: Wed, 4 Feb 2015 10:21:29 +0200 Subject: [PATCH] Removed EverythingNode At the same time, made sure that empty nodes in where clause match everything. --- django/db/models/fields/related.py | 1 + django/db/models/sql/query.py | 28 +++++--------- django/db/models/sql/where.py | 60 +++++++++--------------------- tests/queries/tests.py | 39 ++++++------------- 4 files changed, 38 insertions(+), 90 deletions(-) diff --git a/django/db/models/fields/related.py b/django/db/models/fields/related.py index 3a4cf17df3..6cc26dcd57 100644 --- a/django/db/models/fields/related.py +++ b/django/db/models/fields/related.py @@ -1767,6 +1767,7 @@ class ForeignObject(RelatedField): root_constraint.add(lookup_class(targets[0].get_col(alias, sources[0]), value), AND) elif lookup_type == 'in': values = [get_normalized_value(value) for value in raw_value] + root_constraint.connector = OR for value in values: value_constraint = constraint_class() for source, target, val in zip(sources, targets, value): diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py index 719ef0f572..5335d27570 100644 --- a/django/db/models/sql/query.py +++ b/django/db/models/sql/query.py @@ -24,8 +24,8 @@ from django.db.models.sql.constants import (QUERY_TERMS, ORDER_DIR, SINGLE, ORDER_PATTERN, INNER, LOUTER) from django.db.models.sql.datastructures import ( EmptyResultSet, Empty, MultiJoin, Join, BaseTable) -from django.db.models.sql.where import (WhereNode, EverythingNode, - ExtraWhere, AND, OR, NothingNode) +from django.db.models.sql.where import (WhereNode, ExtraWhere, AND, OR, + NothingNode) from django.utils import six from django.utils.deprecation import RemovedInDjango20Warning from django.utils.encoding import force_text @@ -526,20 +526,8 @@ class Query(object): # Now relabel a copy of the rhs where-clause and add it to the current # one. - if rhs.where: - w = rhs.where.clone() - 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) - else: - w = self.where_class() + w = rhs.where.clone() + w.relabel_aliases(change_map) self.where.add(w, connector) # Selection columns and extra extensions are those provided by 'rhs'. @@ -1207,8 +1195,9 @@ class Query(object): # So, demotion is OK. existing_inner = set( (a for a in self.alias_map if self.alias_map[a].join_type == INNER)) - clause, require_inner = self._add_q(q_object, self.used_aliases) - self.where.add(clause, AND) + clause, _ = self._add_q(q_object, self.used_aliases) + if clause: + self.where.add(clause, AND) self.demote_joins(existing_inner) def _add_q(self, q_object, used_aliases, branch_negated=False, @@ -1233,7 +1222,8 @@ class Query(object): child, can_reuse=used_aliases, branch_negated=branch_negated, current_negated=current_negated, connector=connector, allow_joins=allow_joins) joinpromoter.add_votes(needed_inner) - target_clause.add(child_clause, connector) + if child_clause: + target_clause.add(child_clause, connector) needed_inner = joinpromoter.update_join_types(self) return target_clause, needed_inner diff --git a/django/db/models/sql/where.py b/django/db/models/sql/where.py index 6472c47f4f..2b04cbe708 100644 --- a/django/db/models/sql/where.py +++ b/django/db/models/sql/where.py @@ -28,22 +28,22 @@ class WhereNode(tree.Node): the correct SQL). A child is usually an expression producing boolean values. Most likely the - expression is a Lookup instance, but other types of objects fulfilling the - required API could be used too (for example, sql.where.EverythingNode). + expression is a Lookup instance. However, a child could also be any class with as_sql() and either - relabeled_clone() method or relabel_aliases() and clone() methods. The - second alternative should be used if the alias is not the only mutable - variable. + relabeled_clone() method or relabel_aliases() and clone() methods and + contains_aggregate attribute. """ default = AND def split_having(self, negated=False): """ Returns two possibly None nodes: one for those parts of self that - should be pushed to having and one for those parts of self - that should be in where. + should be included in the WHERE clause and one for those parts of + self that must be included in the HAVING clause. """ + if not self.contains_aggregate: + return self, None in_negated = negated ^ self.negated # If the effective connector is OR and this node contains an aggregate, # then we need to push the whole branch to HAVING clause. @@ -57,9 +57,9 @@ class WhereNode(tree.Node): for c in self.children: if hasattr(c, 'split_having'): where_part, having_part = c.split_having(in_negated) - if where_part: + if where_part is not None: where_parts.append(where_part) - if having_part: + if having_part is not None: having_parts.append(having_part) elif c.contains_aggregate: having_parts.append(c) @@ -76,55 +76,39 @@ class WhereNode(tree.Node): None, [] if this node is empty, and raises EmptyResultSet if this node can't match anything. """ - # Note that the logic here is made slightly more complex than - # necessary because there are two kind of empty nodes: Nodes - # containing 0 children, and nodes that are known to match everything. - # A match-everything node is different than empty node (which also - # technically matches everything) for backwards compatibility reasons. - # Refs #5261. result = [] result_params = [] - everything_childs, nothing_childs = 0, 0 - non_empty_childs = len(self.children) + if self.connector == AND: + full_needed, empty_needed = len(self.children), 1 + else: + full_needed, empty_needed = 1, len(self.children) for child in self.children: try: sql, params = compiler.compile(child) except EmptyResultSet: - nothing_childs += 1 + empty_needed -= 1 else: if sql: result.append(sql) result_params.extend(params) else: - if sql is None: - # Skip empty childs totally. - non_empty_childs -= 1 - continue - everything_childs += 1 + full_needed -= 1 # Check if this node matches nothing or everything. # First check the amount of full nodes and empty nodes # to make this node empty/full. - if self.connector == AND: - full_needed, empty_needed = non_empty_childs, 1 - else: - full_needed, empty_needed = 1, non_empty_childs # Now, check if this node is full/empty using the # counts. - if empty_needed - nothing_childs <= 0: + if empty_needed == 0: if self.negated: return '', [] else: raise EmptyResultSet - if full_needed - everything_childs <= 0: + if full_needed == 0: if self.negated: raise EmptyResultSet else: return '', [] - - if non_empty_childs == 0: - # All the child nodes were empty, so this one is empty, too. - return None, [] conn = ' %s ' % self.connector sql_string = conn.join(result) if sql_string: @@ -186,16 +170,6 @@ class WhereNode(tree.Node): return self._contains_aggregate(self) -class EverythingNode(object): - """ - A node that matches everything. - """ - contains_aggregate = False - - def as_sql(self, compiler=None, connection=None): - return '', [] - - class NothingNode(object): """ A node that matches nothing. diff --git a/tests/queries/tests.py b/tests/queries/tests.py index 6f643c898f..03ee709c73 100644 --- a/tests/queries/tests.py +++ b/tests/queries/tests.py @@ -9,7 +9,7 @@ import unittest from django.core.exceptions import FieldError from django.db import connection, DEFAULT_DB_ALIAS from django.db.models import Count, F, Q -from django.db.models.sql.where import WhereNode, EverythingNode, NothingNode +from django.db.models.sql.where import WhereNode, NothingNode from django.db.models.sql.constants import LOUTER from django.db.models.sql.datastructures import EmptyResultSet from django.test import TestCase, skipUnlessDBFeature @@ -2851,20 +2851,10 @@ class WhereNodeTest(TestCase): def test_empty_full_handling_conjunction(self): compiler = WhereNodeTest.MockCompiler() - w = WhereNode(children=[EverythingNode()]) - self.assertEqual(w.as_sql(compiler, connection), ('', [])) - w.negate() - self.assertRaises(EmptyResultSet, w.as_sql, compiler, connection) w = WhereNode(children=[NothingNode()]) self.assertRaises(EmptyResultSet, w.as_sql, compiler, connection) w.negate() self.assertEqual(w.as_sql(compiler, connection), ('', [])) - w = WhereNode(children=[EverythingNode(), EverythingNode()]) - self.assertEqual(w.as_sql(compiler, connection), ('', [])) - w.negate() - self.assertRaises(EmptyResultSet, w.as_sql, compiler, connection) - w = WhereNode(children=[EverythingNode(), self.DummyNode()]) - self.assertEqual(w.as_sql(compiler, connection), ('dummy', [])) w = WhereNode(children=[self.DummyNode(), self.DummyNode()]) self.assertEqual(w.as_sql(compiler, connection), ('(dummy AND dummy)', [])) w.negate() @@ -2876,22 +2866,10 @@ class WhereNodeTest(TestCase): def test_empty_full_handling_disjunction(self): compiler = WhereNodeTest.MockCompiler() - w = WhereNode(children=[EverythingNode()], connector='OR') - self.assertEqual(w.as_sql(compiler, connection), ('', [])) - w.negate() - self.assertRaises(EmptyResultSet, w.as_sql, compiler, connection) w = WhereNode(children=[NothingNode()], connector='OR') self.assertRaises(EmptyResultSet, w.as_sql, compiler, connection) w.negate() self.assertEqual(w.as_sql(compiler, connection), ('', [])) - w = WhereNode(children=[EverythingNode(), EverythingNode()], connector='OR') - self.assertEqual(w.as_sql(compiler, connection), ('', [])) - w.negate() - self.assertRaises(EmptyResultSet, w.as_sql, compiler, connection) - w = WhereNode(children=[EverythingNode(), self.DummyNode()], connector='OR') - self.assertEqual(w.as_sql(compiler, connection), ('', [])) - w.negate() - self.assertRaises(EmptyResultSet, w.as_sql, compiler, connection) w = WhereNode(children=[self.DummyNode(), self.DummyNode()], connector='OR') self.assertEqual(w.as_sql(compiler, connection), ('(dummy OR dummy)', [])) w.negate() @@ -2905,15 +2883,20 @@ class WhereNodeTest(TestCase): compiler = WhereNodeTest.MockCompiler() empty_w = WhereNode() w = WhereNode(children=[empty_w, empty_w]) - self.assertEqual(w.as_sql(compiler, connection), (None, [])) + self.assertEqual(w.as_sql(compiler, connection), ('', [])) w.negate() - self.assertEqual(w.as_sql(compiler, connection), (None, [])) + with self.assertRaises(EmptyResultSet): + w.as_sql(compiler, connection) w.connector = 'OR' - self.assertEqual(w.as_sql(compiler, connection), (None, [])) + with self.assertRaises(EmptyResultSet): + w.as_sql(compiler, connection) w.negate() - self.assertEqual(w.as_sql(compiler, connection), (None, [])) + self.assertEqual(w.as_sql(compiler, connection), ('', [])) w = WhereNode(children=[empty_w, NothingNode()], connector='OR') - self.assertRaises(EmptyResultSet, w.as_sql, compiler, connection) + self.assertEqual(w.as_sql(compiler, connection), ('', [])) + w = WhereNode(children=[empty_w, NothingNode()], connector='AND') + with self.assertRaises(EmptyResultSet): + w.as_sql(compiler, connection) class IteratorExceptionsTest(TestCase):