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 #23797 -- Fixed QuerySet.exclude() when rhs is a nullable column. #13118

Merged
merged 1 commit into from Jul 6, 2020

Conversation

jacobtylerwalls
Copy link
Member

@jacobtylerwalls jacobtylerwalls commented Jun 27, 2020

Fixes https://code.djangoproject.com/ticket/23797

The referenced ticket demonstrates that exclude() and filter() are not complementary when the value of an F() expression is NULL. The discussion on the ticket reports that:
exclude() and filter() should be complementary, and thus that exclude() should return a record where the F() expression evaluates to NULL
• The requisite query should be: NOT ((length = width) AND (length IS NOT NULL) AND (width IS NOT NULL)) to avoid performance problems with IS TRUE, which is to say, for exclude() to return a record, one of three things must obtain: the comparison test fails, the tested value is null, or the comparison value is null.
• The change should happen directly in build_filter()

Then discussion trails off. This was straightforward to implement; I just needed to check if the value (in the place of the F expression) was an instance of a Col before adding the IS NOT NULL clause. I look forward to your feedback. :-)

@jacobtylerwalls
Copy link
Member Author

The doctest is failing because it thinks complementarity is not a word, even though it is used elsewhere in the codebase.

Copy link
Member

@felixxm felixxm left a comment

Choose a reason for hiding this comment

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

@jacobtylerwalls Thanks for this patch 👍

docs/releases/3.0.8.txt Outdated Show resolved Hide resolved
tests/queries/models.py Outdated Show resolved Hide resolved
tests/queries/tests.py Outdated Show resolved Hide resolved
@jacobtylerwalls jacobtylerwalls force-pushed the ticket_23797 branch 2 times, most recently from 45cdb22 to 4c5fa34 Compare July 3, 2020 22:35
@felixxm felixxm self-assigned this Jul 6, 2020
@felixxm felixxm changed the title Fixed #23797 - exclude() returns records where F() expression IS NULL Fixed #23797 -- Fixed QuerySet.exclude() when rhs is a nullable column. Jul 6, 2020
@felixxm
Copy link
Member

felixxm commented Jul 6, 2020

@jacobtylerwalls Thanks 👍 I pushed small edits.

@felixxm felixxm merged commit 512da9d into django:master Jul 6, 2020
@jacobtylerwalls jacobtylerwalls deleted the ticket_23797 branch July 6, 2020 11:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants