Opened 4 years ago

Closed 20 months ago

#24279 closed Bug (fixed)

ORing with an empty Q() produces inconsistent results

Reported by: ris Owned by: nobody
Component: Database layer (models, ORM) Version: 1.7
Severity: Normal Keywords: Q empty in or
Cc: Дилян Палаузов Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Using django 1.7.4, postgres 9.1.

Minimal testcase models.py:

from django.db import models

class ModelA ( models.Model ):
	pass

then:

>>> for i in xrange ( 1 , 5 ):
...     ModelA.objects.create ( pk = i )

>>> ModelA.objects.filter ( Q ( pk__in = () ) )
[]

correctly returning no results.

also

>>> ModelA.objects.filter ( Q ( pk__in = (1,2,) ) | Q () )
[<ModelA: 1>, <ModelA: 2>]

again, correct

however

>>> ModelA.objects.filter ( Q ( pk__in = () ) | Q () )
[<ModelA: 1>, <ModelA: 2>, <ModelA: 3>, <ModelA: 4>]

where I would have expected no instances to be returned.

The empty Q here has the effect of turning the pk__in into a no-op.

Interestingly, reformulating it as a (no-result-returning) subquery produces the expected result:

>>> ModelA.objects.filter ( Q ( pk__in = ModelA.objects.filter ( pk__isnull = True ) ) | Q () )
[]

I realize that OR'ing with an empty Q may not be something you're really supposed to do, but sometimes it does happen and this behaviour is quite unexpected.

Apologies if this is a duplicate but I was unable to find an existing ticket covering this.

Change History (6)

comment:1 Changed 4 years ago by Anssi Kääriäinen

Triage Stage: UnreviewedAccepted

ORin with an empty Q() is something we support. A common pattern is to do something like this:

condition = Q()
if foo:
    condition = condition | Q(foo=True)
if bar:
   condition = condition | Q(bar=True)
qs.filter(condition)

The empty Q() should not have any effect at all, whether ORed or ANDed into the query.

I confirmed the issue with a test available in https://github.com/akaariai/django/tree/ticket_24279.

As it happens, work done in https://github.com/django/django/pull/3779 fixes this issue (it changes the way empty Q() nodes are handled). It should be committed to master any minute now. I'm not sure if we need to fix this in 1.8 or 1.7. This isn't a serious bug (as in, things have worked like this for a long time, and users have survived it). This will also subtly change results of some queries, so I'm not too enthusiastic to backpatch this to released versions.

comment:2 Changed 4 years ago by Anssi Kääriäinen

PR 3779 is now merged. I'll make a PR for this ticket to add the test. I think fixing this somewhat rare issue on master only should be OK.

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

comment:4 Changed 21 months ago by Дилян Палаузов

Is this fixed or not? The last comment says it is, but the status is "new Bug".

comment:5 Changed 21 months ago by Дилян Палаузов

Cc: Дилян Палаузов added

comment:6 Changed 20 months ago by Tim Graham

Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.
Back to Top