Bug 6175 - Mismatch between auth_subfield_structure and authorised_values data type - varchar(32) vs. varchar(10)
Summary: Mismatch between auth_subfield_structure and authorised_values data type - v...
Status: NEW
Alias: None
Product: Koha
Classification: Unclassified
Component: Database (show other bugs)
Version: master
Hardware: All All
: P3 normal with 1 vote (vote)
Assignee: Galen Charlton
QA Contact: Bugs List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-04-12 15:35 UTC by Fernando L. Canizo
Modified: 2019-03-16 10:59 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Fix mismatch, add foreign key constraints, reshuffle sql scripts, fix UI to match DB (49.42 KB, patch)
2011-04-13 16:19 UTC, Fernando L. Canizo
Details | Diff | Splinter Review
Bug 6175 Mismatch on tables definition (50.65 KB, patch)
2012-06-10 11:12 UTC, MJ Ray (software.coop)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Fernando L. Canizo 2011-04-12 15:35:19 UTC
kohastructure.sql defines:

marc_subfield_structure.authorised_value varchar(20) default NULL

this column points to:

authorised_values.category varchar(10) NOT NULL default ''

Quick fix is straightforward: modify marc_subfield_structure to match authorised_values definition.

However there's been a request to expand size for category column (http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5458).

Also there is discussion to make all references to authorised_values.category foreign key references on koha-dev list.
Comment 1 Fernando L. Canizo 2011-04-13 16:19:55 UTC Comment hidden (obsolete)
Comment 2 Fernando L. Canizo 2011-04-14 11:34:53 UTC
(In reply to comment #1)
> Created attachment 3911 [details] [review]
> Fix mismatch, add foreign key constraints, reshuffle sql scripts, fix UI to
> match DB

This is broken, working on new one.
Comment 3 MJ Ray (software.coop) 2012-06-10 11:12:38 UTC
Created attachment 10086 [details] [review]
Bug 6175 Mismatch on tables definition

kohastructure.sql defines:
marc_subfield_structure.authorised_value varchar(20)
this column points to:
authorised_values.category varchar(10)

At first tought you would think about shortening in
marc_subfield_structure table, but install scripts had values longer
than 10 characters. Also there's been talk in the list about expanding
the column.

So:

- Expand on authorised_values.category.
- Add foreign keys constraints.
- Reshuffle sql scripts to allow insertions given the new constraints.
- Expand template input text field to match database column.

Signed-off-by: MJ Ray <mjr@phonecoop.coop>
Comment 4 MJ Ray (software.coop) 2012-06-10 11:14:29 UTC
At a basic level, this looks like it works (it now applies and tests pass), so I have signed it off.

If the brokenness was more subtle/structural, please make it Failed QA.
Comment 5 Paul Poulain 2012-06-11 14:52:16 UTC
QA comment: This patch has some problems:
  * it introduces a constraint that is wrong:
+  CONSTRAINT `auth_subfield_structure_ibfk_1` FOREIGN KEY (`authorised_value`) REFERENCES `authorised_values` (`category`) ON DELETE CASCADE ON UPDATE CASCADE
 => the constraint can be on authorised_values, but also on branches or itemtype, so a SQL constraint is unapplicable here
  * it contains a lot of changes in french file that are just moving things from the bottom to the top of file without reason (at least without relation to this patch. As the patch has been made by someone from argentina, I think it was not intended to update french files !)
  * there is no updatedatabase part on this patch making existing Koha being de-synchronised from freshly installed Koha

for those reasons, failed QA, sorry
Comment 6 MJ Ray (software.coop) 2012-06-12 09:02:03 UTC
(In reply to comment #5)
> QA comment: This patch has some problems:
>   * it introduces a constraint that is wrong:
> +  CONSTRAINT `auth_subfield_structure_ibfk_1` FOREIGN KEY
> (`authorised_value`) REFERENCES `authorised_values` (`category`) ON DELETE
> CASCADE ON UPDATE CASCADE
>  => the constraint can be on authorised_values, but also on branches or
> itemtype, so a SQL constraint is unapplicable here

So is this potential mismatch unavoidable with the current database design?

>  * it contains a lot of changes in french file that are just moving things from 
> the bottom to the top of file without reason (at least without relation to this 
> patch. As the patch has been made by someone from argentina, I think it was not > intended to update french files !)
>   * there is no updatedatabase part on this patch making existing Koha being
> de-synchronised from freshly installed Koha

These are not currently among the http://wiki.koha-community.org/wiki/Coding_Guidelines#Database but perhaps they should be?

This would not be the first patch to change kohastructure.sql without a matching updatedatabase, after all... but I remember them because they caused bugs, so maybe it should be a guideline.
Comment 7 Owen Leonard 2016-06-23 12:25:48 UTC
This bug is still valid in master, but the patch appears to be abandoned. I'm going to reset this to NEW.
Comment 8 Katrin Fischer 2019-03-16 10:59:10 UTC
marc_subfield_structure
 `authorised_value` varchar(32) COLLATE utf8mb4_unicode_ci DEFAULT NULL,

auth_subfield_structure
 `authorised_value` varchar(10) COLLATE utf8mb4_unicode_ci DEFAULT NULL,

authorised_value_categories
 `category_name` varchar(32) COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT '',

authorised_values
  `category` varchar(32) COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT '',

I think there is still a issue for the auth_subfield_structure.