Commit e6ea9c39 authored by Max Kanat-Alexander's avatar Max Kanat-Alexander

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)
parent 0fd1b7d5
...@@ -421,7 +421,10 @@ sub bz_setup_database { ...@@ -421,7 +421,10 @@ sub bz_setup_database {
# If we haven't ever stored a serialized schema, # If we haven't ever stored a serialized schema,
# set up the bz_schema table and store it. # set up the bz_schema table and store it.
$self->_bz_init_schema_storage(); $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 @desired_tables = $self->_bz_schema->get_table_list();
my $bugs_exists = $self->bz_table_info('bugs'); my $bugs_exists = $self->bz_table_info('bugs');
if (!$bugs_exists) { if (!$bugs_exists) {
...@@ -460,23 +463,38 @@ sub bz_setup_foreign_keys { ...@@ -460,23 +463,38 @@ sub bz_setup_foreign_keys {
# so if it doesn't have them, then we're setting up FKs # so if it doesn't have them, then we're setting up FKs
# for the first time, and should be quieter about it. # for the first time, and should be quieter about it.
my $activity_fk = $self->bz_fk_info('profiles_activity', 'userid'); 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"; print get_text('install_fk_setup'), "\n";
} }
# We use _bz_schema because bz_add_table has removed all REFERENCES my @tables = $self->bz_table_list();
# items from _bz_real_schema.
my @tables = $self->_bz_schema->get_table_list();
foreach my $table (@tables) { foreach my $table (@tables) {
my @columns = $self->_bz_schema->get_table_columns($table); my @columns = $self->bz_table_columns($table);
my %add_fks; my %add_fks;
foreach my $column (@columns) { foreach my $column (@columns) {
my $def = $self->_bz_schema->get_column_abstract($table, $column); # First we check for any FKs that have created => 0,
if ($def->{REFERENCES}) { # in the _bz_real_schema. This also picks up FKs with
$add_fks{$column} = $def->{REFERENCES}; # 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 { ...@@ -484,9 +502,9 @@ sub bz_setup_foreign_keys {
sub bz_drop_foreign_keys { sub bz_drop_foreign_keys {
my ($self) = @_; my ($self) = @_;
my @tables = $self->_bz_real_schema->get_table_list(); my @tables = $self->bz_table_list();
foreach my $table (@tables) { 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) { foreach my $column (@columns) {
$self->bz_drop_fk($table, $column); $self->bz_drop_fk($table, $column);
} }
...@@ -523,6 +541,20 @@ sub bz_add_column { ...@@ -523,6 +541,20 @@ sub bz_add_column {
foreach my $sql (@statements) { foreach my $sql (@statements) {
$self->do($sql); $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_real_schema->set_column($table, $name, $new_def);
$self->_bz_store_real_schema; $self->_bz_store_real_schema;
} }
...@@ -538,17 +570,17 @@ sub bz_add_fks { ...@@ -538,17 +570,17 @@ sub bz_add_fks {
my %add_these; my %add_these;
foreach my $column (keys %$column_fks) { foreach my $column (keys %$column_fks) {
my $col_def = $self->bz_column_info($table, $column); my $current_fk = $self->bz_fk_info($table, $column);
next if $col_def->{REFERENCES}; next if ($current_fk and $current_fk->{created});
my $fk = $column_fks->{$column}; my $new_fk = $column_fks->{$column};
$self->_check_references($table, $column, $fk); $self->_check_references($table, $column, $new_fk);
$add_these{$column} = $fk; $add_these{$column} = $new_fk;
if (Bugzilla->usage_mode == USAGE_MODE_CMDLINE if (Bugzilla->usage_mode == USAGE_MODE_CMDLINE
and !$options->{silently}) and !$options->{silently})
{ {
print get_text('install_fk_add', print get_text('install_fk_add',
{ table => $table, column => $column, fk => $fk }), { table => $table, column => $column,
"\n"; fk => $new_fk }), "\n";
} }
} }
...@@ -558,9 +590,9 @@ sub bz_add_fks { ...@@ -558,9 +590,9 @@ sub bz_add_fks {
$self->do($_) foreach @sql; $self->do($_) foreach @sql;
foreach my $column (keys %add_these) { foreach my $column (keys %add_these) {
my $col_def = $self->bz_column_info($table, $column); my $fk_def = $add_these{$column};
$col_def->{REFERENCES} = $add_these{$column}; $fk_def->{created} = 1;
$self->_bz_real_schema->set_column($table, $column, $col_def); $self->_bz_real_schema->set_fk($table, $column, $fk_def);
} }
$self->_bz_store_real_schema(); $self->_bz_store_real_schema();
...@@ -586,10 +618,8 @@ sub bz_alter_column { ...@@ -586,10 +618,8 @@ sub bz_alter_column {
} }
# Preserve foreign key definitions in the Schema object when altering # Preserve foreign key definitions in the Schema object when altering
# types. # types.
if (defined $current_def->{REFERENCES}) { if (my $fk = $self->bz_fk_info($table, $name)) {
# Make sure we don't modify the caller's $new_def. $new_def->{REFERENCES} = $fk;
$new_def = dclone($new_def);
$new_def->{REFERENCES} = $current_def->{REFERENCES};
} }
$self->bz_alter_column_raw($table, $name, $new_def, $current_def, $self->bz_alter_column_raw($table, $name, $new_def, $current_def,
$set_nulls_to); $set_nulls_to);
...@@ -635,6 +665,15 @@ sub bz_alter_column_raw { ...@@ -635,6 +665,15 @@ sub bz_alter_column_raw {
$self->do($_) foreach (@statements); $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 { sub bz_add_index {
my ($self, $table, $name, $definition) = @_; my ($self, $table, $name, $definition) = @_;
...@@ -686,7 +725,9 @@ sub bz_add_table { ...@@ -686,7 +725,9 @@ sub bz_add_table {
# initial table creation, because column names have changed # initial table creation, because column names have changed
# over history and it's impossible to keep track of that info # over history and it's impossible to keep track of that info
# in ABSTRACT_SCHEMA. # 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_real_schema->add_table($name, $table_def);
$self->_bz_store_real_schema; $self->_bz_store_real_schema;
...@@ -790,22 +831,25 @@ sub bz_drop_column { ...@@ -790,22 +831,25 @@ sub bz_drop_column {
sub bz_drop_fk { sub bz_drop_fk {
my ($self, $table, $column) = @_; my ($self, $table, $column) = @_;
my $col_def = $self->bz_column_info($table, $column); my $fk_def = $self->bz_fk_info($table, $column);
if ($col_def && exists $col_def->{REFERENCES}) { if ($fk_def and $fk_def->{created}) {
my $def = $col_def->{REFERENCES};
print get_text('install_fk_drop', 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; . "\n" if Bugzilla->usage_mode == USAGE_MODE_CMDLINE;
my @statements = 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) { foreach my $sql (@statements) {
# Because this is a deletion, we don't want to die hard if # Because this is a deletion, we don't want to die hard if
# we fail because of some local customization. If something # we fail because of some local customization. If something
# is already gone, that's fine with us! # is already gone, that's fine with us!
eval { $self->do($sql); } or warn "Failed SQL: [$sql] Error: $@"; eval { $self->do($sql); } or warn "Failed SQL: [$sql] Error: $@";
} }
delete $col_def->{REFERENCES}; # Under normal circumstances, we don't permanently drop the fk--
$self->_bz_real_schema->set_column($table, $column, $col_def); # 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; $self->_bz_store_real_schema;
} }
...@@ -818,8 +862,7 @@ sub bz_get_related_fks { ...@@ -818,8 +862,7 @@ sub bz_get_related_fks {
foreach my $check_table (@tables) { foreach my $check_table (@tables) {
my @columns = $self->bz_table_columns($check_table); my @columns = $self->bz_table_columns($check_table);
foreach my $check_column (@columns) { foreach my $check_column (@columns) {
my $def = $self->bz_column_info($check_table, $check_column); my $fk = $self->bz_fk_info($check_table, $check_column);
my $fk = $def->{REFERENCES};
if ($fk if ($fk
and (($fk->{TABLE} eq $table and $fk->{COLUMN} eq $column) and (($fk->{TABLE} eq $table and $fk->{COLUMN} eq $column)
or ($check_column eq $column and $check_table eq $table))) or ($check_column eq $column and $check_table eq $table)))
...@@ -1031,6 +1074,11 @@ sub bz_table_indexes { ...@@ -1031,6 +1074,11 @@ sub bz_table_indexes {
return \%return_indexes; return \%return_indexes;
} }
sub bz_table_list {
my ($self) = @_;
return $self->_bz_real_schema->get_table_list();
}
##################################################################### #####################################################################
# Protected "Real Database" Schema Information Methods # Protected "Real Database" Schema Information Methods
##################################################################### #####################################################################
......
...@@ -721,7 +721,6 @@ EOT ...@@ -721,7 +721,6 @@ EOT
print "Converting table storage format to UTF-8. This may take a", print "Converting table storage format to UTF-8. This may take a",
" while.\n"; " while.\n";
my @dropped_fks;
foreach my $table ($self->bz_table_list_real) { foreach my $table ($self->bz_table_list_real) {
my $info_sth = $self->prepare("SHOW FULL COLUMNS FROM $table"); my $info_sth = $self->prepare("SHOW FULL COLUMNS FROM $table");
$info_sth->execute(); $info_sth->execute();
...@@ -740,8 +739,9 @@ EOT ...@@ -740,8 +739,9 @@ EOT
print "$table.$name needs to be converted to UTF-8...\n"; print "$table.$name needs to be converted to UTF-8...\n";
my $dropped = $self->bz_drop_related_fks($table, $name); # These will be automatically re-created at the end
push(@dropped_fks, @$dropped); # of checksetup.
$self->bz_drop_related_fks($table, $name);
my $col_info = my $col_info =
$self->bz_column_info_real($table, $name); $self->bz_column_info_real($table, $name);
...@@ -792,10 +792,6 @@ EOT ...@@ -792,10 +792,6 @@ EOT
} }
} # foreach my $table (@tables) } # 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, # Sometimes you can have a situation where all the tables are utf8,
......
...@@ -247,7 +247,7 @@ sub bz_drop_table { ...@@ -247,7 +247,7 @@ sub bz_drop_table {
# Dropping all FKs for a specified table. # Dropping all FKs for a specified table.
sub _bz_drop_fks { sub _bz_drop_fks {
my ($self, $table) = @_; my ($self, $table) = @_;
my @columns = $self->_bz_real_schema->get_table_columns($table); my @columns = $self->bz_table_columns($table);
foreach my $column (@columns) { foreach my $column (@columns) {
$self->bz_drop_fk($table, $column); $self->bz_drop_fk($table, $column);
} }
......
...@@ -44,12 +44,12 @@ use Bugzilla::Constants; ...@@ -44,12 +44,12 @@ use Bugzilla::Constants;
use Carp qw(confess); use Carp qw(confess);
use Digest::MD5 qw(md5_hex); use Digest::MD5 qw(md5_hex);
use Hash::Util qw(lock_value unlock_hash lock_keys unlock_keys); 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; use Safe;
# Historical, needed for SCHEMA_VERSION = '1.00' # Historical, needed for SCHEMA_VERSION = '1.00'
use Storable qw(dclone freeze thaw); use Storable qw(dclone freeze thaw);
# New SCHEMA_VERSION (2.00) use this # New SCHEMA_VERSIONs (2+) use this
use Data::Dumper; use Data::Dumper;
=head1 NAME =head1 NAME
...@@ -208,7 +208,7 @@ update this column in this table." ...@@ -208,7 +208,7 @@ update this column in this table."
=cut =cut
use constant SCHEMA_VERSION => '2.00'; use constant SCHEMA_VERSION => 3;
use constant ADD_COLUMN => 'ADD COLUMN'; use constant ADD_COLUMN => 'ADD COLUMN';
# Multiple FKs can be added using ALTER TABLE ADD CONSTRAINT in one # Multiple FKs can be added using ALTER TABLE ADD CONSTRAINT in one
# SQL statement. This isn't true for all databases. # SQL statement. This isn't true for all databases.
...@@ -2545,6 +2545,28 @@ sub set_column { ...@@ -2545,6 +2545,28 @@ sub set_column {
$self->_set_object($table, $column, $new_def, $fields); $self->_set_object($table, $column, $new_def, $fields);
} }
=item C<set_fk($table, $column \%fk_def)>
Sets the C<REFERENCES> 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 { sub set_index {
=item C<set_index($table, $name, $definition)> =item C<set_index($table, $name, $definition)>
...@@ -2697,7 +2719,7 @@ sub serialize_abstract { ...@@ -2697,7 +2719,7 @@ sub serialize_abstract {
Description: Used for when you've read a serialized Schema off the disk, Description: Used for when you've read a serialized Schema off the disk,
and you want a Schema object that represents that data. and you want a Schema object that represents that data.
Params: $serialized - scalar. The serialized 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. of the Schema that did the serialization.
See the docs for C<SCHEMA_VERSION> for more details. See the docs for C<SCHEMA_VERSION> for more details.
Returns: A Schema object. It will have the methods of (and work Returns: A Schema object. It will have the methods of (and work
...@@ -2710,7 +2732,7 @@ sub deserialize_abstract { ...@@ -2710,7 +2732,7 @@ sub deserialize_abstract {
my ($class, $serialized, $version) = @_; my ($class, $serialized, $version) = @_;
my $thawed_hash; my $thawed_hash;
if (int($version) < 2) { if ($version < 2) {
$thawed_hash = thaw($serialized); $thawed_hash = thaw($serialized);
} }
else { else {
...@@ -2720,6 +2742,22 @@ sub deserialize_abstract { ...@@ -2720,6 +2742,22 @@ sub deserialize_abstract {
$thawed_hash = ${$cpt->varglob('VAR1')}; $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); return $class->new(undef, $thawed_hash);
} }
......
...@@ -3155,13 +3155,11 @@ sub _add_foreign_keys_to_multiselects { ...@@ -3155,13 +3155,11 @@ sub _add_foreign_keys_to_multiselects {
WHERE type = ' . FIELD_TYPE_MULTI_SELECT); WHERE type = ' . FIELD_TYPE_MULTI_SELECT);
foreach my $name (@$names) { foreach my $name (@$names) {
$dbh->bz_add_fk("bug_$name", "bug_id", {TABLE => 'bugs', $dbh->bz_add_fk("bug_$name", "bug_id",
COLUMN => 'bug_id', {TABLE => 'bugs', COLUMN => 'bug_id', DELETE => 'CASCADE'});
DELETE => 'CASCADE',});
$dbh->bz_add_fk("bug_$name", "value", {TABLE => $name, $dbh->bz_add_fk("bug_$name", "value",
COLUMN => 'value', {TABLE => $name, COLUMN => 'value', DELETE => 'RESTRICT'});
DELETE => 'RESTRICT',});
} }
} }
...@@ -3380,9 +3378,8 @@ sub _convert_flagtypes_fks_to_set_null { ...@@ -3380,9 +3378,8 @@ sub _convert_flagtypes_fks_to_set_null {
foreach my $column (qw(request_group_id grant_group_id)) { foreach my $column (qw(request_group_id grant_group_id)) {
my $fk = $dbh->bz_fk_info('flagtypes', $column); my $fk = $dbh->bz_fk_info('flagtypes', $column);
if ($fk and !defined $fk->{DELETE}) { if ($fk and !defined $fk->{DELETE}) {
# checksetup will re-create the FK with the appropriate definition $fk->{DELETE} = 'SET NULL';
# at the end of its table upgrades, so we just drop it here. $dbh->bz_alter_fk('flagtypes', $column, $fk);
$dbh->bz_drop_fk('flagtypes', $column);
} }
} }
} }
...@@ -3398,10 +3395,9 @@ sub _fix_decimal_types { ...@@ -3398,10 +3395,9 @@ sub _fix_decimal_types {
sub _fix_series_creator_fk { sub _fix_series_creator_fk {
my $dbh = Bugzilla->dbh; my $dbh = Bugzilla->dbh;
my $fk = $dbh->bz_fk_info('series', 'creator'); 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') { if ($fk and $fk->{DELETE} eq 'SET NULL') {
$dbh->bz_drop_fk('series', 'creator'); $fk->{DELETE} = 'CASCADE';
$dbh->bz_alter_fk('series', 'creator', $fk);
} }
} }
......
...@@ -95,7 +95,12 @@ ...@@ -95,7 +95,12 @@
[% ELSIF error == "chart_file_open_fail" %] [% ELSIF error == "chart_file_open_fail" %]
Unable to open the chart datafile <tt>[% filename FILTER html %]</tt>. Unable to open the chart datafile <tt>[% filename FILTER html %]</tt>.
[% ELSIF error == "column_alter_nonexistent_fk" %]
You attempted to modify the foreign key for
[%+ table FILTER html %].[% column FILTER html %], but there is
no foreign key on that column.
[% ELSIF error == "column_not_null_without_default" %] [% ELSIF error == "column_not_null_without_default" %]
Failed adding the column [% name FILTER html %]: Failed adding the column [% name FILTER html %]:
You cannot add a NOT NULL column with no default to an existing table You cannot add a NOT NULL column with no default to an existing table
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment