Opened 18 months ago

Closed 18 months ago

Last modified 16 months ago

#21432 closed Bug (fixed)

datetimes method always raise AttributeError

Reported by: Enrique Martínez <enrique@…> Owned by: aaugustin
Component: Core (Other) Version: 1.6
Severity: Release blocker Keywords:
Cc: timo, bmispelon, 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 bmispelon 18 months ago.
Failing testcase

Download all attachments as: .zip

Change History (14)

comment:1 Changed 18 months ago by timo

  • Cc timo added
  • Component changed from Uncategorized to Core (Other)
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Severity changed from Normal to Release blocker
  • Triage Stage changed from Unreviewed to Accepted

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 Changed 18 months ago by timo

>>> 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 Changed 18 months ago by bmispelon

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 follow-up: Changed 18 months ago by 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.

comment:5 in reply to: ↑ 4 Changed 18 months ago by bmispelon

  • Cc bmispelon 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).

Changed 18 months ago by bmispelon

Failing testcase

comment:6 Changed 18 months ago by akaariai

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 Changed 18 months ago by loic84

  • Cc loic@… added
  • Has patch set

comment:8 Changed 18 months ago by aaugustin

  • Owner changed from nobody to aaugustin
  • Status changed from new to assigned

The patch looks appropriate.

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

comment:9 Changed 18 months ago by bmispelon

  • Triage Stage changed from Accepted to Ready for checkin

Patch looks good.

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

comment:10 Changed 18 months ago by Baptiste Mispelon <bmispelon@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In 17ed99f3a3eea4bd27fa34be59c3582616ed8079:

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

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

comment:11 Changed 18 months ago by Baptiste Mispelon <bmispelon@…>

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 Changed 18 months ago by Loic Bistuer <loic.bistuer@…>

In 32e75803be8b3e9c35e6735c265125c4130ffabc:

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

Refs #21432.

comment:13 Changed 16 months ago by Tim Graham <timograham@…>

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