From ff661dbd506efbdf51cc8da89cb98731c8a62f49 Mon Sep 17 00:00:00 2001 From: Keryn Knight Date: Tue, 20 Jul 2021 07:10:52 +0200 Subject: [PATCH] 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 --- django/utils/tree.py | 41 +++++++++++++++++++---------------------- 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/django/utils/tree.py b/django/utils/tree.py index d13883a00e..a56442c32d 100644 --- a/django/utils/tree.py +++ b/django/utils/tree.py @@ -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."""