Opened 3 months ago

Closed 2 months ago

#35559 closed Cleanup/optimization (fixed)

Calling list() on an empty sliced union still causes a database query

Reported by: Lucidiot Owned by: Simon Charette
Component: Database layer (models, ORM) Version: 5.0
Severity: Normal Keywords:
Cc: 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

When upgrading from Django 4.1 to 4.2, some of our unit tests failed because of unexpected SQL queries, and those queries are still present in Django 5.0.

We have a Django REST Framework API view, which relies on Django's Paginator to do its pagination. It uses a QuerySet which involves a UNION of two other queries that both have some filters. Those filters can cause the whole union to be empty. The Paginator knows that the result count is 0, so it returns a page of results by slicing with [0:0].

In Django 4.1, calling list() on that sliced union would cause no query and an empty list is returned, but starting with 4.2, a query runs:

qs = Thing.objects.filter(id__in=Thing.objects.none())
list(qs)                 # 0 queries
list(qs[0:0])            # 0 queries
list(qs.union(qs))       # 0 queries
list(qs.union(qs)[0:0])  # 1 query
(
    SELECT "app_thing"."id" AS "col1"
    FROM "app_thing"
    WHERE 0 = 1
) UNION (
    SELECT "app_thing"."id" AS "col1"
    FROM "app_thing"
    WHERE 0 = 1
)

I think this extra query was added in the fix for #34125.

Change History (6)

comment:1 by Simon Charette, 3 months ago

Owner: changed from nobody to Simon Charette
Status: newassigned
Triage Stage: UnreviewedAccepted
Type: BugCleanup/optimization

I confirm that c2cc80756b8949cdd87b88bbfdfee698ced441e0 focused on the correctness aspect of the problem (avoiding the crash) without focusing on some edge cases that might still allow the query eliding to take place.

comment:2 by Simon Charette, 3 months ago

Has patch: set

comment:3 by Sarah Boyce, 2 months ago

Needs tests: set

comment:4 by Simon Charette, 2 months ago

Needs tests: unset

Wrong assessment, test coverage exists as the crash detailed in #34125 that this ticket change the approach for (to prevent queries) only manifests itself on Postgres.

Version 1, edited 2 months ago by Simon Charette (previous) (next) (diff)

comment:5 by Sarah Boyce, 2 months ago

Triage Stage: AcceptedReady for checkin

comment:6 by Sarah Boyce <42296566+sarahboyce@…>, 2 months ago

Resolution: fixed
Status: assignedclosed

In 9cb8baa0:

Fixed #35559 -- Avoided unnecessary query on sliced union of empty queries.

While refs #34125 focused on the SQL correctness of slicing of union of
potentially empty queries it missed an optimization opportunity to avoid
performing a query at all when all queries are empty.

Thanks Lucidiot for the report.

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