Opened 5 years ago

Closed 5 years ago

Last modified 4 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 5 years ago.

Download all attachments as: .zip

Change History (7)

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

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset

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

Cc: Anssi Kääriäinen added

comment:3 Changed 5 years ago by Anssi Kääriäinen

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.

Changed 5 years ago by Anssi Kääriäinen

Attachment: emptyqs_removal.diff added

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

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.

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

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

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