Opened 3 years ago

Closed 3 years ago

#19360 closed Bug (fixed)

annotate the sum of related TimeField

Reported by: lsaffre Owned by: chrismedrela
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords:
Cc: chrismedrela 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

I get a "TypeError: expected string or buffer" when I try to annotate the sum of a timefield in a related model.

class Ticket(models.Model):  
    name = models.CharField(max_length=200)

class Session(models.Model):  
    ticket = models.ForeignKey(Ticket,related_name="sessions")
    time = models.TimeField()

qs = Ticket.objects.annotate(timesum=models.Sum('sessions__time'))
print [unicode(t.timesum) for t in qs]

Please see the attached files for a complete test case.

Attachments (2)

20121124.zip (1.6 KB) - added by lsaffre 3 years ago.
2 files models.py and settings.py
19360_v1.diff (2.3 KB) - added by chrismedrela 3 years ago.

Download all attachments as: .zip

Change History (10)

Changed 3 years ago by lsaffre

2 files models.py and settings.py

comment:1 Changed 3 years ago by chrismedrela

  • Cc chrismedrela added
  • Component changed from Uncategorized to Database layer (models, ORM)
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Uncategorized to Bug
  • Version changed from 1.4 to master

comment:2 Changed 3 years ago by chrismedrela

  • Owner changed from nobody to chrismedrela

The problem is that django.utils.dateparse.parse_time function is called with float argument instead of string. Then the argument is passed on to re_pattern.match function.

comment:3 Changed 3 years ago by chrismedrela

The problem is that there is no native time field in sqlite. This feature is emulated by Django -- the time is saved as text (HH:MM:SS). The query

Ticket.objects.annotate(timesum=models.Sum('sessions__time'))

is translated into the following SQL statement:

SELECT ..., SUM("polls_session"."time") AS "timesum" FROM "polls_ticket" LEFT OUTER JOIN ...

The sum expression doesn't make sense since "polls_session"."time" is a text column, but it fails silently -- the sum returned by sqlite3 driver is 0.0 float. Then Django try to parse the result as a time using re module, but the result it's not even string which causes raising the exception.

What can we do? We can use integer column representing unix time to emulate date/time fields, but it's lot of work and it breaks backward compatibility. We should at least add note to documentation. Django should fail loudly when you try to aggregate date/time fields in sqlite. Any other ideas?

Changed 3 years ago by chrismedrela

comment:4 Changed 3 years ago by chrismedrela

  • Has patch set

I've written a patch (and created pull request -- see https://github.com/django/django/pull/579). It raises an explicit exception if you aggregate on date/time field in sqlite3. Are tests required for fixes like that?

comment:5 Changed 3 years ago by claudep

  • Needs tests set

Yes, tests are required. Thanks for the patch.

comment:6 Changed 3 years ago by slurms

  • Needs tests unset

Added tests and documentation. New pull request here: https://github.com/django/django/pull/637.

comment:7 Changed 3 years ago by claudep

  • Triage Stage changed from Accepted to Ready for checkin

comment:8 Changed 3 years ago by Claude Paroz <claude@…>

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

In eb6c107624930c97390185fdbf7f887c50665808:

Fixed #19360 -- Raised an explicit exception for aggregates on date/time fields in sqlite3

Thanks lsaffre for the report and Chris Medrela for the initial
patch.

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