Opened 12 years ago

Closed 12 years ago

Last modified 11 years ago

#17681 closed Bug (worksforme)

Annotating an EmptyQueryset lead to not empty queryset

Reported by: inscription.tresontani@… Owned by: nobody
Component: Database layer (models, ORM) Version: 1.3
Severity: Normal Keywords: EmptyQueryset, annotate
Cc: Anssi Kääriäinen Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

Doing Table.objects.none().annotate(result=Sum("field1")) run a query.

On big table, 5 millions row in our case, this lead to a query making mySQL crashed.

Attachments (1)

emptyqs_removal.diff (12.8 KB ) - added by Anssi Kääriäinen 12 years ago.

Download all attachments as: .zip

Change History (7)

comment:1 by Anssi Kääriäinen, 12 years ago

I did some work of removing the EmptyQuerySet class altogether some time ago. The idea is to add something that returns EmptyResultSet as a top-level ANDed where condition. Thus, the query would not get executed at all because compiler.py would see there is no potential for any results. Can't find the code anywhere, maybe it is hidden in my home machine's git repo... Another way to get the same result would be to just add an attribute to sql/query/QuerySet called "empty".

The change above should make models/query.py a lot more DRY (whack away all the EmptyQuerySet stub methods). The cost would be somewhat slower execution of operations after .none(), as the code would actually record the operations done, even if they never have any effect to anything. If I recall correctly what I did passed all tests.

I haven't verified this ticket. But I think the above solution could be a easy way to avoid this bug and at the same time remove some code.

comment:2 by Anssi Kääriäinen, 12 years ago

Cc: Anssi Kääriäinen added

comment:3 by Anssi Kääriäinen, 12 years ago

I don't see how what you describe could happen on trunk version. EmptyQuerySet.annotate() clearly does nothing.

I have attached a patch implementing EmptyQuerySet removal. I am not at all sure it is a good idea, but at least it whacks a good 200 lines of code away.

by Anssi Kääriäinen, 12 years ago

Attachment: emptyqs_removal.diff added

comment:4 by Anssi Kääriäinen, 12 years ago

Easy pickings: set
Resolution: worksforme
Status: newclosed

I am going to close this ticket as worksforme. The reason is that I can't see any way a qs.none().annotate() could lead to a query in the current, or 1.3.1 implementation.

If I am incorrect, please provide more details so that it is possible to confirm and reproduce the problem.

I will let the attached EmptyQuerySet patch rest in peace. If somebody feels like it is a good idea, then lets create another ticket for that and resurrect the patch.

in reply to:  4 comment:5 by Carl Meyer, 12 years ago

Replying to akaariai:

I will let the attached EmptyQuerySet patch rest in peace. If somebody feels like it is a good idea, then lets create another ticket for that and resurrect the patch.

I think it's a good idea, and worth its own ticket.

comment:6 by Anssi Kääriäinen, 11 years ago

Component: ORM aggregationDatabase layer (models, ORM)
Note: See TracTickets for help on using tickets.
Back to Top