Opened 5 years ago

Closed 2 months ago

#16487 closed Bug (fixed)

F expression with timedelta does not work with __range query filter

Reported by: Ben Davis Owned by: nobody
Component: Database layer (models, ORM) Version: 1.3
Severity: Normal Keywords:
Cc: Ben Davis, Matthew Wilkes Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Works:

User.objects.filter(last_login__gt=F('date_joined')+timedelta(weeks=2))

Doesn't work:

User.objects.filter(last_login__range=(F('date_joined'), F('date_joined')+timedelta(weeks=2)))

Change History (7)

comment:1 Changed 5 years ago by Aymeric Augustin

Triage Stage: UnreviewedAccepted

Indeed. Here's the traceback:

>>> from datetime import timedelta
>>> from django.contrib.auth.models import *
>>> from django.db.models import F
>>> User.objects.filter(last_login__gt=F('date_joined')+timedelta(weeks=2))
[<User: admin>]
>>> User.objects.filter(last_login__range=(F('date_joined'), F('date_joined')+timedelta(weeks=2)))
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "path/to/venv/lib/python2.6/site-packages/django/db/models/manager.py", line 141, in filter
    return self.get_query_set().filter(*args, **kwargs)
  File "path/to/venv/lib/python2.6/site-packages/django/db/models/query.py", line 550, in filter
    return self._filter_or_exclude(False, *args, **kwargs)
  File "path/to/venv/lib/python2.6/site-packages/django/db/models/query.py", line 568, in _filter_or_exclude
    clone.query.add_q(Q(*args, **kwargs))
  File "path/to/venv/lib/python2.6/site-packages/django/db/models/sql/query.py", line 1172, in add_q
    can_reuse=used_aliases, force_having=force_having)
  File "path/to/venv/lib/python2.6/site-packages/django/db/models/sql/query.py", line 1107, in add_filter
    connector)
  File "path/to/venv/lib/python2.6/site-packages/django/db/models/sql/where.py", line 67, in add
    value = obj.prepare(lookup_type, value)
  File "path/to/venv/lib/python2.6/site-packages/django/db/models/sql/where.py", line 316, in prepare
    return self.field.get_prep_lookup(lookup_type, value)
  File "path/to/venv/lib/python2.6/site-packages/django/db/models/fields/__init__.py", line 644, in get_prep_lookup
    return super(DateField, self).get_prep_lookup(lookup_type, value)
  File "path/to/venv/lib/python2.6/site-packages/django/db/models/fields/__init__.py", line 294, in get_prep_lookup
    return [self.get_prep_value(v) for v in value]
  File "path/to/venv/lib/python2.6/site-packages/django/db/models/fields/__init__.py", line 721, in get_prep_value
    return self.to_python(value)
  File "path/to/venv/lib/python2.6/site-packages/django/db/models/fields/__init__.py", line 710, in to_python
    raise exceptions.ValidationError(self.error_messages['invalid'])
ValidationError: [u'Enter a valid date/time in YYYY-MM-DD HH:MM[:ss[.uuuuuu]] format.']

comment:2 Changed 4 years ago by anonymous

This should be supported. most databases these days either have support for time intervals or time math.

comment:3 Changed 2 years ago by Ben Davis

Cc: Ben Davis added

comment:4 Changed 9 months ago by Matthew Wilkes

See also: #22288

comment:5 Changed 2 months ago by Tim Graham

Cc: Matthew Wilkes added

Looks like #22288 fixed this. Matthew, do you think we should a test for this case or are the tests from 4f138fe5a496a81115c4fba6615a517fc62c3b17 sufficient?

comment:6 Changed 2 months ago by Matthew Wilkes

My feeling is that the current tests are sufficient, considering https://github.com/django/django/blob/e43ea36b7681e43ea99505a2cf7550d4d36016b3/tests/expressions/tests.py#L801 includes tests about adding timedelta objects to F() expressions. The range lookup doesn't do anything special with expressions, so anything that works individually should work inside the lookup, as long as the SQL generated by the lookup isn't fragile to having expressions as its operands.

That said, I did include explicit tests for adding to the integer fields in my PR, so obviously at some point I thought demonstrating this was valuable, so I'd be happy to submit an amendment if you're leaning towards explicitly checking them.

comment:7 Changed 2 months ago by Tim Graham

Resolution: fixed
Status: newclosed

I trust your judgment, thanks!

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