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.
The existing automated tests fail if only #at:putDateTime: is modified, and succeed if both #at:putDateTime: and #dateTimeAt: are modified.
What do you think?
Thanks! Alistair
UDBCSQLite3Statement>>dateTimeAt: aColumn | utc | utc := DateAndTime readFrom: (self library stringFrom: handle at: aColumn) readStream defaultOffset: Duration zero. ^utc offset: DateAndTime localOffset.
All methods from DateAndTime class>>
readFrom: aStream defaultOffset: defaultOffset "Parse and return a new DateAndTime instance from stream, as a Date, an optional Time and an optional TimeZone offset. The time defaults to midnight, the timezone to defaultOffset" "self readFrom: '2013-03-04T23:47:52.876+01:00' readStream"
| date time offset | date := Date readFrom: aStream. [ aStream atEnd or: [ '0123456789Z+-' includes: aStream peek ] ] whileFalse: [ aStream next ]. ('0123456789' includes: aStream peek) ifTrue: [ time := Time readFrom: aStream ] ifFalse: [ time := Time midnight ]. aStream skipSeparators. offset := self readTimezoneOffsetFrom: aStream default: defaultOffset. ^ self year: date year month: date monthIndex day: date dayOfMonth hour: time hour minute: time minute second: time second nanoSecond: time nanoSecond offset: offset
readFrom: aStream "Parse and return a new DateAndTime instance from stream, as a Date, an optional Time and an optional TimeZone offset. The time defaults to midnight, the timezone to the local offset" "self readFrom: '2013-03-04T23:47:52.876+01:00' readStream"
^self readFrom: aStream defaultOffset: self localOffset.
readTimezoneOffsetFrom: stream default: defaultOffset "Read and return an optional timezone offset in the form of [+|-]hh[[separator]mm[[separator]ss]] or Z from stream as a duration. If there is no offset, return the defaultOffset."
| sign hour minute second | (stream peekFor: $Z) ifTrue: [ ^ Duration zero ]. hour := minute := second := 0. ^ ('+-' includes: stream peek) ifTrue: [ sign := stream next = $- ifTrue: [ -1 ] ifFalse: [ 1 ]. hour := self readTwoDigitIntegerFrom: stream. (self readOptionalSeparatorFrom: stream) ifNotNil: [ minute := self readTwoDigitIntegerFrom: stream. (self readOptionalSeparatorFrom: stream) ifNotNil: [ second := Integer readFrom: stream ] ]. Duration seconds: sign * ((hour * 3600) + (minute * 60) + second) ] ifFalse: [ defaultOffset ]
readTimezoneOffsetFrom: stream "Read and return an optional timezone offset in the form of [+|-]hh[[separator]mm[[separator]ss]] or Z from stream as a duration. If there is no offset, return the local offset."
^self readTimezoneOffsetFrom: stream default: self localOffset