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.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

diff --git a/C4/Installer.pm b/C4/Installer.pm
index 4165183..bd1b150 100644 (file)
--- 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 (executable)
--- 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 (file)
--- 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' );