From c1f804a0a609e6e7376bda2ed3adf3d450c3569f Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Thu, 12 Jan 2017 10:44:46 +0100 Subject: [PATCH] Bug 17234: Need to separate KEY and FOREIGN KEY checks In the previous patch we use the constraint_exists subroutine to verify if an index or a foreign key exists. But the `SHOW INDEX` query does not return foreign keys (as its name suggests!). We need another subroutine foreign_key_exists to check the FK existence. I have found that because t/db_dependent/TestBuilder.t fails on oai_sets_biblios, because oai_sets_biblios_ibfk_1 has not been removed. Test plan: 0/ Do not apply this patch 1/ Use a 3.20 DB 2/ update the DB 3/ SHOW CREATE TABLE oai_sets_biblios will display oai_sets_biblios_ibfk_1 Apply the patch and repeat 1, 2, 3 => Will not display oai_sets_biblios_ibfk_1 It has been removed as expected. --- C4/Installer.pm | 11 +++++++++-- installer/data/mysql/updatedatabase.pl | 32 ++++++++++++++++---------------- t/db_dependent/Installer.t | 9 ++++++--- 3 files changed, 31 insertions(+), 21 deletions(-) diff --git a/C4/Installer.pm b/C4/Installer.pm index 4165183..bd1b150 100644 --- a/C4/Installer.pm +++ b/C4/Installer.pm @@ -30,7 +30,7 @@ use vars qw(@ISA @EXPORT); BEGIN { require Exporter; @ISA = qw( Exporter ); - push @EXPORT, qw( constraint_exists column_exists ); + push @EXPORT, qw( foreign_key_exists index_exists column_exists ); }; =head1 NAME @@ -498,7 +498,14 @@ sub get_file_path_from_name { } -sub constraint_exists { +sub foreign_key_exists { + my ( $table_name, $constraint_name ) = @_; + my $dbh = C4::Context->dbh; + my (undef, $infos) = $dbh->selectrow_array(qq|SHOW CREATE TABLE $table_name|); + return $infos =~ m|CONSTRAINT `$constraint_name` FOREIGN KEY|; +} + +sub index_exists { my ( $table_name, $key_name ) = @_; my $dbh = C4::Context->dbh; my ($exists) = $dbh->selectrow_array( diff --git a/installer/data/mysql/updatedatabase.pl b/installer/data/mysql/updatedatabase.pl index fcf4d27..9f29a39 100755 --- a/installer/data/mysql/updatedatabase.pl +++ b/installer/data/mysql/updatedatabase.pl @@ -10422,17 +10422,17 @@ if ( CheckVersion($DBversion) ) { $DBversion = "3.19.00.041"; if ( CheckVersion($DBversion) ) { - unless ( constraint_exists( 'suggestions', 'status' ) ) { + unless ( index_exists( 'suggestions', 'status' ) ) { $dbh->do(q| ALTER TABLE suggestions ADD KEY status (STATUS) |); } - unless ( constraint_exists( 'suggestions', 'biblionumber' ) ) { + unless ( index_exists( 'suggestions', 'biblionumber' ) ) { $dbh->do(q| ALTER TABLE suggestions ADD KEY biblionumber (biblionumber) |); } - unless ( constraint_exists( 'suggestions', 'branchcode' ) ) { + unless ( index_exists( 'suggestions', 'branchcode' ) ) { $dbh->do(q| ALTER TABLE suggestions ADD KEY branchcode (branchcode) |); @@ -10450,7 +10450,7 @@ if ( CheckVersion($DBversion) ) { WHERE auth_types.authtypecode IS NULL }); - unless ( constraint_exists( 'auth_subfield_structure', 'auth_subfield_structure_ibfk_1' ) ) { + unless ( foreign_key_exists( 'auth_subfield_structure', 'auth_subfield_structure_ibfk_1' ) ) { $dbh->do(q{ ALTER TABLE auth_subfield_structure ADD CONSTRAINT auth_subfield_structure_ibfk_1 @@ -10573,54 +10573,54 @@ if ( CheckVersion($DBversion) ) { $DBversion = "3.21.00.007"; if ( CheckVersion($DBversion) ) { - unless ( constraint_exists( 'aqbasket', 'authorisedby' ) ) { + unless ( index_exists( 'aqbasket', 'authorisedby' ) ) { $dbh->do(q| ALTER TABLE aqbasket ADD KEY authorisedby (authorisedby) |); } - unless ( constraint_exists( 'aqbooksellers', 'name' ) ) { + unless ( index_exists( 'aqbooksellers', 'name' ) ) { $dbh->do(q| ALTER TABLE aqbooksellers ADD KEY name (name(255)) |); } - unless ( constraint_exists( 'aqbudgets', 'budget_parent_id' ) ) { + unless ( index_exists( 'aqbudgets', 'budget_parent_id' ) ) { $dbh->do(q| ALTER TABLE aqbudgets ADD KEY budget_parent_id (budget_parent_id)|); } - unless ( constraint_exists( 'aqbudgets', 'budget_code' ) ) { + unless ( index_exists( 'aqbudgets', 'budget_code' ) ) { $dbh->do(q| ALTER TABLE aqbudgets ADD KEY budget_code (budget_code)|); } - unless ( constraint_exists( 'aqbudgets', 'budget_branchcode' ) ) { + unless ( index_exists( 'aqbudgets', 'budget_branchcode' ) ) { $dbh->do(q| ALTER TABLE aqbudgets ADD KEY budget_branchcode (budget_branchcode)|); } - unless ( constraint_exists( 'aqbudgets', 'budget_period_id' ) ) { + unless ( index_exists( 'aqbudgets', 'budget_period_id' ) ) { $dbh->do(q| ALTER TABLE aqbudgets ADD KEY budget_period_id (budget_period_id)|); } - unless ( constraint_exists( 'aqbudgets', 'budget_owner_id' ) ) { + unless ( index_exists( 'aqbudgets', 'budget_owner_id' ) ) { $dbh->do(q| ALTER TABLE aqbudgets ADD KEY budget_owner_id (budget_owner_id)|); } - unless ( constraint_exists( 'aqbudgets_planning', 'budget_period_id' ) ) { + unless ( index_exists( 'aqbudgets_planning', 'budget_period_id' ) ) { $dbh->do(q| ALTER TABLE aqbudgets_planning ADD KEY budget_period_id (budget_period_id)|); } - unless ( constraint_exists( 'aqorders', 'parent_ordernumber' ) ) { + unless ( index_exists( 'aqorders', 'parent_ordernumber' ) ) { $dbh->do(q| ALTER TABLE aqorders ADD KEY parent_ordernumber (parent_ordernumber)|); } - unless ( constraint_exists( 'aqorders', 'orderstatus' ) ) { + unless ( index_exists( 'aqorders', 'orderstatus' ) ) { $dbh->do(q| ALTER TABLE aqorders ADD KEY orderstatus (orderstatus)|); @@ -10726,7 +10726,7 @@ if ( CheckVersion($DBversion) ) { VALUES ('OAI-PMH:DeletedRecord','persistent','Koha\'s deletedbiblio table will never be deleted (persistent) or might be deleted (transient)','transient|persistent','Choice') }); - if ( constraint_exists( 'oai_sets_biblios', 'oai_sets_biblios_ibfk_1' ) ) { + if ( foreign_key_exists( 'oai_sets_biblios', 'oai_sets_biblios_ibfk_1' ) ) { $dbh->do(q| ALTER TABLE oai_sets_biblios DROP FOREIGN KEY oai_sets_biblios_ibfk_1 |); @@ -10858,7 +10858,7 @@ if ( CheckVersion($DBversion) ) { my ($print_error) = $dbh->{PrintError}; $dbh->{RaiseError} = 0; $dbh->{PrintError} = 0; - if ( constraint_exists('course_reserves', 'course_reserves_ibfk_2') ) { + if ( foreign_key_exists('course_reserves', 'course_reserves_ibfk_2') ) { $dbh->do(q{ALTER TABLE course_reserves DROP FOREIGN KEY course_reserves_ibfk_2}); $dbh->do(q{ALTER TABLE course_reserves DROP INDEX course_reserves_ibfk_2}); } diff --git a/t/db_dependent/Installer.t b/t/db_dependent/Installer.t index 65875b0..0b4cfa2 100644 --- a/t/db_dependent/Installer.t +++ b/t/db_dependent/Installer.t @@ -22,7 +22,7 @@ # Add more tests here!!! use Modern::Perl; -use Test::More tests => 13; +use Test::More tests => 15; use Koha::Database; BEGIN { @@ -59,5 +59,8 @@ ok( ! column_exists( 'borrowers', 'xxx'), 'Column xxx does not exist' ); my @constraint_names = $source->unique_constraint_names(); my $constraint_name = $constraint_names[0]; -ok( constraint_exists( 'borrowers', $constraint_name), 'Known contraint does exist' ); -ok( ! constraint_exists( 'borrowers', 'xxx'), 'Constraint xxx does not exist' ); +ok( index_exists( 'borrowers', $constraint_name), 'Known contraint does exist' ); +ok( ! index_exists( 'borrowers', 'xxx'), 'Constraint xxx does not exist' ); + +ok( foreign_key_exists( 'borrowers', 'borrowers_ibfk_1' ), 'FK borrowers_ibfk_1 exists' ); +ok( ! foreign_key_exists( 'borrowers', 'xxx' ), 'FK xxxx does not exist' ); -- 2.9.3