Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#21432 closed Bug (fixed)

datetimes method always raise AttributeError

Reported by: Enrique Martínez <enrique@…> Owned by: Aymeric Augustin
Component: Core (Other) Version: 1.6
Severity: Release blocker Keywords:
Cc: Tim Graham, Baptiste Mispelon, loic@… 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

NewsItem.objects.all().datetimes('date_time', 'month', order='DESC') always raises: AttributeError: 'DateTimeQuery' object has no attribute 'tzinfo'

and it happens with all my models with some DateTimeField.

Attachments (1)

tests-21432.diff (959 bytes ) - added by Baptiste Mispelon 10 years ago.
Failing testcase

Download all attachments as: .zip

Change History (14)

comment:1 by Tim Graham, 10 years ago

Cc: Tim Graham added
Component: UncategorizedCore (Other)
Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

I can reproduce this on the tutorial with the following query: Poll.objects.datetimes('pub_date', 'month') (Python 2.7, pytz 2013.8, settings.USE_TZ=True). Tentatively marking as a release blocker as I don't see anything wrong with that based on the documentation. On the other hand, we have tests that seem to test the same thing, so wihout digging in I'm unsure what the issue might be.

comment:2 by Tim Graham, 10 years ago

>>> Poll.objects.datetimes('pub_date', 'month')
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/home/timgraham/code/django/django/db/models/query.py", line 115, in __repr__
    data = list(self[:REPR_OUTPUT_SIZE + 1])
  File "/home/timgraham/code/django/django/db/models/query.py", line 140, in __iter__
    self._fetch_all()
  File "/home/timgraham/code/django/django/db/models/query.py", line 962, in _fetch_all
    self._result_cache = list(self.iterator())
  File "/home/timgraham/code/django/django/db/models/sql/compiler.py", line 1089, in results_iter
    datetime = timezone.make_aware(datetime, self.query.tzinfo)
AttributeError: 'DateTimeQuery' object has no attribute 'tzinfo'

comment:3 by Baptiste Mispelon, 10 years ago

Bisecting the error lead me to commit 70679243d1786e03557c28929f9762a119e3ac14, but it's not really obvious (to me anyway) why that would have broken the feature.

comment:4 by Anssi Kääriäinen, 10 years ago

It is possible the above commit just makes the error loud instead of hiding the error and returning silently []. Before the commit errors inside queryset iteration led to empty list as result.

in reply to:  4 comment:5 by Baptiste Mispelon, 10 years ago

Cc: Baptiste Mispelon added

Replying to akaariai:

It is possible the above commit just makes the error loud instead of hiding the error and returning silently []. Before the commit errors inside queryset iteration led to empty list as result.

Indeed, this appears to be the case.

While digging, I also found out that the following code works:

qs = Poll.objects.datetimes('pub_date', 'month')
list(qs)
repr(qs)

While this one doesn't (note how repr and list have been switched):

qs = Poll.objects.datetimes('pub_date', 'month')
repr(qs)
list(qs)

This explains why this issue has been able to fall through the cracks: iterating over a queryset is a much more common use-case than displaying it as-is.

Unfortunately, I haven't been able to identify exactly where the problem lies (I have a suspicion that it's got something to do with the _clone method though) but I've managed to come up with a failing testcase (see attached).

by Baptiste Mispelon, 10 years ago

Attachment: tests-21432.diff added

Failing testcase

comment:6 by Anssi Kääriäinen, 10 years ago

Yeah, the problem seem to be that when doing qs.datetimes() the DateTimeQuery gets a tzinfo attribute (datetimes() end up in here: https://github.com/django/django/blob/master/django/db/models/query.py#L1255), but if the query gets cloned afterwards the tzinfo attribute will not get cloned. The solution seems to be to write a custom .clone() for DateTimeQuery.

comment:7 by loic84, 10 years ago

Cc: loic@… added
Has patch: set

comment:8 by Aymeric Augustin, 10 years ago

Owner: changed from nobody to Aymeric Augustin
Status: newassigned

The patch looks appropriate.

The test could be simplified a bit with now = timezone.now().replace(microsecond=0).

comment:9 by Baptiste Mispelon, 10 years ago

Triage Stage: AcceptedReady for checkin

Patch looks good.

It fixes the issue and the full test suite still passes after applying it.

comment:10 by Baptiste Mispelon <bmispelon@…>, 10 years ago

Resolution: fixed
Status: assignedclosed

In 17ed99f3a3eea4bd27fa34be59c3582616ed8079:

Fixed #21432 -- DateTimeQuery now copies tzinfo when cloning.

Thanks Enrique Martínez for the report and @bmispelon for the tests.

comment:11 by Baptiste Mispelon <bmispelon@…>, 10 years ago

In 67c30426c1370f5d6c39bd73888c3902c1c5f365:

[1.6.x] Fixed #21432 -- DateTimeQuery now copies tzinfo when cloning.

Thanks Enrique Martínez for the report and @bmispelon for the tests.

Backport of 17ed99f3a3eea4bd27fa34be59c3582616ed8079 from master.

comment:12 by Loic Bistuer <loic.bistuer@…>, 10 years ago

In 32e75803be8b3e9c35e6735c265125c4130ffabc:

Fixed typo and slightly improved error message when db is missing time zone definitions.

Refs #21432.

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

In 3bb7de8c7c3f9d5de5426d94f45fe83b5d998357:

[1.6.x] Fixed typo and slightly improved error message when db is missing time zone definitions.

Refs #21432.

Backport of 32e75803be from master

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