Opened 11 years ago

Closed 11 years ago

#19326 closed New feature (fixed)

Add a first method to the QuerySet

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

Description

Many times, get() is being used inside a try except Model.DoesNotExist clause. In order to save us lines of code and in order to prevent the Exception becoming common, a discussion followed which can be found here:

https://groups.google.com/forum/?fromgroups=#!searchin/django-developers/first$20jacob/django-developers/3RwDxWKPZ_A/discussion

According to our BDFL:

After thinking a bit, I'm +1 on the idea, and I think
Queryset.first() is the right name (which implies we should probably
have a last() for completion).

This mirrors the name used by a few other ORM/collection APIs that I
think have really nice APIS:

http://documentcloud.github.com/underscore/#first)

If someone produces a patch I'll review it and commit it.

Jacob

Attachments (2)

19326.diff (2.9 KB ) - added by wim@… 11 years ago.
Patch for ticket 19326: added first. Unfortunately, my implementation fails
19326_2.diff (2.8 KB ) - added by wim@… 11 years ago.
My second take, using filter and IndexError as suggested

Download all attachments as: .zip

Change History (12)

by wim@…, 11 years ago

Attachment: 19326.diff added

Patch for ticket 19326: added first. Unfortunately, my implementation fails

comment:1 by anonymous, 11 years ago

Something like this should work:

try:
    return self.filter(*args, **kwargs)[0]
except IndexError:
    return None

I am not sure if the IndexError is correct error type...

If .last() is needed then that is going to be a bit more problematic as you will need to revert all of the ordering clauses of the query.

by wim@…, 11 years ago

Attachment: 19326_2.diff added

My second take, using filter and IndexError as suggested

comment:2 by Matt Austin, 11 years ago

Triage Stage: UnreviewedAccepted

comment:3 by bleskes, 11 years ago

Triage Stage: AcceptedReady for checkin

Looks good to me.

comment:4 by Mike Fogel, 11 years ago

Cc: mike@… added

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

Some polish done, available from https://github.com/akaariai/django/compare/ticket_19326

Biggest changes are addition of .last() method for symmetry, and automatic ordering by PK if there is no ordering defined otherwise. In addition some docs changes and move of the tests to new test module first_and_last.

I will still ask at django-developers if the API is the wanted one, otherwise this should be 100% ready for commit.

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

Triage Stage: Ready for checkinDesign decision needed

I'm setting this to DDN. If these methods were added, then the API would be confusing. .latest() vs .last() and .earliest() vs .first() will be confusing.

I still think it would be nice to have some way to get None instead of DoesNotExist if the None is expected, not exceptional value. However adding anything that is confusing is a bad idea.

Another API idea: .default(None).get(...), .default(someobj).latest() or .default(None).order_by('somecol')[0] could all make the ORM return None or any other given value instead of DoesNotExist. Does this sound sensible? Naming is of course tentative, for example .default_on_empty() could be better if somewhat long...

comment:7 by Jacob, 11 years ago

Triage Stage: Design decision neededAccepted

I haven't seen a proposal addressing the first/last vs earliest/latest confusion that isn't even *more* confusing, so I'm marking this accepted. Let's just do first/last + earliest/latest and live with the slight redundancy.

comment:8 by Selwin Ong, 11 years ago

I made a pull request implementing these features here: https://github.com/django/django/pull/1056 . Let me know if there's anything else that I need to change.

comment:9 by Mike Fogel, 11 years ago

How about adding a test that first() and last() work as expected on empty querysets? Other than that the patch looks good to me.

comment:10 by Anssi Kääriäinen <akaariai@…>, 11 years ago

Resolution: fixed
Status: newclosed

In ea9a0857d4922fab1f9146f3a7828b67281edc89:

Fixed #19326 -- Added first() and last() methods to QuerySet

Note: See TracTickets for help on using tickets.
Back to Top