Code

Opened 6 years ago

Closed 12 months ago

#6785 closed Cleanup/optimization (fixed)

QuerySet.get() should only attempt to fetch a limited number of rows

Reported by: deadwisdom Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords:
Cc: timograham@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

.get() selects and lists every record in the db matching the given filter, when it only needs to select two at most.

Attachments (1)

query-get-patch.diff (1.8 KB) - added by deadwisdom 6 years ago.
QuerySet.get() nit-pick / optimize

Download all attachments as: .zip

Change History (7)

Changed 6 years ago by deadwisdom

QuerySet.get() nit-pick / optimize

comment:1 Changed 6 years ago by deadwisdom

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Found a small bug that slipped through the tests, so I fixed it and also updated the tests to test .get() raising MultipleObjectsReturned.

comment:2 Changed 6 years ago by mtredinnick

  • Resolution set to wontfix
  • Status changed from new to closed

Not worth the extra overhead in this performance critical piece of code. In the case when you're calling get correctly, only a single result will be returned and we need that result. In the error case, it's not harmful that we're selecting a few extra results. Any code that is relying on the error case for speed is broken by design.

comment:3 Changed 13 months ago by wim@…

  • Easy pickings unset
  • Severity set to Normal
  • Type set to Uncategorized
  • UI/UX unset

Strange though it may seem: would it be possible to explicitly state the number of objects returned and restrain that number to 1, 2, 3, 4, 5, more than 5? In my experience, the number of objects is helpful when debugging.

comment:4 Changed 13 months ago by timo

  • Cc timograham@… added
  • Component changed from Uncategorized to Database layer (models, ORM)
  • Keywords qs-rf removed
  • Patch needs improvement set
  • Summary changed from QuerySet.get(), nit-pick / optimize to QuerySet.get() should only attempt to fetch a limited number of rows
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Uncategorized to Cleanup/optimization
  • Version changed from queryset-refactor to master

Seems like there's consensus to re-open this: https://groups.google.com/d/topic/django-developers/PkzS9Wv6hIU/discussion

Pull request (which needs improvement): https://github.com/django/django/pull/1139

comment:5 Changed 13 months ago by timo

  • Patch needs improvement unset
  • Resolution wontfix deleted
  • Status changed from closed to new

Updated pull request based on django-developers discussion: https://github.com/django/django/pull/1320

I'm not sure it needs docs or a mention in the release notes, but happy to add them if necessary.

comment:6 Changed 12 months ago by Tim Graham <timograham@…>

  • Resolution set to fixed
  • Status changed from new to closed

In da79ccca1d34f427952cce4555e598a700adb8de:

Fixed #6785 -- Made QuerySet.get() fetch a limited number of rows.

Thanks Patryk Zawadzki.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.