fukamachi / integral

[OBSOLETE] Use Mito instead.
https://github.com/fukamachi/mito
BSD 3-Clause "New" or "Revised" License
54 stars 3 forks source link

Auto incremental primary key with sqlite #3

Closed lucashpandolfo closed 9 years ago

lucashpandolfo commented 9 years ago

Using latest Integral from git.

Using sqlite3, insert-dao fails to assign a proper autoincremental primary key. Seems like it is always assigning 1, and gives the error

DB Error: PRIMARY KEY must be unique (Code: CONSTRAINT)

Interestingly enough, insert-dao has a special case just for sqlite3, if the test is invalidated (as below, notice the hatred) the pk is properly assigned:

(defmethod insert-dao ((obj <dao-class>))
  (let ((serial-key (table-serial-key (class-of obj)))
        (sqlite3-p (eq :sqlite3-hate-uuuuuuuuu (database-type))))
    (labels ((get-pk-value ()
               (if serial-key
                   (last-insert-id (get-connection)
                                   (table-name obj)
                                   (string-downcase serial-key))
                   nil)))
      (when sqlite3-p
        (let ((pk-value (get-pk-value)))
          (when (integerp pk-value)
            (setf (slot-value obj serial-key) (1+ pk-value)))))
      (execute-sql (make-insert-sql obj))
      (unless sqlite3-p
        (when-let (pk-value (get-pk-value))
          (setf (slot-value obj serial-key) pk-value)))
      (setf (dao-synced obj) T)
      obj)))
fukamachi commented 9 years ago

Looks weird. I'm looking into it and should be fixed. Thank you for reporting.

fukamachi commented 9 years ago

I added some tests to check the behaviour at f7a4bce618fc0a0bd8ece1d768ba7706abed62e6, however they all passed on my environment. Did I miss something?

lucashpandolfo commented 9 years ago

You are right. I omitted something important.

CL-USER> (ql:quickload 'integral)
(import 'integral:<dao-table-class>)

(defclass user ()
  ((name :col-type text
         :initarg :name))
  (:metaclass <dao-table-class>))

To load "integral":
  Load 1 ASDF system:
    integral
; Loading "integral"
.....
#<<DAO-TABLE-CLASS> USER>
CL-USER> (import 'integral:connect-toplevel)

T
CL-USER> (connect-toplevel :sqlite3
                  :database-name "/tmp/testdb.db")
To load "dbd-sqlite3":
  Load 1 ASDF system:
    dbd-sqlite3
; Loading "dbd-sqlite3"
............
#<DBD.SQLITE3:<DBD-SQLITE3-CONNECTION> {1006FFC1E3}>
CL-USER> (integral:ensure-table-exists 'user)

CREATE TABLE IF NOT EXISTS "user" ("%oid" SERIAL NOT NULL PRIMARY KEY, "name" TEXT);
; No value
CL-USER> (integral:save-dao (make-instance 'user :name "User 1"))
#<USER %oid: 1>
CL-USER> (integral:save-dao (make-instance 'user :name "User 2"))
#<USER %oid: 2>
CL-USER> (integral:save-dao (make-instance 'user :name "User 3"))
#<USER %oid: 3>
CL-USER> (integral:disconnect-toplevel)
NIL
CL-USER> (connect-toplevel :sqlite3
                  :database-name "/tmp/testdb.db")
#<DBD.SQLITE3:<DBD-SQLITE3-CONNECTION> {1008A50333}>
CL-USER> (integral:save-dao (make-instance 'user :name "User 4"))

DB Error: column %oid is not unique (Code: CONSTRAINT)
   [Condition of type DBI.ERROR:<DBI-DATABASE-ERROR>]
fukamachi commented 9 years ago

Thanks for the precise logs. SQLite3's last_insert_rowid() returns the last ID for the current connection, not in the whole records. I've fixed it at the master branch.

lucashpandolfo commented 9 years ago

Thanks for your hard work.

lucashpandolfo commented 9 years ago

Well, turns out i was wrong. It seemed to work, but did not (at least is not working as of latest version).

The id are correctly assigned. After a disconnect it always returns id=2.

It works if the sort order is specified to be DESC at line 36 of sqlite3.lisp.

(order-by (:desc primary-key))

https://github.com/fukamachi/integral/blob/master/src/connection/sqlite3.lisp#L36

rudolph-miller commented 9 years ago

Thanks for you reporting. It seems to return 2 because of ascending order of primary-key.