When creating a brand new database using checksetup.pl and the current bmo/4.2 code, I get an error when creating the foreign keys. It is coming from the schema definition in the ComponentWatching extension. [snip] Adding new column 'comment_count' to the 'profiles' table... Adding new column 'creation_ts' to the 'profiles' table... Adding new column 'first_patch_bug_id' to the 'profiles' table... Adding new column 'votesperuser' to the 'products' table... Adding new column 'maxvotesperbug' to the 'products' table... Adding new column 'votestoconfirm' to the 'products' table... Adding new column 'votes' to the 'bugs' table... Adding new index 'bugs_votes_idx' to the bugs table ... Creating default classification 'Unclassified'... Setting up foreign keys... DBD::mysql::db do failed: Can't create table 'bugs_bmo_42_test.#sql-2d9d_7b' (errno: 121) [for Statement "ALTER TABLE components ADD CONSTRAINT fk_components_initialowner_profiles_userid FOREIGN KEY (initialowner) REFERENCES profiles(userid) ON UPDATE CASCADE ON DELETE RESTRICT, ADD CONSTRAINT fk_components_product_id_products_id FOREIGN KEY (product_id) REFERENCES products(id) ON UPDATE CASCADE ON DELETE CASCADE, ADD CONSTRAINT fk_components_watch_user_profiles_userid FOREIGN KEY (watch_user) REFERENCES profiles(userid) ON UPDATE CASCADE ON DELETE SET NULL, ADD CONSTRAINT fk_components_initialqacontact_profiles_userid FOREIGN KEY (initialqacontact) REFERENCES profiles(userid) ON UPDATE CASCADE ON DELETE SET NULL"] at Bugzilla/DB.pm line 654 If I comment out the REFERENCES part of components/watchuser in install_update_db(), checksetup.pl is able to complete without error. dkl

this doesn't block the bmo 4.2 upgrade.

Used Data::Dumper in Bugzilla::DB::bz_add_column and in Bugzilla::DB::bz_add_fks. Here is the output: ------ snip ------ Adding new column 'watch_user' to the 'components' table... $VAR1 = [ 'ALTER TABLE components ADD COLUMN watch_user mediumint', 'ALTER TABLE components ADD CONSTRAINT fk_components_watch_user_profiles_userid FOREIGN KEY (watch_user) REFERENCES profiles(userid) ON UPDATE CASCADE ON DELETE SET NULL' ]; Setting up foreign keys... $VAR1 = 'component_watch'; $VAR2 = { 'product_id' => { 'TABLE' => 'products', 'DELETE' => 'CASCADE', 'COLUMN' => 'id' }, 'user_id' => { 'TABLE' => 'profiles', 'DELETE' => 'CASCADE', 'COLUMN' => 'userid' }, 'component_id' => { 'TABLE' => 'components', 'DELETE' => 'CASCADE', 'COLUMN' => 'id' } }; $VAR1 = [ 'ALTER TABLE component_watch ADD CONSTRAINT fk_component_watch_product_id_products_id FOREIGN KEY (product_id) REFERENCES products(id) ON UPDATE CASCADE ON DELETE CASCADE, ADD CONSTRAINT fk_component_watch_user_id_profiles_userid FOREIGN KEY (user_id) REFERENCES profiles(userid) ON UPDATE CASCADE ON DELETE CASCADE, ADD CONSTRAINT fk_component_watch_component_id_components_id FOREIGN KEY (component_id) REFERENCES components(id) ON UPDATE CASCADE ON DELETE CASCADE' ]; $VAR1 = 'components'; $VAR2 = { 'initialowner' => { 'TABLE' => 'profiles', 'COLUMN' => 'userid' }, 'product_id' => { 'DELETE' => 'CASCADE', 'TABLE' => 'products', 'COLUMN' => 'id' }, 'watch_user' => { 'TABLE' => 'profiles', 'DELETE' => 'SET NULL', 'COLUMN' => 'userid' }, 'initialqacontact' => { 'DELETE' => 'SET NULL', 'TABLE' => 'profiles', 'COLUMN' => 'userid' } }; $VAR1 = [ 'ALTER TABLE components ADD CONSTRAINT fk_components_initialowner_profiles_userid FOREIGN KEY (initialowner) REFERENCES profiles(userid) ON UPDATE CASCADE ON DELETE RESTRICT, ADD CONSTRAINT fk_components_product_id_products_id FOREIGN KEY (product_id) REFERENCES products(id) ON UPDATE CASCADE ON DELETE CASCADE, ADD CONSTRAINT fk_components_watch_user_profiles_userid FOREIGN KEY (watch_user) REFERENCES profiles(userid) ON UPDATE CASCADE ON DELETE SET NULL, ADD CONSTRAINT fk_components_initialqacontact_profiles_userid FOREIGN KEY (initialqacontact) REFERENCES profiles(userid) ON UPDATE CASCADE ON DELETE SET NULL' ]; DBD::mysql::db do failed: Can't create table 'bugs.#sql-1985_d3' (errno: 121) [for Statement "ALTER TABLE components ADD CONSTRAINT fk_components_initialowner_profiles_userid FOREIGN KEY (initialowner) REFERENCES profiles(userid) ON UPDATE CASCADE ON DELETE RESTRICT, ADD CONSTRAINT fk_components_product_id_products_id FOREIGN KEY (product_id) REFERENCES products(id) ON UPDATE CASCADE ON DELETE CASCADE, ADD CONSTRAINT fk_components_watch_user_profiles_userid FOREIGN KEY (watch_user) REFERENCES profiles(userid) ON UPDATE CASCADE ON DELETE SET NULL, ADD CONSTRAINT fk_components_initialqacontact_profiles_userid FOREIGN KEY (initialqacontact) REFERENCES profiles(userid) ON UPDATE CASCADE ON DELETE SET NULL"] at Bugzilla/DB.pm line 640 Bugzilla::DB::bz_add_fks('Bugzilla::DB::Mysql=HASH(0x5486ae0)', 'components', 'HASH(0x4d69a18)', 'HASH(0x8ed0280)') called at Bugzilla/DB.pm line 542 Bugzilla::DB::bz_setup_foreign_keys('Bugzilla::DB::Mysql=HASH(0x5486ae0)') called at Bugzilla/Install/DB.pm line 722 Bugzilla::Install::DB::update_table_definitions('HASH(0x1b81818)') called at ./checksetup.pl line 169 bash-4.1$ ------ snip ------ In other words: 1. column watch_user added the components table WITH foreign key. 2. alter table fails to add the same foreign key. You can maybe just create the column without the foreign keys, then let Bugzilla::DB::bz_setup_foreign_keys take care of the rest. Possible fix: ------ snip ------ diff --git a/Bugzilla/DB.pm b/Bugzilla/DB.pm index cf828d7..1df5fb8 100644 --- a/Bugzilla/DB.pm +++ b/Bugzilla/DB.pm @@ -577,8 +577,10 @@ sub bz_add_column { my $current_def = $self->bz_column_info($table, $name); if (!$current_def) { + my $trimmed_def = dclone($new_def); + delete $trimmed_def->{REFERENCES}; my @statements = $self->_bz_real_schema->get_add_column_ddl( - $table, $name, $new_def, + $table, $name, $trimmed_def, defined $init_value ? $self->quote($init_value) : undef); print get_text('install_column_add', { column => $name, table => $table }) . "\n" ------ snip ------

I'm able to hit this with master (3f631ad59df2f086e8d8343401ab27082cba5478). The patch in comment #2 works, however.

Ugh what a mess. Took me a bit to figure this one out. Not sure if it is a bug or by design but the Bugzilla DB code is written that $dbh->bz_add_column($table, $column, $def) will create the foreign key at the time the column is added. Then the new reference is added to the components table definition and when the code that installs all of the foreign keys later comes to the components definition, it thinks that the reference has not been added yet and tries to added it again. Adding created => 1, causes the second try to not occur. Also added code for adding the default watch user for the TestProduct/TestComponent created for a new install which was a separate bug somewhat related issue that occurred after foreign keys are created. dkl

(In reply to David Lawrence [:dkl] from comment #4) > Ugh what a mess. Took me a bit to figure this one out. Not sure if it is a > bug or by design but the Bugzilla DB code is written that > $dbh->bz_add_column($table, $column, $def) will create the foreign key at > the time the column is added. Then the new reference is added to the > components table definition and when the code that installs all of the > foreign keys later comes to the components definition, it thinks that the > reference has not been added yet and tries to added it again. > sorry, I took me a long time to figure that out too, and thought that giving the root cause in comment #2 was going to save folks some trouble.

(In reply to Damien Nozay [:dnozay] from comment #5) > sorry, I took me a long time to figure that out too, and thought that giving > the root cause in comment #2 was going to save folks. Ah! I embarrassingly did not go back and read your older comment when I just started investigating this issue locally when I encountered it again recently. That being said, I like your solution better as it will require no change also to bug 1082059 to fix there. Will create a new patch and investigate whether this needs to go upstream. dkl

using proposed fix from comment #2 instead

fwiw this only affects extensions it seems as all of the bz_add_column statements in Bugzilla/Install/DB.pm do not use REFERENCES definitions as those are included the Bugzilla::DB::Schema::ABSTRACT_SCHEMA definition which are added later properly. Still think this is an upstream bug. Maybe we need a new option to bz_add_column that forces the creation of the FK if the user is using bz_add_column outside the context of checksetup.pl dkl

(In reply to David Lawrence [:dkl] from comment #7) > Created attachment 8504169 [details] [diff] [review] > 769829_2.patch > > using proposed fix from comment #2 instead I didn't put comments in comment #2, can you please add comments so it's clear why we don't add FKs early: e.g. # Bug 769829: just create the column without the foreign keys # and let Bugzilla::DB::bz_setup_foreign_keys take care of the rest.

Comment on attachment 8504169 [details] [diff] [review] 769829_2.patch Tracking upsteam instead at 1082106 with a different solution.

769829 < 1082106

(In reply to David Lawrence [:dkl] from comment #10) > Tracking upsteam instead at 1082106 with a different solution. note: since 769829 < 1082106, how come 1082106 is the upstream bug? LpSolit just closed bug 1082106 as INVALID.

(In reply to Damien Nozay [:dnozay] from comment #13) > (In reply to David Lawrence [:dkl] from comment #10) > > Tracking upsteam instead at 1082106 with a different solution. > > note: since 769829 < 1082106, how come 1082106 is the upstream bug? > LpSolit just closed bug 1082106 as INVALID. Well I was just going to commit 1082106 into our codebase (bmo/4.2) once it was reviewed and approved. If upstream continues to push back on the change, we will bring it back here and commit it as a BMO customization only which I was hoping to avoid. dkl

would be great if LpSolit could chime in.