Opened 10 months ago

Closed 9 months ago

Last modified 9 months ago

#32657 closed Bug (fixed)

Combining an empty Q with a negated Exists un-negates the Exists lookup

Reported by: Jaap Roes Owned by: Simon Charette
Component: Database layer (models, ORM) Version: 3.2
Severity: Normal Keywords:
Cc: Simon Charette Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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.

Change History (6)

comment:1 Changed 10 months ago 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)

comment:2 Changed 10 months ago by Mariusz Felisiak

Cc: Simon Charette added
Component: UncategorizedDatabase layer (models, ORM)
Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

This can be fixed by #32632 (see comment).

Last edited 10 months ago by Mariusz Felisiak (previous) (diff)

comment:3 Changed 9 months ago by Mariusz Felisiak

Has patch: set
Owner: changed from nobody to Simon Charette
Status: newassigned

comment:4 Changed 9 months ago by Mariusz Felisiak

Triage Stage: AcceptedReady for checkin

comment:5 Changed 9 months ago by Mariusz Felisiak <felisiak.mariusz@…>

Resolution: fixed
Status: assignedclosed

In c8b65943:

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.

comment:6 Changed 9 months ago by Mariusz Felisiak <felisiak.mariusz@…>

In d5add5d3:

[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

Note: See TracTickets for help on using tickets.
Back to Top