After applying the Bug 36372, it is possible to manually set record source for a given bibliographic record. At the same time, from /cgi-bin/koha/admin/record_sources it is possible to delete a record source currently being used, without any control. This in turn causes the deletion of all the rows from the biblio_metadata table linked with the record source being deleted, which is fatal. BTW the biblio and biblioitems rows remain in the database. This effect is caused by "CONSTRAINT `record_metadata_fk_2` FOREIGN KEY (`record_source_id`) REFERENCES `record_sources` (`record_source_id`) ON DELETE CASCADE" in biblio_metadata setup. Of course the same effect occurs when deleting the record_sources row by hand, directly in the database. I am not sure if the decision to link biblio_metadata with record_sources with ON DELETE CASCADE is just, even if we controlled the use of the specific record source before deletion (as we do with itemtypes for instance).
I was able to reproduce the issue
Sounds like this should be "ON DELETE RESTRICT"
(In reply to David Cook from comment #2) > Sounds like this should be "ON DELETE RESTRICT" Makes sense. Adding RM_priority as this could lead to severe data loss, especially since rows are deleted directly instead of being moved...
Created attachment 169823 [details] [review] Bug 37419: Update FK constraint on record sources to avoid data loss Without this patch, deleting a record source will delete the associated biblio_metadata rows, which is a severe data loss. This patch makes the constraint restrict this action. To test: 1. Add a record source 2. Set the record source to some records $ koha-mysql kohadev > UPDATE biblio_metadata SET record_source_id='your source id' WHERE biblionumber=1; 3. Delete the record source => FAIL: Record metadata deleted 4. Apply this patch 5, Run: $ ktd --shell k$ updatedatabase => SUCCESS: DB update goes well 6. Repeat 1~3 with another record => SUCCESS: Source cannot be deleted if there are linked records Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 169824 [details] [review] [DO NOT PUSH] Bug 37419: DBIC schema Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 169827 [details] [review] Bug 37419: Update FK constraint on record sources to avoid data loss Without this patch, deleting a record source will delete the associated biblio_metadata rows, which is a severe data loss. This patch makes the constraint restrict this action. To test: 1. Add a record source 2. Set the record source to some records $ koha-mysql kohadev > UPDATE biblio_metadata SET record_source_id='your source id' WHERE biblionumber=1; 3. Delete the record source => FAIL: Record metadata deleted 4. Apply this patch 5, Run: $ ktd --shell k$ updatedatabase => SUCCESS: DB update goes well 6. Repeat 1~3 with another record => SUCCESS: Source cannot be deleted if there are linked records Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Janusz Kaczmarek <januszop@gmail.com>
Created attachment 169828 [details] [review] Bug 37419: DBIC schema Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Janusz Kaczmarek <januszop@gmail.com>
Just one remark: the message generated after an unsuccessful deletion sounds: "Something went wrong: Error: Something went wrong, check Koha logs for details." I wonder if it shouldn't/couldn't be more detailed...
(In reply to Janusz Kaczmarek from comment #8) > Just one remark: the message generated after an unsuccessful deletion > sounds: "Something went wrong: Error: Something went wrong, check Koha logs > for details." I wonder if it shouldn't/couldn't be more detailed... Indeed. I'm working on bug 37510 to properly handle the DBIC exception from ->delete() in the codebase, and then will handle it on the Vue side. Thanks for the feedback!
Created attachment 169932 [details] [review] Bug 37419: Update FK constraint on record sources to avoid data loss Without this patch, deleting a record source will delete the associated biblio_metadata rows, which is a severe data loss. This patch makes the constraint restrict this action. To test: 1. Add a record source 2. Set the record source to some records $ koha-mysql kohadev > UPDATE biblio_metadata SET record_source_id='your source id' WHERE biblionumber=1; 3. Delete the record source => FAIL: Record metadata deleted 4. Apply this patch 5, Run: $ ktd --shell k$ updatedatabase => SUCCESS: DB update goes well 6. Repeat 1~3 with another record => SUCCESS: Source cannot be deleted if there are linked records Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Janusz Kaczmarek <januszop@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 169933 [details] [review] Bug 37419: DBIC schema Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Janusz Kaczmarek <januszop@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Pushed for 24.11! Well done everyone, thank you!
Backported to 24.05.x for upcoming 24.05.04
Depends on Bug 35919 not in 23.11.x
I am seeing something strange here when running the database scripts for comparing new/udpate before release: In the generated kohastructure.sql: @@ -1151,7 +1150,7 @@ CREATE TABLE `biblio_metadata` ( KEY `timestamp` (`timestamp`), KEY `record_metadata_fk_2` (`record_source_id`), CONSTRAINT `record_metadata_fk_1` FOREIGN KEY (`biblionumber`) REFERENCES `biblio` (`biblionumber`) ON DELETE CASCADE ON UPDATE CASCADE, - CONSTRAINT `record_metadata_fk_2` FOREIGN KEY (`record_source_id`) REFERENCES `record_sources` (`record_source_id`) ON DELETE RESTRICT ON UPDATE CASCADE + CONSTRAINT `record_metadata_fk_2` FOREIGN KEY (`record_source_id`) REFERENCES `record_sources` (`record_source_id`) ON UPDATE CASCADE ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; /*!40101 SET character_set_client = @saved_cs_client */; Where has the "ON DELETE RESTRICT" gone?
How is that possible? In master we see at line 1147 (from this bug): - CONSTRAINT `record_metadata_fk_2` FOREIGN KEY (`record_source_id`) REFERENCES `record_sources` (`record_source_id`) ON DELETE CASCADE ON UPDATE CASCADE + CONSTRAINT `record_metadata_fk_2` FOREIGN KEY (`record_source_id`) REFERENCES `record_sources` (`record_source_id`) ON DELETE RESTRICT ON UPDATE CASCADE and then noting in this region. Strange...
(In reply to Janusz Kaczmarek from comment #16) > How is that possible? In master we see at line 1147 (from this bug): > > - CONSTRAINT `record_metadata_fk_2` FOREIGN KEY (`record_source_id`) > REFERENCES `record_sources` (`record_source_id`) ON DELETE CASCADE ON UPDATE > CASCADE > + CONSTRAINT `record_metadata_fk_2` FOREIGN KEY (`record_source_id`) > REFERENCES `record_sources` (`record_source_id`) ON DELETE RESTRICT ON > UPDATE CASCADE > > and then noting in this region. Strange... I have no explanation either. I'll probably repeat the steps on Monday just to make sure, but we need to resolve.
@@ -1151,7 +1150,7 @@ CREATE TABLE `biblio_metadata` ( KEY `timestamp` (`timestamp`), KEY `record_metadata_fk_2` (`record_source_id`), CONSTRAINT `record_metadata_fk_1` FOREIGN KEY (`biblionumber`) REFERENCES `biblio` (`biblionumber`) ON DELETE CASCADE ON UPDATE CASCADE, - CONSTRAINT `record_metadata_fk_2` FOREIGN KEY (`record_source_id`) REFERENCES `record_sources` (`record_source_id`) ON DELETE RESTRICT ON UPDATE CASCADE + CONSTRAINT `record_metadata_fk_2` FOREIGN KEY (`record_source_id`) REFERENCES `record_sources` (`record_source_id`) ON UPDATE CASCADE ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; /*!40101 SET character_set_client = @saved_cs_client */; We have come to the conclusion that ON UPDATE RESTRICT is missing form the dump, as this is the default behavior. So this is a non-issue actually and things are in order.
(In reply to Katrin Fischer from comment #18) > > We have come to the conclusion that ON UPDATE RESTRICT is missing form the > dump, as this is the default behavior. So this is a non-issue actually and > things are in order. Small behavior differences in MariaDB would cause this. It really depends on which MariaDB version you're running this at. When you regenerated for 24.05, this is what we used: ``` -- Server version 11.3.2-MariaDB-1:11.3.2+maria~ubu2204 ``` for 23.11 we used this: ``` -- Server version 11.2.2-MariaDB-1:11.2.2+maria~ubu2204 ```
(In reply to Tomás Cohen Arazi (tcohen) from comment #19) > (In reply to Katrin Fischer from comment #18) > > > > We have come to the conclusion that ON UPDATE RESTRICT is missing form the > > dump, as this is the default behavior. So this is a non-issue actually and > > things are in order. > > Small behavior differences in MariaDB would cause this. It really depends on > which MariaDB version you're running this at. > > When you regenerated for 24.05, this is what we used: > > ``` > -- Server version 11.3.2-MariaDB-1:11.3.2+maria~ubu2204 > ``` > > for 23.11 we used this: > > ``` > -- Server version 11.2.2-MariaDB-1:11.2.2+maria~ubu2204 > ``` The instructions say to use this: DB_IMAGE=mariadb:latest ktd up -d So it makes sense that a newer version was used this time.
(In reply to Katrin Fischer from comment #20) > The instructions say to use this: > DB_IMAGE=mariadb:latest ktd up -d > > So it makes sense that a newer version was used this time. Absolutely. Just noted that minor changes are common because of the version change.