Jeremy- Thanks for the quick response.
On Sep 14, 2013, at 3:39 AM, Jeremy Evans <[email protected]> wrote: > On Friday, September 13, 2013 8:50:48 PM UTC-7, GregD wrote: > 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. > > Subclass the AlterTableGenerator in the adapter and override drop_foreign_key > to only call drop_composite_foreign_key if name.is_a?(Array). Set > Database#alter_table_generator_class to the subclass. See the shared > postgres adapter for an example of adapter-specific generator overrides. > That should ensure that the related DROP statement is only issued once. Okay, I'll buy that and I'll override it. > > 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. > > If your add_foreign_key is also adding a unique or NOT NULL constraint to the > referenced table, I think that is what needs to change. > alter_table(:foo){add_foreign_key :column, :bar} should never do anything > starting with ALTER TABLE bar. If there needs to be a unique or NOT NULL > constraint on the referenced table, then the user should add that manually. Then this is breaking the other test. Hence my dilemma. In the other test, it is only adding a foreign key (composite key) without creating the unique constraint like this one is. This makes SqlAnywhere throw an exception. So either, this test needs to change or the other. Either force the creation of a unique constraint or not, but it must be consistent. Based on your response, I say the test that is adding a foreign composite key must add the unique constraint prior to adding the foreign key like this test is. Add based on your response, I guess I'm violating this by altering columns in the referencing table to be not null when adding a single foreign key column to prevent SqlAnywhere from throwing an exception and those tests should be changed to make those explicit not nulls, etc. Thanks again for looking at this. -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.
