Opened 2 years ago

Closed 2 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: master
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@… 2 years ago.
Patch for ticket 19326: added first. Unfortunately, my implementation fails
19326_2.diff (2.8 KB) - added by wim@… 2 years ago.
My second take, using filter and IndexError as suggested

Download all attachments as: .zip

Change History (12)

Changed 2 years ago by wim@…

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

comment:1 Changed 2 years ago by anonymous

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

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.

Changed 2 years ago by wim@…

My second take, using filter and IndexError as suggested

comment:2 Changed 2 years ago by mattaustin

  • Triage Stage changed from Unreviewed to Accepted

comment:3 Changed 2 years ago by bleskes

  • Triage Stage changed from Accepted to Ready for checkin

Looks good to me.

comment:4 Changed 2 years ago by carbonXT

  • Cc mike@… added

comment:5 Changed 2 years ago by akaariai

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

  • Triage Stage changed from Ready for checkin to Design 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 Changed 2 years ago by jacob

  • Triage Stage changed from Design decision needed to Accepted

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

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

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 Changed 2 years ago by Anssi Kääriäinen <akaariai@…>

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

In ea9a0857d4922fab1f9146f3a7828b67281edc89:

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

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