Opened 5 years ago

Closed 5 years ago

#28211 closed Cleanup/optimization (fixed)

Inefficient query produced when using OR'ed Q objects

Reported by: Tom Forbes Owned by: nobody
Component: Database layer (models, ORM) Version: dev
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 (last modified by Tom Forbes)

When using OR'ed Q objects in a .filter() call Django could be more intelligent about the query it produces.

For example, the following two queries both produce LEFT OUTER JOIN:

SomeModel.objects.filter(Q(related_objects__field=1) | Q(related_objects__other_field=1)

SomeModel.objects.filter(Q(related_objects__field=1) | Q())

In the case of the second query it could be reduced to a INNER JOIN as the OR is redundant and it is functionally the same as a .filter(related_objects__field=1).

It is also quite a common pattern to do:

filters = Q()
if condition:
   filters |= Q(x=1)
if other_condition:
   filters |= Q(y=2)

And with the current implementation it will always produce a query that assumes filters is a valid OR. Django should/could be more intelligent and detect if there is only one OR condition, and reduce it.

Change History (6)

comment:1 Changed 5 years ago by Tom Forbes

Description: modified (diff)

comment:2 Changed 5 years ago by Tim Graham

Triage Stage: UnreviewedAccepted

Tentatively accepting, though I don't know if this is feasible or correct. If a patch is provided, hopefully the test suite will validate its correctness.

comment:3 Changed 5 years ago by Tom Forbes

Has patch: set

I added a patch here: https://github.com/django/django/pull/8517

I took the easy route and fixed what I think is an issue in the Q object. Combining empty Q objects should be a no-op. This effectively fixes this ticket without many changes.

comment:4 Changed 5 years ago by Simon Charette

Triage Stage: AcceptedReady for checkin
Version: 1.10master

I think the provided patch solves this in an efficient way.

We could add folding logic in the query compiler as well but given it will be very hard to generate combined Q with a falsey sides after with this patch (one would have to call Q.add manually) I don't think it's required.

comment:5 Changed 5 years ago by Tim Graham <timograham@…>

In 1c3a6ce:

Refs #28211 -- Added a test for ANDing empty Q()'s.

This test passes to due some logic in Node.add().

comment:6 Changed 5 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: newclosed

In bb0b6e52:

Fixed #28211 -- Prevented ORing an empty Q() from reducing query join efficiency.

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