TimeDelta support needs improvements

Bug #163565 reported by Gustavo Niemeyer
4
Affects Status Importance Assigned to Milestone
Storm
Fix Released
Undecided
Gustavo Niemeyer

Bug Description

TimeDelta support isn't as well tested as the other date/time types, is
broken in SQLite and MySQL, has precision issues in PostgreSQL, and
potentially uses code which conflicts with the license for parsing.

Related branches

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

The attached branch fixes all of these issues, and improves the format
accepted by the interval parser.

Changed in storm:
assignee: nobody → niemeyer
status: New → In Progress
Revision history for this message
Barry Warsaw (barry) wrote :

The branch works great for me. How about committing it? :)

Revision history for this message
Jamu Kakar (jkakar) wrote :

Nice branch! +1.

Revision history for this message
Christopher Armstrong (radix) wrote :

[1] Is there a reason you don't upcall to Connection.to_database in MySQLConnection.to_database? I guess it would be awkward, since you'd have to iterate the results and re-yield them, but the code duplication is unfortunate.

[2]
Out of curiosity, why did you make these things keyword arguments to _parse_interval? I figured it might've been for testing that function, but it doesn't seem they're used.

+def _parse_interval(interval, _table=_parse_interval_table(),
+ _re=_parse_interval_re):

+1!

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

[1]

Yes, that's just preventing the creation of the additional generator. I think it's worth it, as it's in
a very internal loop which is used very frequently.

[2]

A silly early optimization. I took it off.

Thanks for these reviews!

Changed in storm:
status: In Progress → Fix Committed
Changed in storm:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.