Opened 10 years ago
Closed 9 years ago
#25556 closed New feature (duplicate)
Add DatePart db expression to allow complex lookups on date parts (e.g. year, month, day)
| Reported by: | David Filipovic | Owned by: | David Filipovic |
|---|---|---|---|
| Component: | Database layer (models, ORM) | Version: | dev |
| Severity: | Normal | Keywords: | db expressions |
| Cc: | josh.smeaton@… | Triage Stage: | Accepted |
| Has patch: | yes | Needs documentation: | yes |
| Needs tests: | yes | Patch needs improvement: | yes |
| Easy pickings: | no | UI/UX: | no |
Description
Currently it is not possible to filter by matching the year/month/day part of two separate date fields.
I propose something to the effect of:
from django.db.models.expressions import DatePart
MyModel.objects.filter(date1__month=DatePart('date2', 'month'))
Attachments (2)
Change History (11)
by , 10 years ago
| Attachment: | expressions_patch.py added |
|---|
comment:1 by , 10 years ago
I've added a pull request, but I'm unsure about the tests. I've looked for similar tests for the Date and DateTime expressions and I couldn't really find any. The only existing tests are found in tests.expressions.tests and they're only testing the repr.
comment:2 by , 10 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
| Triage Stage: | Unreviewed → Accepted |
by , 10 years ago
| Attachment: | expressions.diff added |
|---|
comment:3 by , 10 years ago
I was sure there was already a ticket addressing this feature but I can't seem to find it just yet. Accepting based on not being able to find a duplicate.
As to the implementation though, as of Django 1.9 (in alpha right now) transformations such as __month __year etc can be used on the right hand side of lookups. There's no need to create new expressions.
django.db.models.lookups provides DateTransform as well as things like MonthTransform and MinuteTransform. So it's already possible to use private classes to achieve the outcome you're after. These transformations also work correctly with Time/DateTime/Date fields and have timezone support.
from django.db.models.lookups import MonthTransform
MyModel.objects.filter(date1__month=MonthTransform('date2'))
I don't love the class names though and we should definitely document these date based transforms.
So let's:
- rename datetime based
XTransformclasses toX. - document these classes somewhere (in expressions.txt or a new docs module?)
- test that they definitely can be used on the right hand side
- add to release notes
Optional/Undecided:
- Move these classes into a separate submodule and import them from lookups.py to ensure they're registered
- Change DateTransform to also accept a
lookup_namein**kwargssoDateTransform('field', lookup_name'month')will work. This should also be renamed to something more appropriate, likeExtract()?DatePartis also a descriptive name.
Should we allow this into 1.9? It doesn't change any public api as these classes were only introduced for 1.9 in the first place. I would say yes, we should do this for 1.9.
comment:4 by , 10 years ago
| Cc: | added |
|---|---|
| Needs documentation: | set |
| Needs tests: | set |
| Patch needs improvement: | set |
comment:5 by , 10 years ago
I'm not too happy with adding this to 1.9. If we were only to document existing code that would be OK for me, but we are doing a bit more here. Accepting new features because they change so little is a slippery slope.
comment:6 by , 10 years ago
The DateTransform + DatePartTransform classes were all added in 1.9. I would really like to at least change their names to remove the Transform part of the name at a minimum. I agree that adding new features is a slippery slope. How do you feel about renaming these classes during the alpha though?
comment:7 by , 10 years ago
I don't mind the renaming. The pre-release period is for polishing new implementations.
I guess I withdraw my objection about 1.9. We are doing two things here:
- Changing some names. We are going to do this no matter if we add documentation for this.
- Adding the date expressions as a new feature by documenting existing code.
Doing 2. alone doesn't seem too bad. We aren't doing any code changes for the feature. So, I don't object, but I still think we should be extremely careful in not making the feature freeze a debatable issue.
comment:8 by , 10 years ago
| Easy pickings: | unset |
|---|---|
| Version: | 1.9a1 → master |
comment:9 by , 9 years ago
| Resolution: | → duplicate |
|---|---|
| Status: | assigned → closed |
Duplicate of #25774 as far as I can tell.
django.db.models.expressions patch