Bug 30449 - Missing FK constraint on borrower_attribute_types
Summary: Missing FK constraint on borrower_attribute_types
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Database (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Marcel de Rooy
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 30486 30899 31037
  Show dependency treegraph
 
Reported: 2022-04-04 13:01 UTC by Marcel de Rooy
Modified: 2022-06-24 09:20 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact: Martin Renvoize
Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.05.00,21.11.06


Attachments
Bug 30449: Add missing FK constraint on borrower_attribute_types (3.46 KB, patch)
2022-04-05 12:38 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 30449: Add missing FK constraint on borrower_attribute_types (3.51 KB, patch)
2022-04-05 12:42 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 30449: Add missing FK constraint on borrower_attribute_types (3.53 KB, patch)
2022-04-25 13:12 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 30449: DBIC updates (3.27 KB, patch)
2022-04-25 13:12 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 30446: (QA follow-up) Report back on problematic situations (2.69 KB, patch)
2022-04-25 13:13 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 30449: Add missing FK constraint on borrower_attribute_types (3.59 KB, patch)
2022-04-26 13:03 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30449: DBIC updates (3.33 KB, patch)
2022-04-26 13:04 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30449: (QA follow-up) Report back on problematic situations (2.70 KB, patch)
2022-04-26 13:04 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel de Rooy 2022-04-04 13:01:04 UTC
Not in kohastructure, but present for a long time already in updatedatabase:

$DBversion = "3.07.00.034";
$dbh->do("ALTER TABLE borrower_attribute_types ADD CONSTRAINT category_code_fk FOREIGN KEY (category_code) REFERENCES categories(categorycode)");
Comment 1 Marcel de Rooy 2022-04-05 12:38:46 UTC
Created attachment 132965 [details] [review]
Bug 30449: Add missing FK constraint on borrower_attribute_types

Old Koha databases probably have it, newer ones might not.
See also BZ description.

Test plan:
[1} Check SHOW CREATE TABLE borrower_attribute_types to see if you
    have any index and FK constraint on category_code.
[2] Run updatedatabase.
[3] If you had category_code_fk, it should be replaced.
[4] Remove index and constraint again using things like:
    alter table borrower_attribute_types drop constraint `borrower_attribute_types_ibfk_1`;
    alter table borrower_attribute_types drop index category_code;
[5] Run updatedatabase.
[6] You should have KEY category_code and FK borrower_attribute_types_ibfk_1.
[7] Run updatedatabase. Idempotent, no changes.
Comment 2 Marcel de Rooy 2022-04-05 12:42:32 UTC
Created attachment 132966 [details] [review]
Bug 30449: Add missing FK constraint on borrower_attribute_types

Old Koha databases probably have it, newer ones might not.
See also BZ description.

Test plan:
[1} Check SHOW CREATE TABLE borrower_attribute_types to see if you
    have any index and FK constraint on category_code.
[2] Run updatedatabase.
[3] If you had category_code_fk, it should be replaced.
[4] Remove index and constraint again using things like:
    alter table borrower_attribute_types drop constraint `borrower_attribute_types_ibfk_1`;
    alter table borrower_attribute_types drop index category_code;
[5] Run updatedatabase.
[6] You should have KEY category_code and FK borrower_attribute_types_ibfk_1.
[7] Run updatedatabase. Idempotent, no changes.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 3 Tomás Cohen Arazi 2022-04-25 12:49:34 UTC
I've checked on 3.0.X original Koha deployments I have, (already in 21.05) and the constraint is there!

Good catch, Marcel!
Comment 4 Marcel de Rooy 2022-04-25 12:54:47 UTC
I am wondering how DBIx::RunSQL handles our /*4xxx SET FOREIGN KEY CHECKS OFF*/ line.
Comment 5 Tomás Cohen Arazi 2022-04-25 13:12:32 UTC
Created attachment 133777 [details] [review]
Bug 30449: Add missing FK constraint on borrower_attribute_types

Old Koha databases probably have it, newer ones might not.
See also BZ description.

Test plan:
[1} Check SHOW CREATE TABLE borrower_attribute_types to see if you
    have any index and FK constraint on category_code.
[2] Run updatedatabase.
[3] If you had category_code_fk, it should be replaced.
[4] Remove index and constraint again using things like:
    alter table borrower_attribute_types drop constraint `borrower_attribute_types_ibfk_1`;
    alter table borrower_attribute_types drop index category_code;
[5] Run updatedatabase.
[6] You should have KEY category_code and FK borrower_attribute_types_ibfk_1.
[7] Run updatedatabase. Idempotent, no changes.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 6 Tomás Cohen Arazi 2022-04-25 13:12:47 UTC
Created attachment 133778 [details] [review]
Bug 30449: DBIC updates

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 7 Tomás Cohen Arazi 2022-04-25 13:13:04 UTC
Created attachment 133779 [details] [review]
Bug 30446: (QA follow-up) Report back on problematic situations

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 8 Marcel de Rooy 2022-04-26 12:24:02 UTC
(In reply to Marcel de Rooy from comment #4)
> I am wondering how DBIx::RunSQL handles our /*4xxx SET FOREIGN KEY CHECKS
> OFF*/ line.

Well, quite simple. It does ignore such lines. The following commit added these checks in Installer:
    Bug 26947: (follow-up) Set key checks to disabled for kohastructure

But note that DBIx::RunSQL was already added on bug 13669 ;)
Comment 9 Marcel de Rooy 2022-04-26 12:25:49 UTC
Anyway, we do not need to move the (re)definition of borrower_attribute_types below categories now.
You should however when you backport this patch BELOW 21.05 !
Or add these lines too in Installer:

+    # Disable checks before load
+    $self->{'dbh'}->do(q{SET NAMES utf8mb4});
+    $self->{'dbh'}->do(q{SET @OLD_UNIQUE_CHECKS=@@UNIQUE_CHECKS, UNIQUE_CHECKS=0});
+    $self->{'dbh'}->do(q{SET @OLD_FOREIGN_KEY_CHECKS=@@FOREIGN_KEY_CHECKS, FOREIGN_KEY_CHECKS=0});
+    $self->{'dbh'}->do(q{SET @OLD_SQL_MODE=@@SQL_MODE, SQL_MODE='NO_AUTO_VALUE_ON_ZERO'});
+    $self->{'dbh'}->do(q{SET @OLD_SQL_NOTES=@@SQL_NOTES, SQL_NOTES=0});
+
+    # Load kohastructure
     my $error = $self->load_sql("$datadir/kohastructure.sql");
+
+    # Re-enable checks after load
+    $self->{'dbh'}->do(q{SET SQL_MODE=@OLD_SQL_MODE});
+    $self->{'dbh'}->do(q{SET FOREIGN_KEY_CHECKS=@OLD_FOREIGN_KEY_CHECKS});
+    $self->{'dbh'}->do(q{SET UNIQUE_CHECKS=@OLD_UNIQUE_CHECKS});
+    $self->{'dbh'}->do(q{SET SQL_NOTES=@OLD_SQL_NOTES});
Comment 10 Martin Renvoize 2022-04-26 13:03:57 UTC
Created attachment 133883 [details] [review]
Bug 30449: Add missing FK constraint on borrower_attribute_types

Old Koha databases probably have it, newer ones might not.
See also BZ description.

Test plan:
[1} Check SHOW CREATE TABLE borrower_attribute_types to see if you
    have any index and FK constraint on category_code.
[2] Run updatedatabase.
[3] If you had category_code_fk, it should be replaced.
[4] Remove index and constraint again using things like:
    alter table borrower_attribute_types drop constraint `borrower_attribute_types_ibfk_1`;
    alter table borrower_attribute_types drop index category_code;
[5] Run updatedatabase.
[6] You should have KEY category_code and FK borrower_attribute_types_ibfk_1.
[7] Run updatedatabase. Idempotent, no changes.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 11 Martin Renvoize 2022-04-26 13:04:02 UTC
Created attachment 133884 [details] [review]
Bug 30449: DBIC updates

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 12 Martin Renvoize 2022-04-26 13:04:07 UTC
Created attachment 133885 [details] [review]
Bug 30449: (QA follow-up) Report back on problematic situations

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 13 Martin Renvoize 2022-04-26 13:04:42 UTC
Working well, a good job resyncing.  Thanks

PQA
Comment 14 Fridolin Somers 2022-05-02 21:24:48 UTC
Pushed to master for 22.05, thanks to everybody involved 🦄
Comment 15 Kyle M Hall 2022-05-06 17:57:18 UTC
Pushed to 21.11.x for 21.11.06
Comment 16 Marcel de Rooy 2022-06-07 14:21:51 UTC
See bug 30899. Patch forthcoming on that report.
No need to push this further down.
Closing the report to prevent that.
Comment 17 Martin Renvoize 2022-06-23 13:46:36 UTC
Architectural change, no documentation update required.