Django

Code

Ticket #6074 (closed: fixed)

Opened 6 months ago

Last modified 3 weeks ago

QOr doesn't handle two empty ResultSets correctly

Reported by: Honza_Kral Assigned to: raphael
Component: Database wrapper Version: SVN
Keywords: or Q sql sprintdec01 qs-rf-fixed Cc:
Triage Stage: Accepted Has patch: 1
Needs documentation: 0 Needs tests: 0
Patch needs improvement: 0

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

patch.diff (1.5 kB) - added by raphael on 12/01/07 07:41:07.
6074.diff (2.3 kB) - added by Honza_Kral on 12/01/07 08:52:05.
my take on the patch
6074-queryset-refactor.diff (1.7 kB) - added by Honza_Kral on 12/01/07 09:14:41.
fixes the same problem for queryset-refactor branch
6074_2.diff (1.6 kB) - added by raphael on 12/01/07 09:55:16.
6074_3.diff (1.5 kB) - added by raphael on 12/01/07 10:15:33.
Remove performance penalty issue

Change History

12/01/07 06:10:29 changed by raphael

  • owner changed from nobody to raphael.
  • needs_better_patch changed.
  • status changed from new to assigned.
  • needs_tests changed.
  • needs_docs changed.

12/01/07 07:41:07 changed by raphael

  • attachment patch.diff added.

12/01/07 07:49:56 changed by raphael

  • status changed from assigned to closed.
  • has_patch set to 1.
  • resolution set to fixed.

12/01/07 07:54:41 changed by raphael

  • status changed from closed to reopened.
  • resolution deleted.

12/01/07 07:56:52 changed by raphael

  • has_patch deleted.

still an issue in this case :

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

12/01/07 08:45:10 changed by Honza_Kral

  • keywords changed from or Q sql to or Q sql sprintdec01.

12/01/07 08:52:05 changed by Honza_Kral

  • attachment 6074.diff added.

my take on the patch

12/01/07 09:14:41 changed by Honza_Kral

  • attachment 6074-queryset-refactor.diff added.

fixes the same problem for queryset-refactor branch

12/01/07 09:55:16 changed by raphael

  • attachment 6074_2.diff added.

12/01/07 10:15:33 changed by raphael

  • attachment 6074_3.diff added.

Remove performance penalty issue

(follow-up: ↓ 7 ) 12/01/07 10:39:42 changed by mtredinnick

  • keywords changed from or Q sql sprintdec01 to or Q sql sprintdec01 qs-rf.
  • stage changed from Unreviewed to Accepted.

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.

(in reply to: ↑ 6 ) 12/01/07 11:12:11 changed by Honza_Kral

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 )

12/02/07 01:09:27 changed by raphael

  • has_patch set to 1.

(follow-up: ↓ 10 ) 12/02/07 20:55:18 changed by 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.

(in reply to: ↑ 9 ) 12/03/07 03:33:53 changed by Honza_Kral

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).

12/03/07 15:10:56 changed by mtredinnick

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

12/03/07 15:11:28 changed by mtredinnick

  • keywords changed from or Q sql sprintdec01 qs-rf to or Q sql sprintdec01 qs-rf-fixed.

04/26/08 21:50:16 changed by mtredinnick

  • status changed from reopened to closed.
  • resolution set to fixed.

(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

Add/Change #6074 (QOr doesn't handle two empty ResultSets correctly)




Change Properties
Action