Bugzilla – Attachment 98177 Details for
Bug 21901
Foreign keys are missing on the serials and subscriptions tables
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 21901: Add FK on subscription and serial tables
Bug-21901-Add-FK-on-subscription-and-serial-tables.patch (text/plain), 13.54 KB, created by
Martin Renvoize (ashimema)
on 2020-01-30 16:11:38 UTC
(
hide
)
Description:
Bug 21901: Add FK on subscription and serial tables
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2020-01-30 16:11:38 UTC
Size:
13.54 KB
patch
obsolete
>From a129a2d89626db2848d3db12745aaee85f5fcc09 Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Date: Wed, 11 Dec 2019 08:29:27 +0100 >Subject: [PATCH] Bug 21901: Add FK on subscription and serial tables > >In order to improve performance in the serial modules and add DB constraints, >this patch is going to add missing foreign key on the following columns: > * serial.biblionumber > * serial.subscription > * subscriptionhistory.biblionumber > * subscriptionhistory.subscriptionid > * subscription.biblionumber > >Once done, some code can be removed from the Del* subroutines, as the ON >CASCASE clause will make the RDBMS handles the deletions. > >Test plan: >0/ It would be useful to test the update DB entry on a big and old >production DB, to make sure the constraints will be added correctly. >We could remove the entries before creating the constraints, but it can >be unecessary >1/ Make sure you can recreate a fresh install with the kohastructure.sql >from this patch >2/ Make sure you can upgrade from a master install >3/ Create a subscription, serial, etc. and delete the biblio >=> The subscription/serials should have been deleted from the DB >4/ Create a subscription, serial, etc. and delete the subscription >=> The serials should have been deleted from the DB > >Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> >--- > C4/Biblio.pm | 7 --- > C4/Serials.pm | 2 - > .../data/mysql/atomicupdate/bug_21901.perl | 62 ++++++++++++++++++ > installer/data/mysql/kohastructure.sql | 63 ++++++++++--------- > t/db_dependent/Biblio.t | 34 +++++++++- > 5 files changed, 128 insertions(+), 40 deletions(-) > create mode 100644 installer/data/mysql/atomicupdate/bug_21901.perl > >diff --git a/C4/Biblio.pm b/C4/Biblio.pm >index 93bed277e8..e8915b336c 100644 >--- a/C4/Biblio.pm >+++ b/C4/Biblio.pm >@@ -386,13 +386,6 @@ sub DelBiblio { > > return $error if $error; > >- # We delete attached subscriptions >- require C4::Serials; >- my $subscriptions = C4::Serials::GetFullSubscriptionsFromBiblionumber($biblionumber); >- foreach my $subscription (@$subscriptions) { >- C4::Serials::DelSubscription( $subscription->{subscriptionid} ); >- } >- > # We delete any existing holds > my $holds = $biblio->holds; > while ( my $hold = $holds->next ) { >diff --git a/C4/Serials.pm b/C4/Serials.pm >index 9d34b14bae..e5beaaf51f 100644 >--- a/C4/Serials.pm >+++ b/C4/Serials.pm >@@ -1731,8 +1731,6 @@ sub DelSubscription { > my ($subscriptionid) = @_; > my $dbh = C4::Context->dbh; > $dbh->do("DELETE FROM subscription WHERE subscriptionid=?", undef, $subscriptionid); >- $dbh->do("DELETE FROM subscriptionhistory WHERE subscriptionid=?", undef, $subscriptionid); >- $dbh->do("DELETE FROM serial WHERE subscriptionid=?", undef, $subscriptionid); > > Koha::AdditionalFieldValues->search({ > 'field.tablename' => 'subscription', >diff --git a/installer/data/mysql/atomicupdate/bug_21901.perl b/installer/data/mysql/atomicupdate/bug_21901.perl >new file mode 100644 >index 0000000000..58657f91b7 >--- /dev/null >+++ b/installer/data/mysql/atomicupdate/bug_21901.perl >@@ -0,0 +1,62 @@ >+$DBversion = 'XXX'; # will be replaced by the RM >+if( CheckVersion( $DBversion ) ) { >+ >+ $dbh->do(q| >+ ALTER TABLE serial >+ MODIFY COLUMN biblionumber INT(11) NOT NULL >+ |); >+ >+ unless ( foreign_key_exists( 'serial', 'serial_ibfk_1' ) ) { >+ $dbh->do(q| >+ ALTER TABLE serial >+ ADD CONSTRAINT serial_ibfk_1 FOREIGN KEY (biblionumber) REFERENCES biblio (biblionumber) ON DELETE CASCADE ON UPDATE CASCADE >+ |); >+ } >+ >+ $dbh->do(q| >+ ALTER TABLE serial >+ MODIFY COLUMN subscriptionid INT(11) NOT NULL >+ |); >+ >+ unless ( foreign_key_exists( 'serial', 'serial_ibfk_2' ) ) { >+ $dbh->do(q| >+ ALTER TABLE serial >+ ADD CONSTRAINT serial_ibfk_2 FOREIGN KEY (subscriptionid) REFERENCES subscription (subscriptionid) ON DELETE CASCADE ON UPDATE CASCADE >+ |); >+ } >+ >+ $dbh->do(q| >+ ALTER TABLE subscriptionhistory >+ MODIFY COLUMN biblionumber int(11), >+ MODIFY COLUMN subscriptionid int(11) >+ |); >+ >+ unless ( foreign_key_exists( 'subscriptionhistory', 'subscription_history_ibfk_1' ) ) { >+ $dbh->do(q| >+ ALTER TABLE subscriptionhistory >+ ADD CONSTRAINT subscription_history_ibfk_1 FOREIGN KEY (biblionumber) REFERENCES biblio (biblionumber) ON DELETE CASCADE ON UPDATE CASCADE >+ |); >+ } >+ >+ unless ( foreign_key_exists( 'subscriptionhistory', 'subscription_history_ibfk_2' ) ) { >+ $dbh->do(q| >+ ALTER TABLE subscriptionhistory >+ ADD CONSTRAINT subscription_history_ibfk_2 FOREIGN KEY (subscriptionid) REFERENCES subscription (subscriptionid) ON DELETE CASCADE ON UPDATE CASCADE >+ |); >+ } >+ >+ $dbh->do(q| >+ ALTER TABLE subscription >+ MODIFY COLUMN biblionumber int(11) >+ |); >+ >+ unless ( foreign_key_exists( 'subscription', 'subscription_ibfk_3' ) ) { >+ $dbh->do(q| >+ ALTER TABLE subscription >+ ADD CONSTRAINT subscription_ibfk_3 FOREIGN KEY (biblionumber) REFERENCES biblio (biblionumber) ON DELETE CASCADE ON UPDATE CASCADE >+ |); >+ } >+ >+ SetVersion( $DBversion ); >+ print "Upgrade to $DBversion done (Bug 21901 - Add foreign key constraints on serial)\n"; >+} >diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql >index ade4c50f64..2667074dda 100644 >--- a/installer/data/mysql/kohastructure.sql >+++ b/installer/data/mysql/kohastructure.sql >@@ -1457,30 +1457,6 @@ CREATE TABLE `search_marc_to_field` ( > FOREIGN KEY(search_field_id) REFERENCES search_field(id) ON DELETE CASCADE ON UPDATE CASCADE > ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; > >--- >--- Table structure for table `serial` >--- >- >-DROP TABLE IF EXISTS `serial`; >-CREATE TABLE `serial` ( -- issues related to subscriptions >- `serialid` int(11) NOT NULL auto_increment, -- unique key for the issue >- `biblionumber` varchar(100) NOT NULL default '', -- foreign key for the biblio.biblionumber that this issue is attached to >- `subscriptionid` varchar(100) NOT NULL default '', -- foreign key to the subscription.subscriptionid that this issue is part of >- `serialseq` varchar(100) NOT NULL default '', -- issue information (volume, number, etc) >- `serialseq_x` varchar( 100 ) NULL DEFAULT NULL, -- first part of issue information >- `serialseq_y` varchar( 100 ) NULL DEFAULT NULL, -- second part of issue information >- `serialseq_z` varchar( 100 ) NULL DEFAULT NULL, -- third part of issue information >- `status` tinyint(4) NOT NULL default 0, -- status code for this issue (see manual for full descriptions) >- `planneddate` date default NULL, -- date expected >- `notes` MEDIUMTEXT, -- notes >- `publisheddate` date default NULL, -- date published >- publisheddatetext varchar(100) default NULL, -- date published (descriptive) >- `claimdate` date default NULL, -- date claimed >- claims_count int(11) default 0, -- number of claims made related to this issue >- `routingnotes` MEDIUMTEXT, -- notes from the routing list >- PRIMARY KEY (`serialid`) >-) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; >- > -- > -- Table structure for table `sessions` > -- >@@ -1993,7 +1969,7 @@ CREATE TABLE subscription_numberpatterns ( > > DROP TABLE IF EXISTS `subscription`; > CREATE TABLE `subscription` ( -- information related to the subscription >- `biblionumber` int(11) NOT NULL default 0, -- foreign key for biblio.biblionumber that this subscription is attached to >+ `biblionumber` int(11) NOT NULL, -- foreign key for biblio.biblionumber that this subscription is attached to > `subscriptionid` int(11) NOT NULL auto_increment, -- unique key for this subscription > `librarian` varchar(100) default '', -- the librarian's username from borrowers.userid > `startdate` date default NULL, -- start date for this subscription >@@ -2037,9 +2013,35 @@ CREATE TABLE `subscription` ( -- information related to the subscription > `previousitemtype` VARCHAR( 10 ) NULL, > `mana_id` int(11) NULL DEFAULT NULL, > PRIMARY KEY (`subscriptionid`), >- KEY `by_biblionumber` (`biblionumber`), > CONSTRAINT subscription_ibfk_1 FOREIGN KEY (periodicity) REFERENCES subscription_frequencies (id) ON DELETE SET NULL ON UPDATE CASCADE, >- CONSTRAINT subscription_ibfk_2 FOREIGN KEY (numberpattern) REFERENCES subscription_numberpatterns (id) ON DELETE SET NULL ON UPDATE CASCADE >+ CONSTRAINT subscription_ibfk_2 FOREIGN KEY (numberpattern) REFERENCES subscription_numberpatterns (id) ON DELETE SET NULL ON UPDATE CASCADE, >+ CONSTRAINT subscription_ibfk_3 FOREIGN KEY (biblionumber) REFERENCES biblio (biblionumber) ON DELETE CASCADE ON UPDATE CASCADE >+) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; >+ >+-- >+-- Table structure for table `serial` >+-- >+ >+DROP TABLE IF EXISTS `serial`; >+CREATE TABLE `serial` ( -- issues related to subscriptions >+ `serialid` int(11) NOT NULL auto_increment, -- unique key for the issue >+ `biblionumber` int(11) NOT NULL, -- foreign key for the biblio.biblionumber that this issue is attached to >+ `subscriptionid` int(11) NOT NULL, -- foreign key to the subscription.subscriptionid that this issue is part of >+ `serialseq` varchar(100) NOT NULL default '', -- issue information (volume, number, etc) >+ `serialseq_x` varchar( 100 ) NULL DEFAULT NULL, -- first part of issue information >+ `serialseq_y` varchar( 100 ) NULL DEFAULT NULL, -- second part of issue information >+ `serialseq_z` varchar( 100 ) NULL DEFAULT NULL, -- third part of issue information >+ `status` tinyint(4) NOT NULL default 0, -- status code for this issue (see manual for full descriptions) >+ `planneddate` date default NULL, -- date expected >+ `notes` MEDIUMTEXT, -- notes >+ `publisheddate` date default NULL, -- date published >+ publisheddatetext varchar(100) default NULL, -- date published (descriptive) >+ `claimdate` date default NULL, -- date claimed >+ claims_count int(11) default 0, -- number of claims made related to this issue >+ `routingnotes` MEDIUMTEXT, -- notes from the routing list >+ PRIMARY KEY (`serialid`), >+ CONSTRAINT serial_ibfk_1 FOREIGN KEY (biblionumber) REFERENCES biblio (biblionumber) ON DELETE CASCADE ON UPDATE CASCADE, >+ CONSTRAINT serial_ibfk_2 FOREIGN KEY (subscriptionid) REFERENCES subscription (subscriptionid) ON DELETE CASCADE ON UPDATE CASCADE > ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; > > -- >@@ -2048,8 +2050,8 @@ CREATE TABLE `subscription` ( -- information related to the subscription > > DROP TABLE IF EXISTS `subscriptionhistory`; > CREATE TABLE `subscriptionhistory` ( >- `biblionumber` int(11) NOT NULL default 0, >- `subscriptionid` int(11) NOT NULL default 0, >+ `biblionumber` int(11) NOT NULL, >+ `subscriptionid` int(11) NOT NULL, > `histstartdate` date default NULL, > `histenddate` date default NULL, > `missinglist` LONGTEXT NOT NULL, >@@ -2057,7 +2059,8 @@ CREATE TABLE `subscriptionhistory` ( > `opacnote` LONGTEXT NULL, > `librariannote` LONGTEXT NULL, > PRIMARY KEY (`subscriptionid`), >- KEY `biblionumber` (`biblionumber`) >+ CONSTRAINT subscription_history_ibfk_1 FOREIGN KEY (biblionumber) REFERENCES biblio (biblionumber) ON DELETE CASCADE ON UPDATE CASCADE, >+ CONSTRAINT subscription_history_ibfk_2 FOREIGN KEY (subscriptionid) REFERENCES subscription (subscriptionid) ON DELETE CASCADE ON UPDATE CASCADE > ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; > > -- >diff --git a/t/db_dependent/Biblio.t b/t/db_dependent/Biblio.t >index cc38e07201..0aba28853f 100755 >--- a/t/db_dependent/Biblio.t >+++ b/t/db_dependent/Biblio.t >@@ -38,6 +38,8 @@ $schema->storage->txn_begin; > my $dbh = C4::Context->dbh; > Koha::Caches->get_instance->clear_from_cache( "MarcSubfieldStructure-" ); > >+my $builder = t::lib::TestBuilder->new; >+ > subtest 'GetMarcSubfieldStructureFromKohaField' => sub { > plan tests => 25; > >@@ -587,7 +589,7 @@ subtest 'deletedbiblio_metadata' => sub { > }; > > subtest 'DelBiblio' => sub { >- plan tests => 2; >+ plan tests => 5; > > my ($biblionumber, $biblioitemnumber) = C4::Biblio::AddBiblio(MARC::Record->new, ''); > my $deleted = C4::Biblio::DelBiblio( $biblionumber ); >@@ -595,6 +597,36 @@ subtest 'DelBiblio' => sub { > > $deleted = C4::Biblio::DelBiblio( $biblionumber ); > is( $deleted, undef, 'DelBiblo should return undef is the record did not exist'); >+ >+ my $biblio = $builder->build_sample_biblio; >+ my $subscription = $builder->build_object( >+ { >+ class => 'Koha::Subscriptions', >+ value => { biblionumber => $biblio->biblionumber } >+ } >+ ); >+ my $serial = $builder->build_object( >+ { >+ class => 'Koha::Serials', >+ value => { >+ biblionumber => $biblio->biblionumber, >+ subscriptionid => $subscription->subscriptionid >+ } >+ } >+ ); >+ my $subscription_history = $builder->build_object( >+ { >+ class => 'Koha::Subscription::Histories', >+ value => { >+ biblionumber => $biblio->biblionumber, >+ subscriptionid => $subscription->subscriptionid >+ } >+ } >+ ); >+ C4::Biblio::DelBiblio($biblio->biblionumber); # Or $biblio->delete >+ is( $subscription->get_from_storage, undef, 'subscription should be deleted on biblio deletion' ); >+ is( $serial->get_from_storage, undef, 'serial should be deleted on biblio deletion' ); >+ is( $subscription_history->get_from_storage, undef, 'subscription history should be deleted on biblio deletion' ); > }; > > subtest 'MarcFieldForCreatorAndModifier' => sub { >-- >2.20.1
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 21901
:
96181
|
98177
|
98229
|
98231
|
100384
|
100545
|
100546
|
100547
|
103122
|
103123
|
103124
|
103125
|
103151