Hi Sven, On Sun, Jun 26, 2016 at 03:34:15PM +0200, Sven Van Caekenberghe wrote:
On 26 Jun 2016, at 14:59, Alistair Grant <akgrant0710@gmail.com> wrote:
Hi All,
UDBCSQLite3Statement>>at:putDateTime: currently writes date and times using DateAndTime's default print string, i.e. YYYY-MM-DDTHH:MM:SS.mmmmmm+hh:ss
SQLite3 doesn't support timezones, and expects text formatted date/times to be:
YYYY-MM-DD HH:MM:SS.mmm
See: https://www.sqlite.org/datatype3.html
The concensus, as far as I can tell, is that UTC times should always be stored (which seems to be the only sensible option).
Changing UDBCSQLite3Statement>>at:putDateTime: to:
at: aColumn putDateTime: aDateTime "Put the supplied DateAndTime in column aColumn.
SQLite3 supports three date/time formats: text, real (Julian day), integer (unix seconds). We use the ISO8601 text format: YYYY-MM-DD HH:MM:SS.SSS See https://www.sqlite.org/datatype3.html"
| s |
s := UDBCSQLite3DateTimeString streamContents: [ :stream | |utc | utc := aDateTime asDateAndTime asUTC. utc printYMDOn: stream. stream nextPut: Character space. utc printHMSOn: stream. stream nextPut: $.. stream nextPutAll: ((aDateTime nanoSecond / 1000000) rounded printPaddedWith: $0 to: 3) ]. ^ self library with: handle at: aColumn putString: s
writes date / times in the correct format.
Note that SQLite3 doesn't do much in the way of format checking, so the old implementation didn't raise an error, but can cause problems if the database is shared with other applications that follow the rules (which is why I hit this problem).
To provide some backward compatibility, and follow the rule of being strict in what you write and forgiving in what you read, we need to modify UDBCSQLite3Statement>>dateTimeAt: to accept timezones when supplied, or default to UTC time.
Unfortunately, the current DateAndTime>>readFrom: method assumes the local timezone if none is specified.
I can think of two approaches to solve this:
1. Modify UDBCSQLite3Statement>>dateTimeAt: to add 'Z' to the string if no timezone is specified, i.e. make UTC explicit.
2. Extend DateAndTime class to support #readFrom:defaultOffset: so that UTC can be specified as the default, instead of the local timezone, and modify UDBCSQLite3Statement>>dateTimeAt: to use UTC as the default.
I prefer the second approach, but realise that this is a change to the core Pharo classes, so will implement the first if that is the concensus. I've included the modified methods for the second approach below.
Yes, option 2 is the right way to go. Your code looks OK from a cursory look.
Please propose a real slice and make certain you add a couple of good unit tests, this is really crucial.
Thanks for your feedback. I'll certainly include automated tests in the slice. I'm still getting up to speed with fogbugz and the patch system. Should this be submitted as two separate issues (one for the UDBC package and one for Kernel-Chronolgy), or as a single issue? Thanks again, Alistair