Jeremy- 

Okay, this is probably going to be the hardest sell for me, but I'll give it a 
try because I did get it all integration tests to work except I had to make 1 
change to Sequel proper and 1 to the schema tests. The duplicate SQL was not 
because of the adapter's alter_table_sql was calling itself, but it was in 
Sequel proper. Sybase does not do implicit alters. In other words, you can NOT 
drop a column that is a foreign key with out dropping the foreign key first. 
Same with adding foreign keys where either the column can not be nullable or in 
the case of a composite key the unique constraint must be in place to add the 
foreign key. My guess is others don't require this. 


in schema_generator.rb about line 431 we have: 


def drop_foreign_key(name, opts=OPTS) 

drop_composite_foreign_key(Array(name), opts) 
drop_column(name) unless name.is_a?(Array) <--- this is causing the grieve 
end 


If you just drop a column that is a foreign key and in a database the requires 
an explicit drop of the foreign key, then you must drop the foreign key first. 
So, I put the code in to check if a column is part of a foreign key on a plane 
drop column to get that test to pass (a failing test earlier). Then when I get 
to a test that drops a foreign key, it also drops the column. It may be a 
convenience to do so, but I will argue they are exclusive actions especially 
for a database that will not allow you to drop a column until you drop the 
foreign key constraint first. Once I commented out that line, then a drop a 
single column foreign key worked and the drop of a column that is a foreign key 
still worked because of the schema check. I still cannot believe other database 
don't have a problem with that, but I'm guessing most will drop a foreign key 
implicitly when just dropping a column. 




Now for the other problem and it is with a test but along the same reasoning. 
If we add a foreign key in a database that requires explicit commands to do so, 
then we need to add the constraint prior to adding the foreign key. This could 
be making a column not nullable or adding a unique constraint on a composite 
key. Maybe other DBs will do this for you implicitly, but I can not imagine all 
do so. So, in order to get a add composite foreign key to work, I had to make 
sure that a unique constraint existed and create it if you add a composite key. 
This caused the following test to fail because it was adding the unique 
constraint prior to adding the key. 


in integration/schema_test.rb about line 549 



@db.alter_table(:items) do 
add_unique_constraint [:item_id, :id] <--- this is causing the grieve 
add_foreign_key [:id, :item_id], :items, :key=>[:item_id, :id] <--- because of 
the checking, this will also add the constraint for a db that requires it do. 
end 




My thought is this test was set up for a database that requires explicit 
commands and other test were mostly set up for database that will implicitly do 
things for you if they are not there. 


I like to make Sequel act like things are done implicitly and if the DB 
requires explicit commands to accomplish X then it must generate all the 
commands to accomplish X. I'm an old c programmer and if you know anything 
about make then creating implicit rules rule. 


Finally, I know these 2 minor changes may break a lot of DBs, but maybe they 
will make Sequel more consistent when altering a db schema and the breaks are 
not that bad. People would just have to remember that dropping a single column 
foreign key will not automatically drop the column from the table and that you 
don't have to create a unique constraint when adding a composite key which 
probably already happens and the DB that implicitly do it probably check to see 
if it exists before doing so. 


Sorry for being so long winded on these and I hope I made sense. I feel like I 
would fix 1 test to only break another and I'd have to put in 1 ugly hack to 
get them all to work. 


Remember my intentions are always to make Sequel better, but in this case to 
add a new adapter too. 


Regards, 


-GregD 













-- 
You received this message because you are subscribed to the Google Groups 
"sequel-talk" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/sequel-talk.
For more options, visit https://groups.google.com/groups/opt_out.

Reply via email to