Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#25774 closed New feature (fixed)

Refactor of datetime expressions and better, official support for right-hand-side date part extraction

Reported by: David Filipovic Owned by: Josh Smeaton
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords: db, expressions, date, time, extract, transform 1.10
Cc: josh.smeaton@…, info+coding@…, aksheshdoshi@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by David Filipovic)

As previously stated in this ticket and this thread, I have begun work on the following:

  • making the so called datetime transforms (currently residing in django.db.models.lookups) part of the officially supported ORM API as part of the db expressions,
  • making these transforms easily usable on the right hand side of lookups / Q objects


To this effect, I propose the following:

  • replace module django.db.models.expressions with a package of the same name
  • add module django.db.models.expressions.datetimes
  • rename XTransform to XExtract and move them from django.db.models.lookups to django.db.models.lookups to django.db.models.expressions.datetimes
  • change YearLookup and YearComparisonLookup to allow for functions on the right hand side
  • extensive testing of these db expressions
  • adding this to the release notes
  • any other change that might support this


I'm also adding a PR with some of these already implemented. What remains to be done is:

  • replace references XTransform type classes in django.db.models.lookups with XExtract classes from django.db.models.expressions.datetimes
  • replace these references anywhere else in the project (I've noticed there are some in the tests, for instance)


With the API in the PR it becomes possible to do the following lookup:

from django.db.models.expressions.datetimes import DateExtract

Person.objects.filter(birth_date__year=DateExtract('job__start_date', lookup_name='year'))

which is equivalent to:

from django.db.models.expressions.datetimes import YearExtract

Person.objects.filter(birth_date__year=YearExtract('job__start_date'))

Additionally, @jarshwah suggested that these should maybe be named: ExtractX instead of XExtract.

Change History (19)

comment:1 by David Filipovic, 8 years ago

Description: modified (diff)
Needs documentation: set
Needs tests: set

comment:2 by Josh Smeaton, 8 years ago

Triage Stage: UnreviewedAccepted

I think you're on the right track for most of these points. I'm not yet convinced that adding DateExtract is necessary, but once the patch is written I'll take another look then. I just worry that there'll be two ways to extract a date part and that may lead to confusion.

Would like others opinions on ExtractX rather than XExtract. I think it reads and sorts better.

YearLookup and YearComparisonLookup are going to be interesting. They'll still need to work the same way when used on the left hand side. Perhaps we can rename to YearLookupOptimised (for example, that's a bad name) and register that as a lookup, but provide a different ExtractYear that does what you'd expect on the right hand side. The optimised version would then just be a hidden implementation detail that users shouldn't touch themselves.

I'll have a look at the actual PR tomorrow. Great work so far.

comment:3 by David Filipovic, 8 years ago

Josh, I've actually tweaked YearLookup and YearComparisonLookup already.

I've reversed the order of process_lhs and process_rhs to make process_rhs occur first, and then based on the result of that processing, specifically, based on whether there are any rhs_params returned, the lhs processing branches to accommodate right hand side based on both a parameter and a function.

comment:4 by Markus Holtermann, 8 years ago

Cc: info+coding@… added

comment:5 by Josh Smeaton, 8 years ago

Owner: changed from nobody to Josh Smeaton
Status: newassigned

Since David appears to have left this ticket I'll pick this up. Initial patch: https://github.com/django/django/pull/6243

comment:6 by Josh Smeaton, 8 years ago

Needs documentation: unset
Needs tests: unset
Triage Stage: AcceptedReady for checkin

Patch is ready for review.

comment:7 by Tim Graham, 8 years ago

Triage Stage: Ready for checkinAccepted

The review queue is "Accepted + Has patch".

comment:8 by Akshesh Doshi, 8 years ago

Cc: aksheshdoshi@… added

comment:9 by Tim Graham, 8 years ago

Patch needs improvement: set

comment:10 by Tim Graham, 8 years ago

Patch needs improvement: unset

comment:11 by Tim Graham, 8 years ago

Keywords: 1.10 added
Patch needs improvement: set

Left comments for improvement on the PR.

comment:12 by Josh Smeaton, 8 years ago

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

Ready for merge unless feature required for timezone support requiring pytz as mentioned https://github.com/django/django/pull/6243#issuecomment-219282120

comment:13 by Josh Smeaton, 8 years ago

Triage Stage: Ready for checkinAccepted

I added another commit to provide tzinfo support for Extract subclasses. New commit is: https://github.com/django/django/pull/6243/commits/4f08553fb0431184fdcc3e0eb9c3c52fa2bba09e

comment:14 by Tim Graham, 8 years ago

Triage Stage: AcceptedReady for checkin

comment:15 by Josh Smeaton <josh.smeaton@…>, 8 years ago

In 77b73e79:

Refs #25774 -- Made Oracle truncate microseconds if USE_TZ=False.

The tests for this change are in the fix for #25774.

comment:16 by Josh Smeaton <josh.smeaton@…>, 8 years ago

Resolution: fixed
Status: assignedclosed

In 2a4af0ea:

Fixed #25774 -- Refactor datetime expressions into public API

comment:17 by Simon Charette <charette.s@…>, 8 years ago

In 9046807:

Refs #25774 -- Adjusted datetime database function docs field names.

comment:18 by Simon Charette <charette.s@…>, 8 years ago

In 082c52db:

Refs #25774, #26348 -- Allowed Trunc functions to operate with time fields.

Thanks Josh for the amazing testing setup and Tim for the review.

comment:19 by Simon Charette <charette.s@…>, 8 years ago

In cf6f0e9:

[1.10.x] Refs #25774 -- Adjusted datetime database function docs field names.

Backport of 90468079ec6f72a1b8b6a908d81d3201a3204b24 from master

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