Combining Q with a Q containing Exists crashes

[Jaap Roes]

I'm in the process of upgrading a project from Django 3.1 to 3.2 and one of the tests keeps crashing. I reduced the code to the following statement:

Q() & Q(Exists(Book.objects.all()))

This worked correctly in Django 3.1.

I found this ticket: #32548 and can confirm (that after some struggles related to incompatibilities in 3rd party packges) I was able to get the tests to pass on Django main. Is it possible to backport the fix to Django 3.2?

[Jaap Roes]

From #32548 I gather that Q() & Exists(Book.objects.all()) is supposed to be equivalent to Q() & Q(Exists(Book.objects.all())).

I tried this as a workaround, but our tests fail because the returned data after applying the filters is not the same. On Django 3.1 this crashes with a TypeError raised from query_utils.py._combine.

This makes it impossible to upgrade to Django 3.2 at this point in time.

[Jaap Roes]

I figured out why the tests results where different, for some reason combining an empty Q with a negated Exists (using the ~ operator) results in an unnegated Exists:

>>> q = Q() & ~Exists(Book.objects.all())
>>> q.negated
False

Setting negated on the constructor does work:

>>> q = Q() & Exists(Vacancy.objects.all(), negated=True)
>>> q.negated
True

This might be a separate bug.

At least I'm now able to finish the Django 3.2 upgrade.

[Mariusz Felisiak]

Replying to Jaap Roes:

I'm in the process of upgrading a project from Django 3.1 to 3.2 and one of the tests keeps crashing. I reduced the code to the following statement:

Q() & Q(Exists(Book.objects.all()))

This worked correctly in Django 3.1.

I found this ticket: #32548 and can confirm (that after some struggles related to incompatibilities in 3rd party packges) I was able to get the tests to pass on Django main. Is it possible to backport the fix to Django 3.2?

Thanks I didn't notice that it's a regression in 466920f6d726eee90d5566e0a9948e92b33a122e (backported in 0e2979e95d0f68f28c92c84c14655e7510d4e789). I will prepare a backport to Django 3.2.

Let's close it as a duplicate of #32548.

[Jaap Roes]

@Mariusz Nice! Does that backport also fix the weird behaviour in my previous comment?

[Mariusz Felisiak]

Replying to Jaap Roes:

@Mariusz Nice! Does that backport also fix the weird behaviour in my previous comment?

Unfortunately no, it's a separate issue related with the way how Exists.__invert__() and Subquery.__getstate__() are defined. It's quite serious, see

q = Exists(Employee.objects.all())
assert hash(q) != hash(~q)

Can you create a separate ticket.

[Simon Charette]

Right, yet another issue with how BaseExpression.identity is based off _constructor_args. Defining a proper Subquery.identity should address this issue but it might not even be necessary if we go forward with https://code.djangoproject.com/ticket/32632#comment:4 and drop partial support Subquery.deconstruct, __eq__, and __hash__.

[Jaap Roes]

I created #32657 for the q = Q() & ~Exists(Book.objects.all()) issue