Summary: | Deleting the record source deletes the associated biblio_metadata rows | ||
---|---|---|---|
Product: | Koha | Reporter: | Janusz Kaczmarek <januszop> |
Component: | System Administration | Assignee: | Tomás Cohen Arazi (tcohen) <tomascohen> |
Status: | RESOLVED FIXED | QA Contact: | Martin Renvoize (ashimema) <martin.renvoize> |
Severity: | major | ||
Priority: | P5 - low | CC: | arthur.suzuki, caroline.cyr-la-rose, dcook, fridolin.somers, gmcharlt, lucas, martin.renvoize, pedro.amorim, tomascohen |
Version: | unspecified | ||
Hardware: | All | ||
OS: | All | ||
GIT URL: | Change sponsored?: | --- | |
Patch complexity: | Trivial patch | Documentation contact: | |
Documentation submission: | Text to go in the release notes: | ||
Version(s) released in: |
24.11.00,24.05.04
|
Circulation function: | |
Bug Depends on: | 35919 | ||
Bug Blocks: | 36372, 37513, 37510 | ||
Attachments: |
Bug 37419: Update FK constraint on record sources to avoid data loss
[DO NOT PUSH] Bug 37419: DBIC schema Bug 37419: Update FK constraint on record sources to avoid data loss Bug 37419: DBIC schema Bug 37419: Update FK constraint on record sources to avoid data loss Bug 37419: DBIC schema |
Description
Janusz Kaczmarek
2024-07-22 08:57:40 UTC
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. Internal architecture, nothing to add/edit in the manual. |