Refs #32940 -- Removed unnecessary branch in Node.add().

The "data in self.children" branch was causing data.__eq__ to be
called for each entries in "self.children" which resulted in a huge
slowdown during queryset construction.

It's purpose was to prevent queries of the form
    Model.objects.filter(foo='bar').filter(foo='bar')
from resulting in
    WHERE foo='bar' AND foo='bar'
but it's not covered by the suite and has arguable performance benefits
since it's not very common and SQL engines are usually very good at
folding/optimizing these.

See also #32632 for prior discussion around comparing data to the
Node's children.

Co-authored-by: Nick Pope <nick@nickpope.me.uk>
This commit is contained in:
Keryn Knight 2021-07-20 07:10:52 +02:00 committed by Mariusz Felisiak
parent fb35e0a2fe
commit ff661dbd50
1 changed files with 19 additions and 22 deletions

View File

@ -88,31 +88,28 @@ class Node:
Return a node which can be used in place of data regardless if the
node other got squashed or not.
"""
if self.connector == conn_type and data in self.children:
return data
if self.connector == conn_type:
# We can reuse self.children to append or squash the node other.
if (isinstance(data, Node) and not data.negated and
(data.connector == conn_type or len(data) == 1)):
# We can squash the other node's children directly into this
# node. We are just doing (AB)(CD) == (ABCD) here, with the
# addition that if the length of the other node is 1 the
# connector doesn't matter. However, for the len(self) == 1
# case we don't want to do the squashing, as it would alter
# self.connector.
self.children.extend(data.children)
return self
else:
# We could use perhaps additional logic here to see if some
# children could be used for pushdown here.
self.children.append(data)
return data
else:
obj = self._new_instance(self.children, self.connector,
self.negated)
if self.connector != conn_type:
obj = self._new_instance(self.children, self.connector, self.negated)
self.connector = conn_type
self.children = [obj, data]
return data
elif (
isinstance(data, Node) and
not data.negated and
(data.connector == conn_type or len(data) == 1)
):
# We can squash the other node's children directly into this node.
# We are just doing (AB)(CD) == (ABCD) here, with the addition that
# if the length of the other node is 1 the connector doesn't
# matter. However, for the len(self) == 1 case we don't want to do
# the squashing, as it would alter self.connector.
self.children.extend(data.children)
return self
else:
# We could use perhaps additional logic here to see if some
# children could be used for pushdown here.
self.children.append(data)
return data
def negate(self):
"""Negate the sense of the root connector."""