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

Thanks for the patches, I'll preparing to release a new
version of CLSQL with your improvements.

Kevin

On Mar 10, 2011, at 6:42 AM, Holger Schauer wrote:

> 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
> 
> <auto-increment-patches.zip><master-patches.zip>_______________________________________________
> CLSQL mailing list
> [email protected]
> http://lists.b9.com/cgi-bin/mailman/listinfo/clsql