[Django] #32657: Combining an empty Q with a negated Exists un-negates the Exists lookup

50 views
Skip to first unread message

Django

unread,
Apr 15, 2021, 3:14:03 AM4/15/21
to django-...@googlegroups.com
#32657: Combining an empty Q with a negated Exists un-negates the Exists lookup
-----------------------------------------+------------------------
Reporter: Jaap Roes | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 3.2
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-----------------------------------------+------------------------
The following test case fails in Django 3.2 and main:

{{{
class TestEmptyQExistsCombination(TestCase):
def test_combine(self):
q = Q() & Exists(Book.objects.all())
self.assertFalse(q.negated) # passes

def test_combine_negated(self):
q = Q() & ~Exists(Book.objects.all())
self.assertTrue(q.negated) # fails
}}}

I noticed this issue trying to work around issue #32651/ #32548.

--
Ticket URL: <https://code.djangoproject.com/ticket/32657>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Apr 15, 2021, 3:17:40 AM4/15/21
to django-...@googlegroups.com
#32657: Combining an empty Q with a negated Exists un-negates the Exists lookup
-------------------------------+--------------------------------------

Reporter: Jaap Roes | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 3.2
Severity: Normal | Resolution:

Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------------------------

Comment (by Jaap Roes):

Note, in Django 3.1 the code in the previous tests will raise a
`TypeError`, the following test case achieves the same and passes in
Django 3.1, main and (from what I understand) Django 3.2.1

{{{
class TestEmptyQExistsCombination(TestCase):
def test_combine(self):
q = Q() & Q(Exists(Book.objects.all()))
self.assertFalse(q.children[0].negated)

def test_combine_negated(self):
q = Q() & Q(~Exists(Book.objects.all()))
self.assertTrue(q.children[0].negated)
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/32657#comment:1>

Django

unread,
Apr 15, 2021, 3:42:39 AM4/15/21
to django-...@googlegroups.com
#32657: Combining an empty Q with a negated Exists un-negates the Exists lookup
-------------------------------------+-------------------------------------

Reporter: Jaap Roes | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):

* cc: Simon Charette (added)
* type: Uncategorized => Bug
* component: Uncategorized => Database layer (models, ORM)
* stage: Unreviewed => Accepted


Comment:

This can be fixed by #32632.

--
Ticket URL: <https://code.djangoproject.com/ticket/32657#comment:2>

Django

unread,
Apr 28, 2021, 4:46:36 AM4/28/21
to django-...@googlegroups.com
#32657: Combining an empty Q with a negated Exists un-negates the Exists lookup
-------------------------------------+-------------------------------------
Reporter: Jaap Roes | Owner: Simon
| Charette
Type: Bug | Status: assigned

Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):

* owner: nobody => Simon Charette
* status: new => assigned
* has_patch: 0 => 1


Comment:

[https://github.com/django/django/pull/14309 PR]

--
Ticket URL: <https://code.djangoproject.com/ticket/32657#comment:3>

Django

unread,
Apr 28, 2021, 5:56:52 AM4/28/21
to django-...@googlegroups.com
#32657: Combining an empty Q with a negated Exists un-negates the Exists lookup
-------------------------------------+-------------------------------------
Reporter: Jaap Roes | Owner: Simon
| Charette
Type: Bug | Status: assigned
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):

* stage: Accepted => Ready for checkin


--
Ticket URL: <https://code.djangoproject.com/ticket/32657#comment:4>

Django

unread,
Apr 28, 2021, 2:28:13 PM4/28/21
to django-...@googlegroups.com
#32657: Combining an empty Q with a negated Exists un-negates the Exists lookup
-------------------------------------+-------------------------------------
Reporter: Jaap Roes | Owner: Simon
| Charette
Type: Bug | Status: closed

Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution: fixed

Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):

* status: assigned => closed
* resolution: => fixed


Comment:

In [changeset:"c8b659430556dca0b2fe27cf2ea0f8290dbafecd" c8b65943]:
{{{
#!CommitTicketReference repository=""
revision="c8b659430556dca0b2fe27cf2ea0f8290dbafecd"
Fixed #32632, Fixed #32657 -- Removed flawed support for Subquery
deconstruction.

Subquery deconstruction support required implementing complex and
expensive equality rules for sql.Query objects for little benefit as
the latter cannot themselves be made deconstructible to their reference
to model classes.

Making Expression @deconstructible and not BaseExpression allows
interested parties to conform to the "expression" API even if they are
not deconstructible as it's only a requirement for expressions allowed
in Model fields and meta options (e.g. constraints, indexes).

Thanks Phillip Cutter for the report.

This also fixes a performance regression in
bbf141bcdc31f1324048af9233583a523ac54c94.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/32657#comment:5>

Django

unread,
Apr 28, 2021, 2:28:36 PM4/28/21
to django-...@googlegroups.com
#32657: Combining an empty Q with a negated Exists un-negates the Exists lookup
-------------------------------------+-------------------------------------
Reporter: Jaap Roes | Owner: Simon
| Charette
Type: Bug | Status: closed
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Mariusz Felisiak <felisiak.mariusz@…>):

In [changeset:"d5add5d3a26f98e961dfbcad67bb04d936f2f332" d5add5d3]:
{{{
#!CommitTicketReference repository=""
revision="d5add5d3a26f98e961dfbcad67bb04d936f2f332"
[3.2.x] Fixed #32632, Fixed #32657 -- Removed flawed support for Subquery
deconstruction.

Subquery deconstruction support required implementing complex and
expensive equality rules for sql.Query objects for little benefit as
the latter cannot themselves be made deconstructible to their reference
to model classes.

Making Expression @deconstructible and not BaseExpression allows
interested parties to conform to the "expression" API even if they are
not deconstructible as it's only a requirement for expressions allowed
in Model fields and meta options (e.g. constraints, indexes).

Thanks Phillip Cutter for the report.

This also fixes a performance regression in
bbf141bcdc31f1324048af9233583a523ac54c94.

Backport of c8b659430556dca0b2fe27cf2ea0f8290dbafecd from main
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/32657#comment:6>

Reply all
Reply to author
Forward
0 new messages