#32651 closed Bug (duplicate)
Combining Q with a Q containing Exists crashes
| Reported by: | Jaap Roes | Owned by: | nobody |
|---|---|---|---|
| Component: | Database layer (models, ORM) | Version: | 3.2 |
| Severity: | Normal | Keywords: | |
| Cc: | Simon Charette | Triage Stage: | Unreviewed |
| Has patch: | no | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
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?
Change History (9)
comment:1 by , 5 years ago
comment:2 by , 5 years ago
| Summary: | Combining Q with a Q containing exists crashes → Combining Q with a Q containing Exists crashes |
|---|
comment:3 by , 5 years ago
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(Book.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.
comment:4 by , 5 years ago
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.
follow-up: 7 comment:5 by , 5 years ago
@Mariusz Nice! Does that backport also fix the weird behaviour in my previous comment?
comment:6 by , 5 years ago
| Resolution: | → duplicate |
|---|---|
| Status: | new → closed |
comment:7 by , 5 years ago
| Cc: | added |
|---|
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.
comment:8 by , 5 years ago
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__.
From #32548 I gather that
Q() & Exists(Book.objects.all())is supposed to be equivalent toQ() & 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
TypeErrorraised fromquery_utils.py._combine.This makes it impossible to upgrade to Django 3.2 at this point in time.