fukamachi / mito

An ORM for Common Lisp with migrations, relationships and PostgreSQL support
284 stars 31 forks source link

Inconsistent use of timezone when setting dao creation and update times #2

Closed fchurca closed 6 years ago

fchurca commented 8 years ago

Creation and update timestamps sometimes differ from the intended values, probably by multiples of the local timezone. Using the sample user class from README.markdown: (tests were run around 2016-04-02T19:05-03:00)

Machine: Linux 3.11-2-amd64 Lisp: SBCL 1.1.15.debian ASDF: 3.1.7 Quicklisp: 2016-03-18 (latest)

CL-USER> (ql:quickload :mito)
To load "mito":
  Load 1 ASDF system:
    mito
; Loading "mito"
......
(:MITO)
CL-USER> (mito.connection:connect-toplevel :sqlite3 :database-name #P"/tmp/test.db3")
#<DBD.SQLITE3:<DBD-SQLITE3-CONNECTION> {10059B6DE3}>
CL-USER> (defclass user ()
  ((name :col-type (:varchar 64)
         :initarg :name
         :accessor user-name)
   (email :col-type (or (:varchar 128) :null)
          :initarg :email
          :accessor user-email))
  (:metaclass mito:dao-table-class))
#<MITO.DAO.TABLE:DAO-TABLE-CLASS USER>
CL-USER> (mito.dao:ensure-table-exists 'user)
;; CREATE TABLE "user" (
    "id" INTEGER PRIMARY KEY AUTOINCREMENT,
    "name" VARCHAR(64) NOT NULL,
    "email" VARCHAR(128),
    "created_at" TIMESTAMP,
    "updated_at" TIMESTAMP
) () [0 rows] | MITO.DB:EXECUTE-SQL
(#<SXQL-STATEMENT: CREATE TABLE user (
    id INTEGER PRIMARY KEY AUTOINCREMENT,
    name VARCHAR(64) NOT NULL,
    email VARCHAR(128),
    created_at TIMESTAMP,
    updated_at TIMESTAMP
)>)
CL-USER> (mito.dao:create-dao 'user :name "fchurca") ; This happened around 19:33, timezone -3:00
#<USER {1005ACE613}>
CL-USER> (defparameter *me* (mito.dao:find-dao 'user :name "fchurca"))
*ME*
CL-USER> (inspect *me*)

The object is a STANDARD-OBJECT of type USER.
0. CREATED-AT: @2016-04-02T16:33:45.000000-03:00 ; Creation and update times off by 3h
1. UPDATED-AT: @2016-04-02T16:33:45.000000-03:00
2. SYNCED: T
3. ID: 5
4. NAME: "fchurca"
5. EMAIL: NIL
> q
; No value
CL-USER> (setf (user-email *me*) "fchurca@example.com")
"fchurca@example.com"
CL-USER> (mito.dao:save-dao *me*)
; No value
CL-USER> (inspect *me*) 

The object is a STANDARD-OBJECT of type USER.
0. CREATED-AT: @2016-04-02T16:33:45.000000-03:00
1. UPDATED-AT: @2016-04-02T19:36:53.100683-03:00 ; Update time is now correct
2. SYNCED: T
3. ID: 5
4. NAME: "fchurca"
5. EMAIL: "fchurca@example.com"
>
lucashpandolfo commented 8 years ago

Relevant: Sqlite is kind of awkward when it comes to timezones The default timezone is always used by mito, but sqlite doesn't care at all. The zone information is not stored, so when you update the object, the correct time is displayed in the 'updated-at' field, since it was filled by mito. If you reload the object you will know what i mean:

(ql:quickload :mito)
(mito.connection:connect-toplevel :sqlite3 :database-name #P"/tmp/test.db3")
(defclass user ()
  ((name :col-type (:varchar 64)
         :initarg :name
         :accessor user-name)
   (email :col-type (or (:varchar 128) :null)
          :initarg :email
          :accessor user-email))
  (:metaclass mito:dao-table-class))
(mito.dao:ensure-table-exists 'user)
(mito.dao:create-dao 'user :name "fchurca")
(defparameter *me* (mito.dao:find-dao 'user :name "fchurca"))
(describe *me*)
#<USER {1006C64D93}>
  [standard-object]

Slots with :INSTANCE allocation:
  CREATED-AT  = @2016-04-25T07:39:24.000000-03:00 ;; off by 3
  UPDATED-AT  = @2016-04-25T07:39:24.000000-03:00 ;; off by 3
  SYNCED      = T
  ID          = 1
  NAME        = "fchurca"
  EMAIL       = NIL
(setf (user-email *me*) "fchurca@example.com")
(mito.dao:save-dao *me*)
(describe *me*)
#<USER {1006C64D93}>
  [standard-object]

Slots with :INSTANCE allocation:
  CREATED-AT  = @2016-04-25T07:39:24.000000-03:00 ;; off by 3
  UPDATED-AT  = @2016-04-25T10:40:32.455256-03:00 ;; correct
  SYNCED      = T
  ID          = 1
  NAME        = "fchurca"
  EMAIL       = "fchurca@example.com"
(setf *me* (mito.dao:find-dao 'user :name "fchurca"))
(describe *me*)
#<USER {10072DFBF3}>
  [standard-object]

Slots with :INSTANCE allocation:
  CREATED-AT  = @2016-04-25T04:39:24.000000-03:00 ;; off by 6
  UPDATED-AT  = @2016-04-25T07:40:32.000000-03:00 ;; off by 3
  SYNCED      = T
  ID          = 1
  NAME        = "fchurca"
  EMAIL       = "fchurca@example.com"

The next time created-at will be off by 9, then 12, etc.

lucashpandolfo commented 8 years ago

By the way, as a workaround, you can force the timezone to UTC, so the offset is always 0.

(setf local-time:*default-timezone* local-time:+utc-zone+)
fchurca commented 8 years ago

I have tried the same replacing the sqlite3 database with a mysql and still get the same behaviour. traceing sxql:make-clause reveals that timezone is ignored by sxql, at least in the version in the current quicklisp dist. This may actually be an issue in sxql.operator:convert-for-sql.

fchurca commented 8 years ago

I have dug further and have found the following: created-at and updated-at are converted by sxql from timestamp to string before being given to dbi for inserting and updating. This conversion is done by sxql:convert-for-sql, who discards the offset.

Later, in the way back from a select, mito receives a string, which it passes into local-time:parse-timestring, where UTC is assumed.

A possible fix could involve patching sxql:convert-for-sql to use the following:

(local-time:format-timestring nil object
                              :format +sql-datetime-format+
                              :timezone local-time:+utc-zone+)

with +sql-datetime-format+ being '((:year 4) #\- (:month 2) #\- (:day 2) #\Space (:hour 2) #\: (:min 2) #\: (:sec 2) #\. (:usec 6)) or thereabouts. It would also entail making sxql depend on local-time.

Another possible fix could involve defining deflators for the timestamp columns in mito, which would do the same.

fchurca commented 6 years ago

Fixed at least since version shipped with QL 2018-07-11

SBCL 1.4.9.debian  Port: 4005  Pid: 7025
; SWANK 2.21
CL-USER> (ql:dist-version "quicklisp")
"2018-07-11"
CL-USER> (ql:quickload :mito)
To load "mito":
  Load 1 ASDF system:
    mito
; Loading "mito"
...
(:MITO)
CL-USER> (mito.connection:connect-toplevel :sqlite3 :database-name #P"/tmp/test.db3") 
#<DBD.SQLITE3:<DBD-SQLITE3-CONNECTION> {10045C6E73}>
CL-USER> (defclass user ())
error while parsing arguments to DEFMACRO DEFCLASS:
  too few elements in
    (USER NIL)
  to satisfy lambda list
    (SB-PCL::NAME SB-PCL::DIRECT-SUPERCLASSES
     SB-PCL::DIRECT-SLOTS &REST SB-PCL::OPTIONS):
  at least 3 expected, but got 2
   [Condition of type SB-KERNEL::ARG-COUNT-ERROR]
; Evaluation aborted on NIL
CL-USER> ; Quit to level 1
CL-USER> ; Evaluation aborted on #<SB-KERNEL::ARG-COUNT-ERROR {10045C8713}>
CL-USER> (defclass user ()
  ((name :col-type (:varchar 64)
         :initarg :name
         :accessor user-name)
   (email :col-type (or (:varchar 128) :null)
          :initarg :email
          :accessor user-email))
  (:metaclass mito:dao-table-class))
#<MITO.DAO.TABLE:DAO-TABLE-CLASS COMMON-LISP-USER::USER>
CL-USER> (mito.dao:ensure-table-exists 'user)
;; CREATE TABLE "user" (
    "id" INTEGER PRIMARY KEY AUTOINCREMENT,
    "name" VARCHAR(64) NOT NULL,
    "email" VARCHAR(128),
    "created_at" TIMESTAMP,
    "updated_at" TIMESTAMP
) () [0 rows] | MITO.DAO:ENSURE-TABLE-EXISTS
(#<SXQL-STATEMENT: CREATE TABLE user (
    id INTEGER PRIMARY KEY AUTOINCREMENT,
    name VARCHAR(64) NOT NULL,
    email VARCHAR(128),
    created_at TIMESTAMP,
    updated_at TIMESTAMP
)>)
CL-USER> (mito.dao:create-dao 'user :name "fchurca")
#<USER {1005D371C3}>
CL-USER> (defparameter *me* (mito.dao:find-dao 'user :name "fchurca")) 
*ME*
CL-USER> (inspect *me*) 

The object is a STANDARD-OBJECT of type USER.
0. CREATED-AT: @2018-07-28T13:18:04.413678-03:00 ; Creation local time and timezone correct
1. UPDATED-AT: @2018-07-28T13:18:04.413678-03:00 ; Update local time and timezone correct
2. SYNCED: T
3. ID: 1
4. NAME: "fchurca"
5. EMAIL: NIL
> q
; No value
CL-USER> (setf (user-email *me*) "fchurca@example.com")
"fchurca@example.com"
CL-USER> (mito.dao:save-dao *me*)
; No value
CL-USER> (inspect *me*) 

The object is a STANDARD-OBJECT of type USER.
0. CREATED-AT: @2018-07-28T13:18:04.413678-03:00 ; Creation local time and timezone respected after update
1. UPDATED-AT: @2018-07-28T13:18:54.883662-03:00 ; Update local time and timezone correct
2. SYNCED: T
3. ID: 1
4. NAME: "fchurca"
5. EMAIL: "fchurca@example.com"
> q
; No value
CL-USER> (setf *me* (mito.dao:find-dao 'user :name "fchurca"))
#<USER {100264EB23}>
CL-USER> (inspect *me*)

The object is a STANDARD-OBJECT of type USER.
0. CREATED-AT: @2018-07-28T13:18:04.413678-03:00 ; Creation local time and timezone correct after retrieval
1. UPDATED-AT: @2018-07-28T13:18:54.883662-03:00 ; Update local time and timezone correct after retrieval
2. SYNCED: T
3. ID: 1
4. NAME: "fchurca"
5. EMAIL: "fchurca@example.com"
>