From 0ac4d26f78657a3bb089711cac2fd5f76d077437 Mon Sep 17 00:00:00 2001 From: Max Kanat-Alexander Date: Tue, 1 Jun 2010 13:33:49 -0700 Subject: Bug 569312: Speed up the adding of many FKs to the same table for MySQL and PostgreSQL, by adding them all in one ALTER statement r=mkanat, a=mkanat (module owner) --- Bugzilla/DB.pm | 47 +++++++++++++++++++++++---------- Bugzilla/DB/Schema.pm | 26 +++++++++++++++--- Bugzilla/DB/Schema/Oracle.pm | 63 ++++++++++++++++++++++++-------------------- 3 files changed, 89 insertions(+), 47 deletions(-) diff --git a/Bugzilla/DB.pm b/Bugzilla/DB.pm index e6c29e0b0..79a5639dd 100644 --- a/Bugzilla/DB.pm +++ b/Bugzilla/DB.pm @@ -447,12 +447,14 @@ sub bz_setup_foreign_keys { my @tables = $self->_bz_schema->get_table_list(); foreach my $table (@tables) { my @columns = $self->_bz_schema->get_table_columns($table); + my %add_fks; foreach my $column (@columns) { my $def = $self->_bz_schema->get_column_abstract($table, $column); if ($def->{REFERENCES}) { - $self->bz_add_fk($table, $column, $def->{REFERENCES}); + $add_fks{$column} = $def->{REFERENCES}; } } + $self->bz_add_fks($table, \%add_fks); } } @@ -506,19 +508,36 @@ sub bz_add_column { sub bz_add_fk { my ($self, $table, $column, $def) = @_; + $self->bz_add_fks($table, { $column => $def }); +} - my $col_def = $self->bz_column_info($table, $column); - if (!$col_def->{REFERENCES}) { - $self->_check_references($table, $column, $def); +sub bz_add_fks { + my ($self, $table, $column_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; print get_text('install_fk_add', - { table => $table, column => $column, fk => $def }) + { table => $table, column => $column, fk => $fk }) . "\n" if Bugzilla->usage_mode == USAGE_MODE_CMDLINE; - my @sql = $self->_bz_real_schema->get_add_fk_sql($table, $column, $def); - $self->do($_) foreach @sql; - $col_def->{REFERENCES} = $def; + } + + return if !scalar(keys %add_these); + + my @sql = $self->_bz_real_schema->get_add_fks_sql($table, \%add_these); + $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); - $self->_bz_store_real_schema; } + + $self->_bz_store_real_schema(); } sub bz_alter_column { @@ -700,11 +719,11 @@ sub bz_add_field_tables { $self->_bz_add_field_table($ms_table, $self->_bz_schema->MULTI_SELECT_VALUE_TABLE); - $self->bz_add_fk($ms_table, 'bug_id', {TABLE => 'bugs', - COLUMN => 'bug_id', - DELETE => 'CASCADE'}); - $self->bz_add_fk($ms_table, 'value', {TABLE => $field->name, - COLUMN => 'value'}); + $self->bz_add_fks($ms_table, + { bug_id => {TABLE => 'bugs', COLUMN => 'bug_id', + DELETE => 'CASCADE'}, + + value => {TABLE => $field->name, COLUMN => 'value'} }); } } diff --git a/Bugzilla/DB/Schema.pm b/Bugzilla/DB/Schema.pm index 5da55cf26..97e59efbd 100644 --- a/Bugzilla/DB/Schema.pm +++ b/Bugzilla/DB/Schema.pm @@ -210,6 +210,9 @@ update this column in this table." use constant SCHEMA_VERSION => '2.00'; 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. +use constant MULTIPLE_FKS_IN_ALTER => 1; # This is a reasonable default that's true for both PostgreSQL and MySQL. use constant MAX_IDENTIFIER_LEN => 63; @@ -1817,11 +1820,26 @@ sub _hash_identifier { } -sub get_add_fk_sql { - my ($self, $table, $column, $def) = @_; +sub get_add_fks_sql { + my ($self, $table, $column_fks) = @_; - my $fk_string = $self->get_fk_ddl($table, $column, $def); - return ("ALTER TABLE $table ADD $fk_string"); + my @add; + foreach my $column (keys %$column_fks) { + my $def = $column_fks->{$column}; + my $fk_string = $self->get_fk_ddl($table, $column, $def); + push(@add, $fk_string); + } + my @sql; + if ($self->MULTIPLE_FKS_IN_ALTER) { + my $alter = "ALTER TABLE $table ADD " . join(', ADD ', @add); + push(@sql, $alter); + } + else { + foreach my $fk_string (@add) { + push(@sql, "ALTER TABLE $table ADD $fk_string"); + } + } + return @sql; } sub get_drop_fk_sql { diff --git a/Bugzilla/DB/Schema/Oracle.pm b/Bugzilla/DB/Schema/Oracle.pm index e8905eb80..af0c11e9f 100644 --- a/Bugzilla/DB/Schema/Oracle.pm +++ b/Bugzilla/DB/Schema/Oracle.pm @@ -35,6 +35,7 @@ use Carp qw(confess); use Bugzilla::Util; use constant ADD_COLUMN => 'ADD'; +use constant MULTIPLE_FKS_IN_ALTER => 0; # Whether this is true or not, this is what it needs to be in order for # hash_identifier to maintain backwards compatibility with versions before # 3.2rc2. @@ -136,40 +137,44 @@ sub get_drop_index_ddl { # - Delete CASCADE # - Delete SET NULL sub get_fk_ddl { - my ($self, $table, $column, $references) = @_; - return "" if !$references; + my $self = shift; + my $ddl = $self->SUPER::get_fk_ddl(@_); - my $update = $references->{UPDATE} || 'CASCADE'; - my $delete = $references->{DELETE}; - my $to_table = $references->{TABLE} || confess "No table in reference"; - my $to_column = $references->{COLUMN} || confess "No column in reference"; - my $fk_name = $self->_get_fk_name($table, $column, $references); + # iThe Bugzilla Oracle driver implements UPDATE via a trigger. + $ddl =~ s/ON UPDATE \S+//i; + # RESTRICT is the default for DELETE on Oracle and may not be specified. + $ddl =~ s/ON DELETE RESTRICT//i; - # 'ON DELETE RESTRICT' is enabled by default - $delete = "" if ( defined $delete && $delete =~ /RESTRICT/i); + return $ddl; +} - my $fk_string = "\n CONSTRAINT $fk_name FOREIGN KEY ($column)\n" - . " REFERENCES $to_table($to_column)\n"; - - $fk_string = $fk_string . " ON DELETE $delete" if $delete; - - if ( $update =~ /CASCADE/i ){ - my $tr_str = "CREATE OR REPLACE TRIGGER ${fk_name}_UC" - . " AFTER UPDATE OF $to_column ON $to_table " - . " REFERENCING " - . " NEW AS NEW " - . " OLD AS OLD " - . " FOR EACH ROW " - . " BEGIN " - . " UPDATE $table" - . " SET $column = :NEW.$to_column" - . " WHERE $column = :OLD.$to_column;" - . " END ${fk_name}_UC;"; - my $dbh = Bugzilla->dbh; - $dbh->do($tr_str); +sub get_add_fks_sql { + my $self = shift; + my ($table, $column_fks) = @_; + my @sql = $self->SUPER::get_add_fks_sql(@_); + + foreach my $column (keys %$column_fks) { + my $fk = $column_fks->{$column}; + next if $fk->{UPDATE} && uc($fk->{UPDATE}) ne 'CASCADE'; + my $fk_name = $self->_get_fk_name($table, $column, $fk); + my $to_column = $fk->{COLUMN}; + my $to_table = $fk->{TABLE}; + + my $trigger = <