769829 - Creating new database causes foreign key error when ComponentWatching extensions is enabled

Status: RESOLVED DUPLICATE of bug 1082106

(bugzilla.mozilla.org :: Extensions, defect)

Description

[David Lawrence [:dkl]]

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

Comment 1

[:glob ✱]

this doesn't block the bmo 4.2 upgrade.

Comment 2

[Damien Nozay [:dnozay]]

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 ------

Comment 3

[Gregory Szorc [:gps]]

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

Comment 4

[David Lawrence [:dkl]]

Attachment: 769829_1.patch

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

Comment 5

[Damien Nozay [:dnozay]]

(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.

Comment 6

[David Lawrence [:dkl]]

(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

Comment 7

[David Lawrence [:dkl]]

Attachment: 769829_2.patch

using proposed fix from comment #2 instead

Comment 8

[David Lawrence [:dkl]]

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

Comment 9

[Damien Nozay [:dnozay]]

(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 10

[David Lawrence [:dkl]]

Comment on attachment 8504169 [details] [diff] [review]
769829_2.patch

Tracking upsteam instead at 1082106 with a different solution.

Comment 12

[Damien Nozay [:dnozay]]

769829 < 1082106

Comment 13

[Damien Nozay [:dnozay]]

(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.

Comment 14

[David Lawrence [:dkl]]

(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

Comment 15

[Damien Nozay [:dnozay]]

would be great if LpSolit could chime in.