Opened 9 years ago

Closed 7 years ago

Last modified 7 years ago

#25240 closed New feature (fixed)

Add "week" lookup to DateField/DateTimeField

Reported by: Chris Foresman Owned by: Mads Jensen
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords: QuerySet.extra
Cc: tarkatronic@…, borfast@…, josh.smeaton@…, code+django@… 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

Per the recommendation here, filling a ticket requesting that a "give me all items for the current week" option be added to the ORM to filter DateTime fields. I'm using the following extra WHERE clause to achieve this:

where=['EXTRACT(WEEK FROM timestamp) = EXTRACT(WEEK FROM CURRENT_TIMESTAMP)'])

Change History (11)

comment:1 by Tim Graham, 9 years ago

Summary: Current week query for DateTime fieldsAdd "week" lookup to DateField/DateTimeField
Triage Stage: UnreviewedAccepted
Type: UncategorizedNew feature
Version: 1.8master

comment:2 by Josh Smeaton, 9 years ago

I'm fairly sure that this can be implemented in 1.9 with bilateral transformations. 1.9 is only required because that is when Now() was added. If you'd like to copy the implementation of Now() into your current codebase, then you could get it to work in 1.8.

from django.db.models.fields import DateTransform
from django.db.models.functions import Now

class WeekTransform(DateTransform):
    lookup_name = 'week'
    bilateral = True

Model.objects.filter(timestamp__week__exact=Now())

So there's an argument to be made that WeekTransform could be added to core, but without the bilateral argument in order to remain consistent with the rest of the Date based transforms. I'm currently working on a patch (#24629) that would allow the transform to be used explicitly on the right hand side:

class WeekTransform(DateTransform):
    lookup_name = 'week'

Model.objects.filter(timestamp__week__exact=WeekTransform(Now()))

comment:3 by Joey Wilhelm, 9 years ago

Cc: tarkatronic@… added

I just tried creating such a lookup (by subclassing DateLookup) but I ran into a brick wall in django.db.models.fields. The lookup itself is very simple, emulating precisely how the 'year', 'day', etc lookups are done:

from django.db.models import DateTimeField
from django.db.models.lookup import DateLookup

@DateTimeField.register_lookup
class WeekLookup(DateLookup):
    lookup_name = 'week'
    extract_type = 'week'

Simple enough. The problem, however, comes from django/db/models/fields/__init__.py:

    def get_prep_lookup(self, lookup_type, value):
        # For dates lookups, convert the value to an int
        # so the database backend always sees a consistent type.
        if lookup_type in ('month', 'day', 'week_day', 'hour', 'minute', 'second'):
            return int(value)
        return super(DateField, self).get_prep_lookup(lookup_type, value)

There are two other such special cases in that file. Perhaps I'm just being a bit pedantic, but this seems to violate PEP 20:

Special cases aren't special enough to break the rules.

Shy of subclassing DateTimeField, it would seem that there's no way, at present, to use this lookup.

Additionally, as a note on jarshwah's suggestion, you would also have to copy in the implementation of DateTransform, since that isn't present in 1.8 either. A complete implementation of the suggestion looks like:

from django.conf import settings
from django.db.models.fields import DateField, DateTimeField, IntegerField, TimeField
from django.db.models.functions import Func
from django.db.models.lookups import Transform
from django.utils import timezone
from django.utils.functional import cached_property


class Now(Func):
    template = 'CURRENT_TIMESTAMP'

    def __init__(self, output_field=None, **extra):
        if output_field is None:
            output_field = DateTimeField()
        super(Now, self).__init__(output_field=output_field, **extra)

    def as_postgresql(self, compiler, connection):
        # Postgres' CURRENT_TIMESTAMP means "the time at the start of the
        # transaction". We use STATEMENT_TIMESTAMP to be cross-compatible with
        # other databases.
        self.template = 'STATEMENT_TIMESTAMP()'
        return self.as_sql(compiler, connection)


class DateTransform(Transform):
    def as_sql(self, compiler, connection):
        sql, params = compiler.compile(self.lhs)
        lhs_output_field = self.lhs.output_field
        if isinstance(lhs_output_field, DateTimeField):
            tzname = timezone.get_current_timezone_name() if settings.USE_TZ else None
            sql, tz_params = connection.ops.datetime_extract_sql(self.lookup_name, sql, tzname)
            params.extend(tz_params)
        elif isinstance(lhs_output_field, DateField):
            sql = connection.ops.date_extract_sql(self.lookup_name, sql)
        elif isinstance(lhs_output_field, TimeField):
            sql = connection.ops.time_extract_sql(self.lookup_name, sql)
        else:
            raise ValueError('DateTransform only valid on Date/Time/DateTimeFields')
        return sql, params

    @cached_property
    def output_field(self):
        return IntegerField()


@DateTimeField.register_lookup
class WeekTransform(DateTransform):
    lookup_name = 'week'
    bilateral = True

This does work with the suggested usage:

Model.objects.filter(timestamp__week__exact=Now())

However, there is one major limitation. When you do the comparison against a week number, as you would do with the other date part comparisons, you get an AttributeError: 'QueryWrapper' object has no attribute 'output_field'. And when comparing to a full datetime object, you get TypeError: int() argument must be a string, a bytes-like object or a number, not 'datetime.datetime'.

So long story short, the transform approach does work, but does not stay consistent with the existing lookups, and adding a "__week" lookup will require modification of django/db/models/fields/__init__.py

comment:4 by Raúl Pedro Santos, 8 years ago

Cc: borfast@… added

comment:5 by Josh Smeaton, 8 years ago

Cc: josh.smeaton@… added

comment:6 by Mads Jensen, 7 years ago

Has patch: set
Patch needs improvement: set

WIP PR

Last edited 7 years ago by Tim Graham (previous) (diff)

comment:7 by Mads Jensen, 7 years ago

Owner: changed from nobody to Mads Jensen
Patch needs improvement: unset
Status: newassigned

comment:8 by Rivo Laks, 7 years ago

Cc: code+django@… added
Triage Stage: AcceptedReady for checkin

I have no further comments to the PR (https://github.com/django/django/pull/7447), LGTM.

comment:9 by Tim Graham <timograham@…>, 7 years ago

Resolution: fixed
Status: assignedclosed

In 1446902b:

Fixed #25240 -- Added ExtractWeek and exposed it through the week lookup.

Thanks to Mariusz Felisiak and Tim Graham for review.

comment:10 by Tim Graham <timograham@…>, 7 years ago

In 085c2f94:

Refs #25240 -- Added ExtractWeek examples.

comment:11 by Tim Graham <timograham@…>, 7 years ago

In 319839d7:

[1.11.x] Refs #25240 -- Added ExtractWeek examples.

Backport of 085c2f94ec0155417601a9750ad60bb93536e166 from master

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