Opened 9 years ago

Closed 8 years ago

#6074 closed (fixed)

QOr doesn't handle two empty ResultSets correctly

Reported by: Honza Král Owned by: Raphaël Braud
Component: Database layer (models, ORM) Version: master
Severity: Keywords: or Q sql sprintdec01 qs-rf-fixed
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

>>> from django.db.models import Q
>>> from django.contrib.auth.models import User
>>> User.objects.filter( Q( id__in=[] ) | Q( id__in=[] ) )

Should be empty queryset, but is not

whe generated WHERE clause is empty, the problem seems to be around
django/db/models/query.py:725

Attachments (5)

patch.diff (1.5 KB) - added by Raphaël Braud 9 years ago.
6074.diff (2.3 KB) - added by Honza Král 9 years ago.
my take on the patch
6074-queryset-refactor.diff (1.7 KB) - added by Honza Král 9 years ago.
fixes the same problem for queryset-refactor branch
6074_2.diff (1.6 KB) - added by Raphaël Braud 9 years ago.
6074_3.diff (1.5 KB) - added by Raphaël Braud 9 years ago.
Remove performance penalty issue

Download all attachments as: .zip

Change History (18)

comment:1 Changed 9 years ago by Raphaël Braud

Needs documentation: unset
Needs tests: unset
Owner: changed from nobody to Raphaël Braud
Patch needs improvement: unset
Status: newassigned

Changed 9 years ago by Raphaël Braud

Attachment: patch.diff added

comment:2 Changed 9 years ago by Raphaël Braud

Has patch: set
Resolution: fixed
Status: assignedclosed

comment:3 Changed 9 years ago by Raphaël Braud

Resolution: fixed
Status: closedreopened

comment:4 Changed 9 years ago by Raphaël Braud

Has patch: unset

still an issue in this case :

User.objects.filter( Q( idin=[]) | Q( idin=[1,]) | Q(idin=[] ) )

comment:5 Changed 9 years ago by Honza Král

Keywords: sprintdec01 added

Changed 9 years ago by Honza Král

Attachment: 6074.diff added

my take on the patch

Changed 9 years ago by Honza Král

Attachment: 6074-queryset-refactor.diff added

fixes the same problem for queryset-refactor branch

Changed 9 years ago by Raphaël Braud

Attachment: 6074_2.diff added

Changed 9 years ago by Raphaël Braud

Attachment: 6074_3.diff added

Remove performance penalty issue

comment:6 Changed 9 years ago by Malcolm Tredinnick

Keywords: qs-rf added
Triage Stage: UnreviewedAccepted

Any patches against query creation for trunk aren't likely to survive at the moment, since queryset-refactor is very advanced. I have a feeling this is going to be fixed naturally in that branch (there are some known problems with disjunctive joins at the moment, but they'll be fixed). I'll close this once I've verified it's fixed there and the branch is merged.

comment:7 in reply to:  6 Changed 9 years ago by Honza Král

Replying to mtredinnick:

Any patches against query creation for trunk aren't likely to survive at the moment, since queryset-refactor is very advanced. I have a feeling this is going to be fixed naturally in that branch (there are some known problems with disjunctive joins at the moment, but they'll be fixed). I'll close this once I've verified it's fixed there and the branch is merged.

One of the patches fixes the problem in qs-rf branch, where the problem exists as well. ( 6074-queryset-refactor.diff )

comment:8 Changed 9 years ago by Raphaël Braud

Has patch: set

comment:9 Changed 9 years ago by Malcolm Tredinnick

Unfortunately, that doesn't fix it properly in the queryset-refactor branch. There is some more general work required to make disjunctions work correctly there (which is why three tests are currently failing in that branch: to remind me to fix that properly). Thanks for trying, anyway. It's just a bit trickier than it looks because this is a special case of something more general.

comment:10 in reply to:  9 Changed 9 years ago by Honza Král

Replying to mtredinnick:

Unfortunately, that doesn't fix it properly in the queryset-refactor branch. There is some more general work required to make disjunctions work correctly there (which is why three tests are currently failing in that branch: to remind me to fix that properly). Thanks for trying, anyway. It's just a bit trickier than it looks because this is a special case of something more general.

I didn't presume to fix the whole QOr behavior in qs-rf branch - the patch I supplied merely fixed this little issue (the tests attached failed without it and passes with it).

comment:11 Changed 9 years ago by Malcolm Tredinnick

(In [6868]) queryset-refactor: Fixed disjunctions of empty result sets. Refs #6074.

comment:12 Changed 9 years ago by Malcolm Tredinnick

Keywords: qs-rf-fixed added; qs-rf removed

comment:13 Changed 8 years ago by Malcolm Tredinnick

Resolution: fixed
Status: reopenedclosed

(In [7477]) Merged the queryset-refactor branch into trunk.

This is a big internal change, but mostly backwards compatible with existing
code. Also adds a couple of new features.

Fixed #245, #1050, #1656, #1801, #2076, #2091, #2150, #2253, #2306, #2400, #2430, #2482, #2496, #2676, #2737, #2874, #2902, #2939, #3037, #3141, #3288, #3440, #3592, #3739, #4088, #4260, #4289, #4306, #4358, #4464, #4510, #4858, #5012, #5020, #5261, #5295, #5321, #5324, #5325, #5555, #5707, #5796, #5817, #5987, #6018, #6074, #6088, #6154, #6177, #6180, #6203, #6658

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