Opened 3 years ago

Closed 3 years ago

Last modified 3 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 3 years ago.
Failing testcase

Download all attachments as: .zip

Change History (14)

comment:1 Changed 3 years ago by Tim Graham

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 Changed 3 years ago by Tim Graham

>>> 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 3 years ago by Baptiste Mispelon

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 Changed 3 years ago by Anssi Kääriäinen

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 3 years ago by Baptiste Mispelon

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).

Changed 3 years ago by Baptiste Mispelon

Attachment: tests-21432.diff added

Failing testcase

comment:6 Changed 3 years ago by Anssi Kääriäinen

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 3 years ago by loic84

Cc: loic@… added
Has patch: set

comment:8 Changed 3 years ago by Aymeric Augustin

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 Changed 3 years ago by Baptiste Mispelon

Triage Stage: AcceptedReady for checkin

Patch looks good.

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

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

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 Changed 3 years 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 3 years 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 3 years 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