Opened 3 hours ago

Last modified 3 hours ago

#37470 assigned Bug

NegatedExpression doesn't handle FullResultSet, so negating an always-true condition evaluates to true. — at Initial Version

Reported by: maiload Owned by: maiload
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords: NegatedExpression FullResultSet CheckConstraint
Cc: maiload Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

NegatedExpression.as_sql() handles EmptyResultSet raised by the wrapped expression, but not FullResultSet. When the wrapped condition matches everything, FullResultSet propagates un-negated and callers treat NOT <always true> as always true.

One visible consequence is that a CheckConstraint using a negated lookup is not enforced by model validation for NULL values, while the database does enforce it:

from django.db import models
from django.db.models import F
from django.db.models.lookups import IsNull


class Item(models.Model):
    value = models.IntegerField(null=True)

    class Meta:
        constraints = [
            models.CheckConstraint(
                condition=~IsNull(F("value"), True),
                name="value_not_null",
            ),
        ]
Item(value=None).validate_constraints()  # Expected ValidationError, nothing is raised.
Item(value=None).save()  # The database rejects it: IntegrityError.

The equivalent ~Q(value__isnull=True) constraint raises ValidationError as expected.

The same root cause affects queries:

always_false = ~ExpressionWrapper(~Q(pk__in=[]), output_field=BooleanField())
Item.objects.filter(always_false)            # Expected: no rows. Actual: all rows.
Item.objects.annotate(x=always_false)        # Expected: x=False. Actual: x=True.

The CheckConstraint case is a regression in 3434dbd39d373df7193ad006b970c09c1a909ea3 (#34754, backported to 4.2.5), which made IsNull(Value(None), True) raise FullResultSet. On its parent commit (2b582387d5) validate_constraints() correctly raises ValidationError. The queryset cases have been incorrect since FullResultSet was introduced in 76e37513e22f4d9a01c7f15eee36fe44388e6670 (refs #33374), shortly after NegatedExpression was added for #16211.

Adding an except FullResultSet: raise EmptyResultSet branch to NegatedExpression.as_sql() fixes it: the reproduction tests pass and the full test suite passes on SQLite with that change. I haven't tested other backends yet. I'd like to submit a PR with a regression test.

With this change the reproduction tests pass and the full test suite passes on SQLite. I haven't tested other backends yet.

Reproduced on main (b501b8493f) with SQLite.

I used Claude Code to help explore the codebase and draft the reproduction tests. I verified the behavior, the regression commit, and the test results myself.

Change History (0)

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