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

RESOLVED DUPLICATE of bug 1082106

Status

()

defect
RESOLVED DUPLICATE of bug 1082106
7 years ago
5 years ago

People

(Reporter: dkl, Assigned: dkl)

Tracking

Development
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Assignee

Description

7 years ago
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.
No longer blocks: bmo4.2
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.
Assignee

Comment 4

5 years ago
Posted patch 769829_1.patch (obsolete) — Splinter Review
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
Assignee: nobody → dkl
Status: NEW → ASSIGNED
Attachment #8504158 - Flags: review?(dylan)
(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.
Assignee

Comment 6

5 years ago
(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
Assignee

Comment 7

5 years ago
using proposed fix from comment #2 instead
Attachment #8504158 - Attachment is obsolete: true
Attachment #8504158 - Flags: review?(dylan)
Attachment #8504169 - Flags: review?(dylan)
Assignee

Comment 8

5 years ago
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.
Assignee

Updated

5 years ago
Depends on: 1082106
Assignee

Comment 10

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

Tracking upsteam instead at 1082106 with a different solution.
Attachment #8504169 - Flags: review?(dylan)
Assignee

Updated

5 years ago
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1082106
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.
Assignee

Comment 14

5 years ago
(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.
You need to log in before you can comment on or make changes to this bug.