Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed #32940 -- Removed unnecessary branch and unused squash parameter in Node.add(). #14658

Merged
merged 2 commits into from Jul 20, 2021

Conversation

kezabelle
Copy link
Contributor

Ticket 32940. Let's see if the whole test suite passes or if it will make a liar of me.

@kezabelle kezabelle marked this pull request as ready for review July 18, 2021 11:33
Copy link
Member

@ngnpope ngnpope left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@charettes Would you be able to cast an eye over this, particularly because of the removal of the if self.connector == conn_type and data in self.children: line which was recently fixed by you to address a longstanding regression. Thanks! 🙂

django/utils/tree.py Outdated Show resolved Hide resolved
django/utils/tree.py Outdated Show resolved Hide resolved
kezabelle and others added 2 commits July 20, 2021 07:19
Unused since its introduction in d3f00bd.

Co-authored-by: Nick Pope <nick@nickpope.me.uk>
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>
@felixxm
Copy link
Member

felixxm commented Jul 20, 2021

Thanks y'all 🥇 I split commits.

@felixxm felixxm changed the title Fixed #32940 - Removed unused squash parameter and unused conditionals Fixed #32940 -- Removed unnecessary branch and unused squash parameter in Node.add(). Jul 20, 2021
@felixxm felixxm merged commit ff661db into django:main Jul 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants