Opened 10 years ago

Closed 7 years 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 by Anssi Kääriäinen, 10 years ago

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 by Anssi Kääriäinen, 10 years ago

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 by Tim Graham <timograham@…>, 10 years ago

comment:4 by Дилян Палаузов, 7 years ago

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

comment:5 by Дилян Палаузов, 7 years ago

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

comment:6 by Tim Graham, 7 years ago

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