Skip to content

add xor support for Q objects #81

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jul 15, 2024
Merged

add xor support for Q objects #81

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/test-python.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ jobs:
sessions_tests
timezones
update
xor_lookups

docs:
name: Docs Checks
Expand Down
26 changes: 21 additions & 5 deletions django_mongodb/query.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
from functools import wraps
from functools import reduce, wraps
from operator import add as add_operator

from django.core.exceptions import EmptyResultSet, FullResultSet
from django.db import DatabaseError, IntegrityError
from django.db.models import Value
from django.db.models.expressions import Case, Value, When
from django.db.models.functions import Mod
from django.db.models.lookups import Exact
from django.db.models.sql.constants import INNER
from django.db.models.sql.datastructures import Join
from django.db.models.sql.where import AND, XOR, WhereNode
from django.db.models.sql.where import AND, OR, XOR, WhereNode
from pymongo import ASCENDING, DESCENDING
from pymongo.errors import DuplicateKeyError, PyMongoError

Expand Down Expand Up @@ -219,8 +222,21 @@ def where_node(self, compiler, connection):
if self.connector == AND:
operator = "$and"
elif self.connector == XOR:
# https://github.com/mongodb-labs/django-mongodb/issues/27
raise NotImplementedError("XOR is not yet supported.")
# MongoDB doesn't support $xor, so convert:
Copy link
Collaborator

@WaVEV WaVEV Jul 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Another way to compute XOR is by recognizing that XOR is equivalent to "not equal" ($ne). Therefore, the expression a xor b xor c can be interpreted as (a $ne b) $ne c. Whit this we can avoid checking parity.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it was originally implemented like that, but it doesn't work with multiple conditions: django/django#14480 (comment).

Copy link
Contributor

Choose a reason for hiding this comment

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

Just to check, this is as it relates to logical operators and not for numerical calculations?
If so, then I wholeheartedly agree.
Example :

>> XOR(2,5) = 7 # True
>> XOR(2,2) = 0 # False

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, you can look at the new functionality this enables: https://github.com/django/django/blob/main/tests/xor_lookups/tests.py

bitXor (tests) is already implemented: https://github.com/mongodb-labs/django-mongodb/blob/6b18784e443c93c80d317812c9cf0055e1333498/django_mongodb/operations.py#L27

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think I cannot read the commits, but the idea is not use and as a nexo between operators.
I am thinking of:

# (((a != b) != c) != ... )
# I think we cannot create a not Exact so easily. but we have  ~a ≡ a == False => Exact(Exact(a, b), False) will generate a not equal. 
pred = Value(False)  # is the neutro of the xor and base case of XOR quantifier
for c in self.children:
    pred = Exact(Exact(pred, c), Value(False))
return self.__class__((lhs, pred), AND, self.negated).as_mql(compiler, connection)

The other idea is, changing the line when mongo-operator is used, and doing something like:

if len(children_mql) == 1:
    mql = children_mql[0]
elif len(children_mql) > 1:
    if self.connector == XOR:
        mql = {"$literal": False}
        for prop in children_mql:
            mql = {"$ne": [mql, prop]}
    else:
        mql = {operator: children_mql} if children_mql else {}
else:
    mql = {}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The first suggestion doesn't pass xor_lookups.tests.XorLookupsTests.test_empty_in. No results are returned for Number.objects.filter(Q(pk__in=[]) ^ Q(num__gte=5)), probably because Q(pk__in=[]) raises EmptyResultSet short-circuiting the entire expression.

The second suggestion passes the tests. It's less clear to me how to translate that to Django's WhereNode, if we wanted to suggest that implementation upstream.

Is the main advantage you see in your suggestion a simpler query without Case/When?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Well, the advantage is not much, question of taste.
The other idea is:

# I think we cannot create a not Exact so easily. but we have  a!= b ≡  -a = b 
pred = Value(False)  # is the neutro of the xor and base case of XOR quantifier
for c in self.children:
    pred = Exact(pred, Case(When(c, then=False), default=True))
return self.__class__((lhs, pred), AND, self.negated).as_mql(compiler, connection)

The case catch the empty of full results.
Note that the c is negated in the when.
As I said above, question of taste. The PR is good to me. Ignore this if you think the other way is simpler.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I prefer to copy Django's implementation so if something comes up later, we can (probably) just copy in those changes.

# a XOR b XOR c XOR ...
# to:
# (a OR b OR c OR ...) AND MOD(a + b + c + ..., 2) == 1
# The result of an n-ary XOR is true when an odd number of operands
# are true.
lhs = self.__class__(self.children, OR)
rhs_sum = reduce(
add_operator,
(Case(When(c, then=1), default=0) for c in self.children),
)
if len(self.children) > 2:
rhs_sum = Mod(rhs_sum, 2)
rhs = Exact(1, rhs_sum)
return self.__class__([lhs, rhs], AND, self.negated).as_mql(compiler, connection)
else:
operator = "$or"

Expand Down