From e6ea9c3931636f5ebfb7877da18af28b221cdc58 Mon Sep 17 00:00:00 2001 From: Max Kanat-Alexander Date: Wed, 14 Jul 2010 15:30:39 -0700 Subject: Bug 578739: Instead of removing REFERENCES from _bz_real_schema and then populating FKs from _bz_schema at the end of checksetup, store REFERENCES in _bz_real_schema with a special "created => 0" key that tells us that we still need to create the FK. r=mkanat, a=mkanat (module owner) --- Bugzilla/DB.pm | 120 ++++++++++++++++++++++++++++++++++--------------- Bugzilla/DB/Mysql.pm | 10 ++--- Bugzilla/DB/Oracle.pm | 2 +- Bugzilla/DB/Schema.pm | 48 +++++++++++++++++--- Bugzilla/Install/DB.pm | 20 ++++----- 5 files changed, 139 insertions(+), 61 deletions(-) (limited to 'Bugzilla') diff --git a/Bugzilla/DB.pm b/Bugzilla/DB.pm index eeeff2280..31051dd8e 100644 --- a/Bugzilla/DB.pm +++ b/Bugzilla/DB.pm @@ -421,7 +421,10 @@ sub bz_setup_database { # If we haven't ever stored a serialized schema, # set up the bz_schema table and store it. $self->_bz_init_schema_storage(); - + + # We don't use bz_table_list here, because that uses _bz_real_schema. + # We actually want the table list from the ABSTRACT_SCHEMA in + # Bugzilla::DB::Schema. my @desired_tables = $self->_bz_schema->get_table_list(); my $bugs_exists = $self->bz_table_info('bugs'); if (!$bugs_exists) { @@ -460,23 +463,38 @@ sub bz_setup_foreign_keys { # so if it doesn't have them, then we're setting up FKs # for the first time, and should be quieter about it. my $activity_fk = $self->bz_fk_info('profiles_activity', 'userid'); - if (!$activity_fk) { + my $any_fks = $activity_fk && $activity_fk->{created}; + if (!$any_fks) { print get_text('install_fk_setup'), "\n"; } - # We use _bz_schema because bz_add_table has removed all REFERENCES - # items from _bz_real_schema. - my @tables = $self->_bz_schema->get_table_list(); + my @tables = $self->bz_table_list(); foreach my $table (@tables) { - my @columns = $self->_bz_schema->get_table_columns($table); + my @columns = $self->bz_table_columns($table); my %add_fks; foreach my $column (@columns) { - my $def = $self->_bz_schema->get_column_abstract($table, $column); - if ($def->{REFERENCES}) { - $add_fks{$column} = $def->{REFERENCES}; + # First we check for any FKs that have created => 0, + # in the _bz_real_schema. This also picks up FKs with + # created => 1, but bz_add_fks will ignore those. + my $fk = $self->bz_fk_info($table, $column); + # Then we check the abstract schema to see if there + # should be an FK on this column, but one wasn't set in the + # _bz_real_schema for some reason. We do this to handle + # various problems caused by upgrading from versions + # prior to 4.2, and also to handle problems caused + # by enabling an extension pre-4.2, disabling it for + # the 4.2 upgrade, and then re-enabling it later. + if (!$fk) { + my $standard_def = + $self->_bz_schema->get_column_abstract($table, $column); + if (exists $standard_def->{REFERENCES}) { + $fk = dclone($standard_def->{REFERENCES}); + } } + + $add_fks{$column} = $fk if $fk; } - $self->bz_add_fks($table, \%add_fks, { silently => !$activity_fk }); + $self->bz_add_fks($table, \%add_fks, { silently => !$any_fks }); } } @@ -484,9 +502,9 @@ sub bz_setup_foreign_keys { sub bz_drop_foreign_keys { my ($self) = @_; - my @tables = $self->_bz_real_schema->get_table_list(); + my @tables = $self->bz_table_list(); foreach my $table (@tables) { - my @columns = $self->_bz_real_schema->get_table_columns($table); + my @columns = $self->bz_table_columns($table); foreach my $column (@columns) { $self->bz_drop_fk($table, $column); } @@ -523,6 +541,20 @@ sub bz_add_column { foreach my $sql (@statements) { $self->do($sql); } + + # To make things easier for callers, if they don't specify + # a REFERENCES item, we pull it from the _bz_schema if the + # column exists there and has a REFERENCES item. + # bz_setup_foreign_keys will then add this FK at the end of + # Install::DB. + my $col_abstract = + $self->_bz_schema->get_column_abstract($table, $name); + if (exists $col_abstract->{REFERENCES}) { + my $new_fk = dclone($col_abstract->{REFERENCES}); + $new_fk->{created} = 0; + $new_def->{REFERENCES} = $new_fk; + } + $self->_bz_real_schema->set_column($table, $name, $new_def); $self->_bz_store_real_schema; } @@ -538,17 +570,17 @@ sub bz_add_fks { my %add_these; foreach my $column (keys %$column_fks) { - my $col_def = $self->bz_column_info($table, $column); - next if $col_def->{REFERENCES}; - my $fk = $column_fks->{$column}; - $self->_check_references($table, $column, $fk); - $add_these{$column} = $fk; + my $current_fk = $self->bz_fk_info($table, $column); + next if ($current_fk and $current_fk->{created}); + my $new_fk = $column_fks->{$column}; + $self->_check_references($table, $column, $new_fk); + $add_these{$column} = $new_fk; if (Bugzilla->usage_mode == USAGE_MODE_CMDLINE and !$options->{silently}) { print get_text('install_fk_add', - { table => $table, column => $column, fk => $fk }), - "\n"; + { table => $table, column => $column, + fk => $new_fk }), "\n"; } } @@ -558,9 +590,9 @@ sub bz_add_fks { $self->do($_) foreach @sql; foreach my $column (keys %add_these) { - my $col_def = $self->bz_column_info($table, $column); - $col_def->{REFERENCES} = $add_these{$column}; - $self->_bz_real_schema->set_column($table, $column, $col_def); + my $fk_def = $add_these{$column}; + $fk_def->{created} = 1; + $self->_bz_real_schema->set_fk($table, $column, $fk_def); } $self->_bz_store_real_schema(); @@ -586,10 +618,8 @@ sub bz_alter_column { } # Preserve foreign key definitions in the Schema object when altering # types. - if (defined $current_def->{REFERENCES}) { - # Make sure we don't modify the caller's $new_def. - $new_def = dclone($new_def); - $new_def->{REFERENCES} = $current_def->{REFERENCES}; + if (my $fk = $self->bz_fk_info($table, $name)) { + $new_def->{REFERENCES} = $fk; } $self->bz_alter_column_raw($table, $name, $new_def, $current_def, $set_nulls_to); @@ -635,6 +665,15 @@ sub bz_alter_column_raw { $self->do($_) foreach (@statements); } +sub bz_alter_fk { + my ($self, $table, $column, $fk_def) = @_; + my $current_fk = $self->bz_fk_info($table, $column); + ThrowCodeError('column_alter_nonexistent_fk', + { table => $table, column => $column }) if !$current_fk; + $self->bz_drop_fk($table, $column); + $self->bz_add_fk($table, $column, $fk_def); +} + sub bz_add_index { my ($self, $table, $name, $definition) = @_; @@ -686,7 +725,9 @@ sub bz_add_table { # initial table creation, because column names have changed # over history and it's impossible to keep track of that info # in ABSTRACT_SCHEMA. - delete $fields{$col}->{REFERENCES}; + if (exists $fields{$col}->{REFERENCES}) { + $fields{$col}->{REFERENCES}->{created} = 0; + } } $self->_bz_real_schema->add_table($name, $table_def); $self->_bz_store_real_schema; @@ -790,22 +831,25 @@ sub bz_drop_column { sub bz_drop_fk { my ($self, $table, $column) = @_; - my $col_def = $self->bz_column_info($table, $column); - if ($col_def && exists $col_def->{REFERENCES}) { - my $def = $col_def->{REFERENCES}; + my $fk_def = $self->bz_fk_info($table, $column); + if ($fk_def and $fk_def->{created}) { print get_text('install_fk_drop', - { table => $table, column => $column, fk => $def }) + { table => $table, column => $column, fk => $fk_def }) . "\n" if Bugzilla->usage_mode == USAGE_MODE_CMDLINE; my @statements = - $self->_bz_real_schema->get_drop_fk_sql($table,$column,$def); + $self->_bz_real_schema->get_drop_fk_sql($table, $column, $fk_def); foreach my $sql (@statements) { # Because this is a deletion, we don't want to die hard if # we fail because of some local customization. If something # is already gone, that's fine with us! eval { $self->do($sql); } or warn "Failed SQL: [$sql] Error: $@"; } - delete $col_def->{REFERENCES}; - $self->_bz_real_schema->set_column($table, $column, $col_def); + # Under normal circumstances, we don't permanently drop the fk-- + # we want checksetup to re-create it again later. The only + # time that FKs get permanently dropped is if the column gets + # dropped. + $fk_def->{created} = 0; + $self->_bz_real_schema->set_fk($table, $column, $fk_def); $self->_bz_store_real_schema; } @@ -818,8 +862,7 @@ sub bz_get_related_fks { foreach my $check_table (@tables) { my @columns = $self->bz_table_columns($check_table); foreach my $check_column (@columns) { - my $def = $self->bz_column_info($check_table, $check_column); - my $fk = $def->{REFERENCES}; + my $fk = $self->bz_fk_info($check_table, $check_column); if ($fk and (($fk->{TABLE} eq $table and $fk->{COLUMN} eq $column) or ($check_column eq $column and $check_table eq $table))) @@ -1031,6 +1074,11 @@ sub bz_table_indexes { return \%return_indexes; } +sub bz_table_list { + my ($self) = @_; + return $self->_bz_real_schema->get_table_list(); +} + ##################################################################### # Protected "Real Database" Schema Information Methods ##################################################################### diff --git a/Bugzilla/DB/Mysql.pm b/Bugzilla/DB/Mysql.pm index 66a261c75..699fcfdf6 100644 --- a/Bugzilla/DB/Mysql.pm +++ b/Bugzilla/DB/Mysql.pm @@ -721,7 +721,6 @@ EOT print "Converting table storage format to UTF-8. This may take a", " while.\n"; - my @dropped_fks; foreach my $table ($self->bz_table_list_real) { my $info_sth = $self->prepare("SHOW FULL COLUMNS FROM $table"); $info_sth->execute(); @@ -740,8 +739,9 @@ EOT print "$table.$name needs to be converted to UTF-8...\n"; - my $dropped = $self->bz_drop_related_fks($table, $name); - push(@dropped_fks, @$dropped); + # These will be automatically re-created at the end + # of checksetup. + $self->bz_drop_related_fks($table, $name); my $col_info = $self->bz_column_info_real($table, $name); @@ -792,10 +792,6 @@ EOT } } # foreach my $table (@tables) - - foreach my $fk_args (@dropped_fks) { - $self->bz_add_fk(@$fk_args); - } } # Sometimes you can have a situation where all the tables are utf8, diff --git a/Bugzilla/DB/Oracle.pm b/Bugzilla/DB/Oracle.pm index 0819bd19a..a7ac6e93e 100644 --- a/Bugzilla/DB/Oracle.pm +++ b/Bugzilla/DB/Oracle.pm @@ -247,7 +247,7 @@ sub bz_drop_table { # Dropping all FKs for a specified table. sub _bz_drop_fks { my ($self, $table) = @_; - my @columns = $self->_bz_real_schema->get_table_columns($table); + my @columns = $self->bz_table_columns($table); foreach my $column (@columns) { $self->bz_drop_fk($table, $column); } diff --git a/Bugzilla/DB/Schema.pm b/Bugzilla/DB/Schema.pm index 4837ccc5f..a00a7b9d9 100644 --- a/Bugzilla/DB/Schema.pm +++ b/Bugzilla/DB/Schema.pm @@ -44,12 +44,12 @@ use Bugzilla::Constants; use Carp qw(confess); use Digest::MD5 qw(md5_hex); use Hash::Util qw(lock_value unlock_hash lock_keys unlock_keys); -use List::MoreUtils qw(firstidx); +use List::MoreUtils qw(firstidx natatime); use Safe; # Historical, needed for SCHEMA_VERSION = '1.00' use Storable qw(dclone freeze thaw); -# New SCHEMA_VERSION (2.00) use this +# New SCHEMA_VERSIONs (2+) use this use Data::Dumper; =head1 NAME @@ -208,7 +208,7 @@ update this column in this table." =cut -use constant SCHEMA_VERSION => '2.00'; +use constant SCHEMA_VERSION => 3; use constant ADD_COLUMN => 'ADD COLUMN'; # Multiple FKs can be added using ALTER TABLE ADD CONSTRAINT in one # SQL statement. This isn't true for all databases. @@ -2545,6 +2545,28 @@ sub set_column { $self->_set_object($table, $column, $new_def, $fields); } +=item C + +Sets the C item on the specified column. + +=cut + +sub set_fk { + my ($self, $table, $column, $fk_def) = @_; + # Don't want to modify the source def before we explicitly set it below. + # This is just us being extra-cautious. + my $column_def = dclone($self->get_column_abstract($table, $column)); + die "Tried to set an fk on $table.$column, but that column doesn't exist" + if !$column_def; + if ($fk_def) { + $column_def->{REFERENCES} = $fk_def; + } + else { + delete $column_def->{REFERENCES}; + } + $self->set_column($table, $column, $column_def); +} + sub set_index { =item C @@ -2697,7 +2719,7 @@ sub serialize_abstract { Description: Used for when you've read a serialized Schema off the disk, and you want a Schema object that represents that data. Params: $serialized - scalar. The serialized data. - $version - A number in the format X.YZ. The "version" + $version - A number. The "version" of the Schema that did the serialization. See the docs for C for more details. Returns: A Schema object. It will have the methods of (and work @@ -2710,7 +2732,7 @@ sub deserialize_abstract { my ($class, $serialized, $version) = @_; my $thawed_hash; - if (int($version) < 2) { + if ($version < 2) { $thawed_hash = thaw($serialized); } else { @@ -2720,6 +2742,22 @@ sub deserialize_abstract { $thawed_hash = ${$cpt->varglob('VAR1')}; } + # Version 2 didn't have the "created" key for REFERENCES items. + if ($version < 3) { + my $standard = $class->new()->{abstract_schema}; + foreach my $table_name (keys %$thawed_hash) { + my %standard_fields = + @{ $standard->{$table_name}->{FIELDS} || [] }; + my $table = $thawed_hash->{$table_name}; + my %fields = @{ $table->{FIELDS} || [] }; + while (my ($field, $def) = each %fields) { + if (exists $def->{REFERENCES}) { + $def->{REFERENCES}->{created} = 1; + } + } + } + } + return $class->new(undef, $thawed_hash); } diff --git a/Bugzilla/Install/DB.pm b/Bugzilla/Install/DB.pm index 26e36f7df..225e862de 100644 --- a/Bugzilla/Install/DB.pm +++ b/Bugzilla/Install/DB.pm @@ -3155,13 +3155,11 @@ sub _add_foreign_keys_to_multiselects { WHERE type = ' . FIELD_TYPE_MULTI_SELECT); foreach my $name (@$names) { - $dbh->bz_add_fk("bug_$name", "bug_id", {TABLE => 'bugs', - COLUMN => 'bug_id', - DELETE => 'CASCADE',}); + $dbh->bz_add_fk("bug_$name", "bug_id", + {TABLE => 'bugs', COLUMN => 'bug_id', DELETE => 'CASCADE'}); - $dbh->bz_add_fk("bug_$name", "value", {TABLE => $name, - COLUMN => 'value', - DELETE => 'RESTRICT',}); + $dbh->bz_add_fk("bug_$name", "value", + {TABLE => $name, COLUMN => 'value', DELETE => 'RESTRICT'}); } } @@ -3380,9 +3378,8 @@ sub _convert_flagtypes_fks_to_set_null { foreach my $column (qw(request_group_id grant_group_id)) { my $fk = $dbh->bz_fk_info('flagtypes', $column); if ($fk and !defined $fk->{DELETE}) { - # checksetup will re-create the FK with the appropriate definition - # at the end of its table upgrades, so we just drop it here. - $dbh->bz_drop_fk('flagtypes', $column); + $fk->{DELETE} = 'SET NULL'; + $dbh->bz_alter_fk('flagtypes', $column, $fk); } } } @@ -3398,10 +3395,9 @@ sub _fix_decimal_types { sub _fix_series_creator_fk { my $dbh = Bugzilla->dbh; my $fk = $dbh->bz_fk_info('series', 'creator'); - # Change the FK from SET NULL to CASCADE. (It will be re-created - # automatically at the end of all DB changes.) if ($fk and $fk->{DELETE} eq 'SET NULL') { - $dbh->bz_drop_fk('series', 'creator'); + $fk->{DELETE} = 'CASCADE'; + $dbh->bz_alter_fk('series', 'creator', $fk); } } -- cgit v1.2.3-24-g4f1b