Re: RFC/patch: normalised object oriented clsql? breaks test framework for non-mysql users.

The tests contributed with this patch cause problems for me
loading/running tests on anything not-mysql since all the test cases for
it use :auto-increment tables. On a quick look this doesn't even seem to
be something that can be done with the existing skip-tests
infrastructure since the error arises during test-initialize-database
(called on test-init.lisp:628), which tries to create the view class
'node, containing non-standard SQL.

Just realized I now have separate code for implementing the
auto-increment concept for mssql, postgresql, and now mysql... I'll try
to find some time to merge those implementations somewhat, but we still
need to pull that code into a path that depends on the database so we
can run the tests reliably. Off the top of my head I'm thinking move
that class definition/table building into more test-specific locations
rather than general test init.

I didn't find that till after, at first I was only running the mysql
tests after looking at the update-record[s]-from-* functions and
noticing that the update-record-from-slots function didn't look right.
It appears to have gotten skipped in these updates. 4 patches around the
testing file @
http://github.com/UnwashedMeme/clsql/commits/update-records-refactoring.
I tried to simplify some of that testing code a bit, make sure it pulled
fresh objects from the database and added a few more tests which check
the fn update-record-from-slots.

#this grabs the 4 patches for testing, based on current master, should
be a fast-forward merge.
git fetch git://github.com/UnwashedMeme/clsql.git
git merge 21afae2702897a

I suggest refactoring the three functions update-record-from-slot,
update-record-from-slots, and update-records-from-instance to all go
through a common function, probably update-record-from-slot. Here is an
example from our branch which has a bunch of other stuff and no
normalization: e.g.
http://github.com/UnwashedMeme/clsql/blob/accel/sql/oodml.lisp#L171 
That would reduce a lot of somewhat duplicated logic in there right now,
and make these kinds of misses a lot harder in the future.


We just came across a project that requires the mysql backend (one of
the only ones we hadn't used yet) and ran into some FFI problems that
seem to be already fixed. I viewed this as a good opportunity to merge
in master to our branches and try to package up some of our code for
inclusion upstream... a rabbit hole this is turning into ;-)

Will delve further tomorrow.

Nathan

On 12/4/2009 1:52 PM, Kevin Rosenberg wrote:
> Thanks, I'll review your changes.
>
> Kevin
>
> On Dec 4, 2009, at 6:56 AM, Thijs Oppermann wrote:
>
>   
>> I just finished the first draft of adding documentation for normalised
>> view-classes.
>>
>> I pushed the changes to my clsql-fork branch named
>> 'normalised-inheritance', under commit
>> 625970da6c9b22030b6944169d21c4a2e81880b8.
>>
>> Any comments are of course welcome. If I should attach a patch to the
>> list, I could do that too...
>>
>> And as an aside, I also added a small bugfix for the case where the
>> top-level parent primary key column does not use :auto-increment.
>> Casual testing shows it to work for those cases now too. Also, this
>> little bug alerted me to the very high probability of these changes
>> not being compatible as they are now with other databases besides
>> mysql. As I don't have any other available that will need to be taken
>> up by others...
>>
>> Thijs
>>     
> _______________________________________________
> CLSQL mailing list
> [email protected]
> http://lists.b9.com/cgi-bin/mailman/listinfo/clsql
>