Fixed #29865 -- Added logical XOR support for Q() and QuerySet(). #14480

rheard · 2021-06-02T19:49:22Z

Add logical XOR support to the ORM. MySQL will use the logical XOR, while other backends will convert

ngnpope

Here's a very quick pass. I've not looked at the tests yet.

django/db/backends/oracle/features.py

django/db/backends/postgresql/features.py

django/db/backends/sqlite3/features.py

django/db/models/expressions.py

django/db/models/sql/where.py

docs/releases/4.0.txt

ngnpope

Thanks for the updates.

Please update some of the existing tests.

Add the following to the end of tests in QTests.test_combine_empty_copy:

            base_q ^ Q(),
            Q() ^ base_q,

Add the following to the end of QTests.test_combine_not_q_object:

        with self.assertRaisesMessage(TypeError, str(obj)):
            q ^ obj

Add the following to the end of tests in QTests.test_combine_negated_boolean_expression:

            Q() ^ ~Exists(tagged),

It seems as though we'd want to add to QuerySetBitwiseOperationTests too to test the QuerySet behavior?

Also, I don't think that we need to create tests/xor_lookups. These tests should fit in around QuerySet-related tests for & and |.

I've marked this as patch needs improvement on ticket-29865. Please remove that flag when you're ready for another round of review. Thanks.

django/db/models/sql/where.py

docs/ref/models/querysets.txt

docs/releases/4.0.txt

rheard · 2021-07-02T20:16:02Z

Thanks for the updates.

Please update some of the existing tests.

Add the following to the end of tests in QTests.test_combine_empty_copy:
            base_q ^ Q(),
            Q() ^ base_q,
Add the following to the end of QTests.test_combine_not_q_object:
        with self.assertRaisesMessage(TypeError, str(obj)):
            q ^ obj
Add the following to the end of tests in QTests.test_combine_negated_boolean_expression:
            Q() ^ ~Exists(tagged),
It seems as though we'd want to add to QuerySetBitwiseOperationTests too to test the QuerySet behavior?

Also, I don't think that we need to create tests/xor_lookups. These tests should fit in around QuerySet-related tests for & and |.

I've marked this as patch needs improvement on ticket-29865. Please remove that flag when you're ready for another round of review. Thanks.

I had some issues around rebasing and force pushing, a nightmare, I think it has been resolved.

I've added those tests in and I added tests to QuerySetBitwiseOperationTests that emulate the "or" tests there, I'm not sure if its how you imagined though.

The tests/xor_lookups seemed appropriate as they were mostly copies of the existing tests/or_lookups. I can merge them both into tests/or_lookups perhaps? I'm just trying imagine the most logical places for them

ngnpope · 2021-07-02T20:53:32Z

The tests/xor_lookups seemed appropriate as they were mostly copies of the existing tests/or_lookups. I can merge them both into tests/or_lookups perhaps? I'm just trying imagine the most logical places for them

My mistake. That'll teach me for making this point based on the non-existent test/and_lookups and missing that test/or_lookups existed! 😆

ngnpope

I've suggested a few more tweaks in the documentation.

Please can you change the commit message and PR title to the following:

Fixed #29865 -- Added logical XOR support for Q() and QuerySet().

I've also noticed that the ticket was triaged and "Someday/Maybe" and not "Accepted". I'm sorry I didn't notice this before. However, based on the fact that we have what seems to be a working implementation here I'll change it to "Accepted". This will put it on the review queue for the fellows to follow up on.

docs/ref/models/querysets.txt

docs/releases/4.0.txt

ngnpope · 2021-07-14T09:51:36Z

buildbot, test on oracle.

felixxm

@rheard Thanks for this patch 👍

docs/ref/models/querysets.txt

django/db/models/sql/where.py

docs/ref/models/querysets.txt

docs/releases/4.0.txt

tests/queries/tests.py

tests/xor_lookups/models.py

tests/xor_lookups/tests.py

django/db/models/sql/where.py

docs/releases/4.1.txt

tests/queries/tests.py

felixxm · 2021-09-24T08:05:55Z

@rheard Thanks for updates 👍

felixxm

@rheard I pushed small edits to tests and docs. Unfortunately, I found an issue in chaining multiple conditions. Can you take a look?

Also, please don't rebase this patch and push edits in separate commits. I will squash everything before merging.

docs/ref/models/querysets.txt

django/db/models/sql/where.py

felixxm · 2021-09-30T17:14:30Z

It looks like the ordering was removed from the Meta though and broke some tests :/

I changed to assertSequenceEqual() and assertCountEqual() so Meta.ordering is not necessary anymore. Only the regression test (xor_lookups.tests.XorLookupsTests.test_filter_negated) for many conditions is broken on SQLite and PostgreSQL (as expected) .

rheard · 2021-09-30T17:16:04Z

It looks like the ordering was removed from the Meta though and broke some tests :/

I changed to assertSequenceEqual() and assertCountEqual() so Meta.ordering is not necessary anymore. Only the regression test (xor_lookups.tests.XorLookupsTests.test_filter_negated) for many conditions is broken on SQLite and PostgreSQL (as expected) .

Sorry, quickly realized that. Again, will implement my proposed solution when I have time. Thanks!

django/db/models/sql/where.py

felixxm · 2022-03-04T05:58:46Z

I rebased from the main branch, resolved conflicts, and applied black.

felixxm

@rheard Thanks for all your efforts 👍 Welcome aboard ⛵

I pushed final edits and squashed commits.

rheard marked this pull request as draft June 2, 2021 20:24

rheard marked this pull request as ready for review June 2, 2021 20:34

ngnpope suggested changes Jun 2, 2021

View reviewed changes

rheard force-pushed the xor_operator branch 3 times, most recently from 77c86b6 to 271e0cf Compare June 2, 2021 22:08

smithdc1 reviewed Jun 3, 2021

View reviewed changes

docs/releases/4.0.txt Outdated Show resolved Hide resolved

rheard force-pushed the xor_operator branch 3 times, most recently from 058a90f to 0fd449f Compare June 4, 2021 19:22

rheard changed the title ~~Implement #29865~~ Implement #29865 - Add Logical XOR support for Q/QuerySet on MySQL Jun 4, 2021

rheard changed the title ~~Implement #29865 - Add Logical XOR support for Q/QuerySet on MySQL~~ Refs #29865 - Add Logical XOR support for Q/QuerySet on MySQL Jun 4, 2021

rheard changed the title ~~Refs #29865 - Add Logical XOR support for Q/QuerySet on MySQL~~ Refs #29865 - Add Logical XOR support for Q/QuerySet Jun 7, 2021

rheard force-pushed the xor_operator branch from 74ec1f7 to d2561b8 Compare June 7, 2021 20:23

rheard requested a review from ngnpope June 10, 2021 03:02

ngnpope suggested changes Jul 2, 2021

View reviewed changes

django/db/models/sql/where.py Outdated Show resolved Hide resolved

docs/ref/models/querysets.txt Show resolved Hide resolved

docs/ref/models/querysets.txt Show resolved Hide resolved

docs/releases/4.0.txt Outdated Show resolved Hide resolved

rheard force-pushed the xor_operator branch 2 times, most recently from bf29100 to 5e11b75 Compare July 2, 2021 20:13

rheard force-pushed the xor_operator branch from 5e11b75 to 348055a Compare July 2, 2021 20:19

rheard force-pushed the xor_operator branch from 348055a to 5602869 Compare July 14, 2021 00:04

rheard requested a review from ngnpope July 14, 2021 00:23

ngnpope reviewed Jul 14, 2021

View reviewed changes

docs/ref/models/querysets.txt Outdated Show resolved Hide resolved

docs/ref/models/querysets.txt Show resolved Hide resolved

docs/releases/4.0.txt Outdated Show resolved Hide resolved

rheard force-pushed the xor_operator branch from 5602869 to b6d3eda Compare July 14, 2021 14:47

rheard changed the title ~~Refs #29865 - Add Logical XOR support for Q/QuerySet~~ Fixed #29865 -- Added logical XOR support for Q() and QuerySet(). Jul 14, 2021

rheard force-pushed the xor_operator branch from b6d3eda to 62d9a7d Compare August 4, 2021 22:20

rheard requested a review from ngnpope August 4, 2021 22:21

felixxm reviewed Sep 23, 2021

View reviewed changes

rheard force-pushed the xor_operator branch 6 times, most recently from 4659059 to dadea92 Compare September 24, 2021 00:07

felixxm reviewed Sep 24, 2021

View reviewed changes

django/db/models/sql/where.py Outdated Show resolved Hide resolved

docs/releases/4.1.txt Show resolved Hide resolved

tests/queries/tests.py Outdated Show resolved Hide resolved

rheard force-pushed the xor_operator branch 2 times, most recently from 317e173 to dcd3ed0 Compare September 24, 2021 21:39

felixxm reviewed Sep 30, 2021

View reviewed changes

docs/ref/models/querysets.txt Outdated Show resolved Hide resolved

django/db/models/sql/where.py Outdated Show resolved Hide resolved

felixxm force-pushed the xor_operator branch 2 times, most recently from d9fac68 to 05942a5 Compare September 30, 2021 07:35

rheard requested a review from felixxm September 30, 2021 23:12

kezabelle reviewed Oct 1, 2021

View reviewed changes

django/db/models/sql/where.py Outdated Show resolved Hide resolved

felixxm force-pushed the xor_operator branch from 17ac163 to e97f65a Compare March 4, 2022 05:58

felixxm force-pushed the xor_operator branch from e97f65a to c73681f Compare March 4, 2022 06:16

felixxm mentioned this pull request Mar 4, 2022

Refs #11293 -- Added test for filtering aggregates with negated & operator. #15476

Merged

felixxm force-pushed the xor_operator branch 3 times, most recently from 0380451 to 00f10e6 Compare March 4, 2022 11:09

felixxm removed the request for review from ngnpope March 4, 2022 11:45

Fixed #29865 -- Added logical XOR support for Q() and querysets.

c6b4d62

felixxm force-pushed the xor_operator branch from 00f10e6 to c6b4d62 Compare March 4, 2022 11:57

felixxm approved these changes Mar 4, 2022

View reviewed changes

felixxm merged commit c6b4d62 into django:main Mar 4, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed #29865 -- Added logical XOR support for Q() and QuerySet(). #14480

Fixed #29865 -- Added logical XOR support for Q() and QuerySet(). #14480

rheard commented Jun 2, 2021 •

edited

ngnpope left a comment

ngnpope left a comment

rheard commented Jul 2, 2021

ngnpope commented Jul 2, 2021

ngnpope left a comment

ngnpope commented Jul 14, 2021

felixxm left a comment

felixxm commented Sep 24, 2021

felixxm left a comment

felixxm commented Sep 30, 2021

rheard commented Sep 30, 2021

felixxm commented Mar 4, 2022

felixxm left a comment

Fixed #29865 -- Added logical XOR support for Q() and QuerySet(). #14480

Fixed #29865 -- Added logical XOR support for Q() and QuerySet(). #14480

Conversation

rheard commented Jun 2, 2021 • edited

ngnpope left a comment

Choose a reason for hiding this comment

ngnpope left a comment

Choose a reason for hiding this comment

rheard commented Jul 2, 2021

ngnpope commented Jul 2, 2021

ngnpope left a comment

Choose a reason for hiding this comment

ngnpope commented Jul 14, 2021

felixxm left a comment

Choose a reason for hiding this comment

felixxm commented Sep 24, 2021

felixxm left a comment

Choose a reason for hiding this comment

felixxm commented Sep 30, 2021

rheard commented Sep 30, 2021

felixxm commented Mar 4, 2022

felixxm left a comment

Choose a reason for hiding this comment

rheard commented Jun 2, 2021 •

edited