Opened 4 years ago

Last modified 4 years ago

#32549 closed Cleanup/optimization

Add `Q.empty()` to check for nested empty Q objects like `Q(Q())` — at Version 3

Reported by: jonathan-golorry Owned by: nobody
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords: Q objects, nested, empty
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by jonathan-golorry)

Empty Q objects (Q() or ~Q(), etc) evaluate to False, but Q(Q()) evaluates to True. This interferes with the shortcut on logical operations involving empty Q objects and can lead to bugs when trying to detect empty operations.

def q_any(iterable):
    q = Q()
    for element in iterable:
        q |= element
    if q:
        return q
    return Q(pk__in=[])
Item.objects.filter(q_any())  # no items
Item.objects.filter(q_any([Q()]))  # no items
Item.objects.filter(q_any([Q(Q())]))  # all items

Patch https://github.com/django/django/pull/14127 removes empty Q objects from args during Q object initialization.

Patch https://github.com/django/django/pull/14130 adds .empty() for detecting nested empty Q objects and uses that instead of boolean evaluation for shortcutting logical operations between empty Q objects. So Q(x=1) | Q(Q()) will now shortcut to Q(x=1) the same way Q(x=1) | Q() does. No simplifying is done during Q object initialization.

This requires https://code.djangoproject.com/ticket/32548 in order to prevent a regression in logical operations between query expressions and empty Q objects.

Change History (3)

comment:1 by jonathan-golorry, 4 years ago

Description: modified (diff)
Has patch: set

comment:2 by Mariusz Felisiak, 4 years ago

Resolution: wontfix
Status: newclosed

Thanks for this proposition, however I don't think we should implicitly remove anything from args, even an empty Q() instances. This would be backward incompatible, inconsistent with the Node behavior, and can be handled in your code.

comment:3 by jonathan-golorry, 4 years ago

Description: modified (diff)
Resolution: wontfix
Status: closednew
Summary: Make `Q(Q(), ...)` equivalent to `Q(...)`Add `Q.empty()` to check for nested empty Q objects like `Q(Q())`

Here is an alternate approach that adds a way to check for nested empty Q objects instead of making them impossible to create.

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