Re: :auto-increment support for primary keys MySQL only

Holger Schauer <[email protected]> Thu, 10 Mar 2011 14:42:06 +0100
Newsgroups gmane.lisp.clsql.general
Message-ID <[email protected]>
On 6389 September 1993, Holger Schauer wrote:
> On 6384 September 1993, Holger Schauer wrote:
> [:autoincrement for Postgres]

Perhaps I should have said why I'm interested: updating clsql to a
recent version broke my old application. I previously just used
:db-type "serial" on Postgres, and I had working auto-increment
suppport. This no longer works, probably not since the explicit
support for auto-increment on MySQL has been added.

>> So the select should probably move to a backend specific method. I'll
>> see if I can come up with a patch in the next days.

Appended you can find two patch changesets: one applies to Nathans
auto-increment branch, the other one can be applied to master. The
patches add auto-increment support for Postgresql (generic-postgresql,
actually, tested with postgresql-socket so far).

Some notes: I basically went the way I outlined in the mail above,
however it wasn't as easy as I thought. First of all, I added
(preliminary) support for having a :default constraint. This allowed
me to specialize database-generate-column-definition for
postgres. database-make-autoincrement-sequence does what it name
suggests, implicitly generating a sequence for autoincrement usage
that is named as if one would have used :db-type "serial" (The code is
not really Postgres specific and could be used for other backends,
too). This sequence name is then stored in a new slot
autoincrement-sequence on view-class-slot-definition-mixin.  I
extended Nathans database-last-autoincrement-id for Postgres to honour
that slot. This way, a user can specify a different sequence name.

I also tried adding autoincrement support for sqlite3, but eventually
gave up. Adapting database-last-autoincrement-id is easy enough, but
supporting table creation is not. sqlite3 takes autoincrement as a
constraint only for INTEGER PRIMARY KEY column types. However, using
:type integer and :db-kind :key results in a column type INT (not
INTEGER) and the primary key constraint doesn't get included in the
constraint list (it's appended in an ALTER clause, IIRC).
Additionally, AUTO-INCREMENT would need to be translated (in a
specialized version of database-constraint-statement) to AUTOINCREMENT
(instead of AUTO_INCREMENT for mysql). So, supporting autoincrement
properly on sqlite requires quite a lot of rework.

I guarded against usage of autoincrement on unsupported platforms in
database-constraint-statement, so that users get a reasonable error
message instead of database syntax errors. 

Some notes on the testsuite: Five :oodml tests fail on postgres,
because Postgres doesn't support usage of a column name as a boolean
expression (:OODML/SELECT/12, :OODML/SELECT/14,     :OODML/SELECT/16,
:OODML/SELECT/18, :OODML/SELECT/22). I.e., the use of [subloc_id] in a
where clause needs to become "subloc_id NOT NULL" on Postgres. I made
no attempt at fixing these. I also had further test failures on both
Postgres and Mysql that are unrelated to the changes I made (i.e. they
errored before my changes).

Generally, I found the test setup not as helpful as I had hoped. The
major pain was getting enough usable information from the errors or
tracebacks -- typically I couldn't see what the error was, so I used a
lot of traces and format's. This was hindering me at startup with the
existing data sets, so I added a very small new one (ds-artist) with
three new tests (select/23, update-records/11 + /12). Later on, I
found that the ds-nodes set misses autoincrement constraints, I added
those. Additionally, update-records/9-slots contained a mistake. How
to call a specific test should be documented in the README, IMHO. I
used e.g.

(let ((db (connect '("localhost" "clsql_devel" "schauer" "brighton")
                   :database-type :postgresql-socket 
                   :make-default t)))
   (start-sql-recording :type :both)
   (rt:do-test :oodml/update-records/9-slots)
   (stop-sql-recording :type :both)
   (disconnect))

As a final note on the test suite, with the introduction of
autoincrement support for Postgres it's no longer possible to test all
database backends at once, because the Postgres code will modify the
class definition (more exactly, the db-constraint slot of the
view-class-slot-definition-mixin). So, the test suite would need to
forget all (test view) class definitions before going through the next
backend.

Finally, I had trouble getting the mysql version to work at all, due
to package trouble. I was getting package errors (package NIL not
found) on the interning of ATOL64 for clsql-uffi and for
mysql-get-client-info for clsql-mysql. I added a simple (find-package
...) before the call to intern to fix the issue (not included in the
patches). 

Holger

_______________________________________________
CLSQL mailing list
[email protected]
http://lists.b9.com/cgi-bin/mailman/listinfo/clsql
auto-increment-patches.zip (application/zip, 7.9 KB) - not displayed
master-patches.zip (application/zip, 10.7 KB) - not displayed