[Django] #32699: Comparisons using TruncTime are unreliable in MySQL
Django
-----------------------------------------+------------------------
Reporter: Alex Hill | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 3.2
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 1
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-----------------------------------------+------------------------
To reproduce:
{{{
class DTModel(models.Model):
dt = models.DateTimeField()
obj = DTModel.objects.create(dt=datetime(2021, 4, 30, 10, 30))
DTModel.objects.filter(pk=obj.pk, dt__time=time(10, 30)).count() # should
be 1 but returns 0
}}}
TruncTime is implemented in MySQL using the `TIME()` function. When we
pass time values to MySQL, we return `str(value)` from
`adapt_timefield_value` which, when the microseconds field of the value is
zero, yields a string in "HH:MM:SS" format.
And it looks like comparing time values with strings is problematic and
its success depends on including or not including the microseconds value.
* `SELECT TIME('10:30:00.000000') = '10:30:00'` returns 0
* `SELECT TIME('10:30:00.000000') = '10:30:00.000000'` returns 1
But if you make a TIME column and insert '10:30:00.000000' into it, then
`SELECT ... WHERE TIME(dt) = '10:30:00.000000'` won't find the row. It's a
mess.
The problem I'm seeing can be fixed by returning a time string including
the microseconds component from `adapt_timefield_value`, but I wouldn't be
surprised if that breaks something else given the above. What does appear
to work in all cases is passing the value to TIME() or preceding it with
the TIME keyword.
This affects all current versions of Django. I'll open a PR once I have a
ticket number :)
--
Ticket URL: <https://code.djangoproject.com/ticket/32699>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Alex Hill | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 3.2
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Simon Charette):
* stage: Unreviewed => Accepted
Comment:
> But if you make a `TIME` column and insert `'10:30:00.000000'` into it,
then `SELECT ... WHERE TIME(dt) = '10:30:00.000000'` won't find the row.
It's a mess.
It doesn't happen if you create the column as `TIME(6)`
[https://dev.mysql.com/doc/refman/8.0/en/fractional-seconds.html to get
microseconds precision] though which
[https://github.com/django/django/blob/7582d913e7db7f32e4cdcfafc177aa77cbbf4332/django/db/backends/mysql/base.py#L130
is something Django does].
{{{#!sql
mysql dja...@0.0.0.0:django> CREATE TABLE foo (id int auto_increment
primary key, t TIME, t6 TIME(6));
Query OK, 0 rows affected
Time: 0.017s
mysql dja...@0.0.0.0:django> INSERT INTO foo (id, t, t6) VALUES(0,
'10:30:00.000000', '10:30:00.000000');
Query OK, 1 row affected
Time: 0.009s
mysql dja...@0.0.0.0:django> SELECT t = '10:30:00.000000', t6 =
'10:30:00.000000', TIME(t) = '10:30:00.000000', TIME(t6) =
'10:30:00.000000' FROM foo;
+-----------------------+------------------------+-----------------------------+------------------------------+
| t = '10:30:00.000000' | t6 = '10:30:00.000000' | TIME(t) =
'10:30:00.000000' | TIME(t6) = '10:30:00.000000' |
+-----------------------+------------------------+-----------------------------+------------------------------+
| 1 | 1 | 0
| 1 |
+-----------------------+------------------------+-----------------------------+------------------------------+
1 row in set
Time: 0.009s
}}}
Which is kind of expected as `TIME`
[https://dev.mysql.com/doc/refman/8.0/en/date-and-time-
functions.html#function_time returns a string representation of the
temporal value] and the `TIME` column type doesn't have microsecond
precision and thus doesn't represent them.
I think the patch makes sense since the time you'd be passing a literal
`time` instance would be when
1. Comparing against a `TimeField` column from a model and MySQL work
properly in this case.
2. Against `DateTimeField` transform such as `__time` as reported here
where `TIME` is used and works properly with `DATETIME(6)` which are used
by default since Django 1.8.
--
Ticket URL: <https://code.djangoproject.com/ticket/32699#comment:1>
Django
Reporter: Alex Hill | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 3.2
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Alex Hill):
I appreciate the insight, thanks! Good to hear it shouldn't be more
complicated than the existing patch.
> Which is kind of expected as TIME returns a string representation of the
temporal value
All representations of the same time passed to TIME seem to be considered
equal to one another when compared, even though the strings they evaluate
to may not equal to one another. Can you shed any light?
{{{
mysql> SELECT TIME('10:30:000000') = TIME('10:30');
1
mysql> SELECT TIME('10:30:00.000000'), TIME('10:30');
10:30:00.000000 10:30:00
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32699#comment:2>
Django
Reporter: Alex Hill | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 3.2
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Simon Charette):
Not sure I can explain that, maybe that MySQL wanted preserve the equality
behaviour or `TIME` for columns without microsecond precision when it
introduced support for the later?
{{{#!sql
mysql> SELECT t = TIME('10:30:00.00000'), t = '10:30:00.00000', TIME(t) =
TIME(t6), t = 103000.00, TIME(t) = 103000.00 FROM foo;
+----------------------------+----------------------+--------------------+---------------+---------------------+
| t = TIME('10:30:00.00000') | t = '10:30:00.00000' | TIME(t) = TIME(t6) |
t = 103000.00 | TIME(t) = 103000.00 |
+----------------------------+----------------------+--------------------+---------------+---------------------+
| 1 | 1 | 1 |
1 | 1 |
+----------------------------+----------------------+--------------------+---------------+---------------------+
}}}
Given that `TIME('10:30:000000') = TIME('10:30')` an alternative to the
patch could be always wrap `TIME` comparison in the `TIME` function on
MySQL.
--
Ticket URL: <https://code.djangoproject.com/ticket/32699#comment:3>
Django
Reporter: Alex Hill | Owner: Alex Hill
Type: Bug | Status: assigned
Component: Uncategorized | Version: 3.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* owner: nobody => Alex Hill
* status: new => assigned
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/32699#comment:4>
Django
Reporter: Alex Hill | Owner: Alex Hill
Component: Uncategorized | Version: 3.2
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"c4ee3b208a2c95a5102b5e4fa789b10f8ee29b84" c4ee3b20]:
{{{
#!CommitTicketReference repository=""
revision="c4ee3b208a2c95a5102b5e4fa789b10f8ee29b84"
Fixed #32699 -- Fixed comparing to TruncTime() with 0 microseconds on
MySQL.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32699#comment:5>