Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sqlite3 auto-increment slots in ooddl not working #7

Open
u-u-h opened this issue Apr 7, 2016 · 2 comments
Open

sqlite3 auto-increment slots in ooddl not working #7

u-u-h opened this issue Apr 7, 2016 · 2 comments

Comments

@u-u-h
Copy link

u-u-h commented Apr 7, 2016

I've tried to use auto-increment slots on the sqlite3 backend, and they seem to be working only partially. The following code is not working:

(ql:quickload "clsql")`
(ql:quickload "clsql-sqlite3")
(clsql:def-view-class foo ()
       ((id :accessor id :initarg :id :type integer
       :db-constraints (:not-null :auto-increment))))
(defun test ()
  (clsql:with-database (clsql:*default-database* '("foo.sqlite3") :database-type :sqlite3)
    (let ((clsql:*db-auto-sync* t))
      (ignore-errors
        (clsql:create-view-from-class 'foo))
      (let ((x (make-instance 'foo)))
        ;; this will fail -- error: The slot CL-USER::ID is unbound in the object #<FOO {100BF60C03}>.                        
        (id x)))))

(test)
=> error: The slot CL-USER::ID is unbound in the object #<FOO {100BF60C03}>

It seems that the issue has been mentioned on the mailing list in 2010 http://article.gmane.org/gmane.lisp.clsql.general/1221 and traces of the patch suggested back then (before code refactoring) seem to be around, yet behavior is wrong.

I'm struggling to suggest an appropriate fix. My analysis is that update-autoincrement-keys is not called, since %update-instance-helper decides the slot is not one that needs updating. That is a decision apparently made at view-class construction time. I can't currently test against a different backend to see whether (and why) it works there.

Any hints? I'm ready to try and fix it with some guidance.

(for the record: this is on OSx, sbcl 1.3.3, quicklisp's march release, which contains clsql from kpe.io master branch in january 2016 AFAICS)

uuh

@u-u-h
Copy link
Author

u-u-h commented Apr 7, 2016

Looking into this further, it seems that with the stripped down view class above update-records-from-instance does not find any slot worth saving (view-classes-and-storable-slots returns NIL -- due to storable-slots in view-classes-and-storable-slots explicitly dropping them). Thus, %update-instance-helper is not even called once in the loop.
This seems like it should not work with any backend.
Now, one might argue that a view-class with only an auto-increment slot is weird, but the problem also manifests if I add another slot:

(clsql:def-view-class foo2 ()
  ((id :accessor id :initarg :id :type integer
       :db-constraints (:not-null :auto-increment))
  (bar :accessor bar :initarg :bar :type integer)))

Now, if I do (make-instance 'foo2) the bar slot remains unbound, which will let %update-instance-helper bail out early due to ATTRIBUTE-VALUE-PAIRS saying there are no updates. That leaves the auto-increment slot unbound too. (This situation might be considered a user-error; but having an unbound slot in a view-class instance with auto-commit turned on might as well raise an error I think).

If I initialize slot bar, e.g. (make-instance 'foo2 :bar 21) it starts working. Things are more subtle here, though. update-auto-increment-keys is called, but keyslots-for-class consider the id slot a keyslot only I set :db-kind :key. Is it intentional that auto-increment columns always have to be KEYs? I can live with it for now, since I changed to denormalized schemata in my application (The reason was that mixing :db-kind :key and :auto-increment t and :normalized t gave me SQL syntax errors from sqlite3 -- I'll see if I can reproduce that in a separate issue).

Summary:

  1. It seems that a view-class object must have at least one :key or :base slot besides the :auto-increment slot to have that updated correctly. working.
  2. Failing to initialize a slot with clsql:*db-auto-sync* turned on will not raise an error, and may keep the object from being synced to the database; that may include not creating the object at all in the database.

uuh

@bobbysmith007
Copy link
Collaborator

As you are discovering, there is a lot of silent "by convention" semantics in CLSQL. Its large and complex and has lots of cases like this where things don't work as expected if they are not used as expected ("expected" being how the authors thought).

If you add warnings or errors as patches, or figure out something to add to the documentation that will help, I will gladly merge them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants