Opened 3 years ago

Closed 3 years ago

Last modified 2 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: akaariai 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 akaariai 3 years ago.

Download all attachments as: .zip

Change History (7)

comment:1 Changed 3 years ago by akaariai

  • 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 3 years ago by akaariai

  • Cc akaariai added

comment:3 Changed 3 years ago by akaariai

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 3 years ago by akaariai

comment:4 follow-up: Changed 3 years ago by akaariai

  • Easy pickings set
  • Resolution set to worksforme
  • Status changed from new to closed

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 3 years ago by carljm

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 2 years ago by akaariai

  • Component changed from ORM aggregation to Database layer (models, ORM)
Note: See TracTickets for help on using tickets.
Back to Top