Bug 5377 - Database fields too small for multiple ISBN and ISSN
Summary: Database fields too small for multiple ISBN and ISSN
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Database (show other bugs)
Version: 3.16
Hardware: All All
: P5 - low normal (vote)
Assignee: Kyle M Hall
QA Contact: Bugs List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-11-09 10:18 UTC by Fridolin SOMERS
Modified: 2015-12-03 22:02 UTC (History)
13 users (show)

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


Attachments
Bug 5377 - Database fields too small for multiple ISBN and ISSN (3.06 KB, patch)
2014-02-19 21:57 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 5377 - Database fields too small for multiple ISBN and ISSN (3.31 KB, patch)
2014-02-20 03:02 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 5377 [QA Followup] - Alter deletedbiblioitems to match biblioitems (2.57 KB, patch)
2014-02-20 11:27 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 5377 [QA Followup] - Alter deletedbiblioitems to match biblioitems (2.49 KB, patch)
2014-02-20 11:30 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 5377 - Database fields too small for multiple ISBN and ISSN (3.38 KB, patch)
2014-02-20 11:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 5377 [QA Followup] - Alter deletedbiblioitems to match biblioitems (2.55 KB, patch)
2014-02-20 11:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 5377: No index on deletedbiblioitems.issn (1.27 KB, patch)
2014-02-20 11:45 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Fridolin SOMERS 2010-11-09 10:18:32 UTC
Hie,

I noticed a wrong behavior in "koha to MARC mapping".

Using a mapping for ISBN and ISSN fields, I noticed that fields "isbn" and "issn" where truncated in "biblioitems" table when using multiple values.
With multiple values, ISBN and ISSN are separated by a " | ".

In fact, "isbn" field is a VARCHAR(30)
     and "issn" field is a VARCHAR(9).

This is far to small. I propose a VARCHAR(255).

Regards,
Comment 1 Galen Charlton 2010-11-24 19:05:30 UTC
A suggestion - rather than widening the field, if bibliographic fields need to be stored outside of the MARC record for display, performance, or caching reason, multiple values would be better handled by keeping them in a separate table, something like:

biblio_attributes:
  biblionumber (foreign key)
  code (identifies the attribute, i.e., 'isbn', 'issn', etc.)
  seqno (sequence number, if needed)
  value
Comment 2 Fridolin SOMERS 2010-11-25 08:39:42 UTC
(In reply to comment #1)
> A suggestion - rather than
Comment 3 David Cook 2012-09-26 22:44:58 UTC
I would like to breathe some life back into this bug. 

I've noticed that the detail pages of the catalogue in the OPAC and the staff client go straight to the MARC to get the ISBNs, but the database field is still used in areas like the "Receipt Summary" in acquisitions after receiving new items. 

While it may be trivial, multiple isbns are getting cut off, so that they do not display properly.

The easiest solution would be to lengthen like Fridolyn has suggested.

However, I think I understand Galen's suggestion to make a different table instead to work with multiple values. After all, it's not good practice to be putting multiple values in a single field, right? 

That said, a new table might be overkill, because ISBN might be the only field in biblioitems that contains multiple values. I'm not 100% sure on this one, as I just took a quick look glance through some sample data.

Thoughts?
Comment 4 mathieu saby 2012-11-21 09:47:12 UTC
If someone is working on that bug, please consider applying the same treatment to EAN field in biblioitems table.

You say it is not good to have multiple values in a field, but this occurs elsewhere. For example in biblio table, for author, unititle and title (at least in UNIMARC, because they are mapped to repeatable subfields...). Should we do something for that ?


M. Saby
Rennes 2 University
Comment 5 Fridolin SOMERS 2012-11-21 10:12:54 UTC
(In reply to comment #4)
> If someone is working on that bug, please consider applying the same
> treatment to EAN field in biblioitems table.
Correct, ean is VARCHAR(13).

> You say it is not good to have multiple values in a field, but this occurs
> elsewhere. For example in biblio table, for author, unititle and title (at
> least in UNIMARC, because they are mapped to repeatable subfields...).
> Should we do something for that ?
author, title and unititle are mediumtext, they can accept 16 millions of characters.
Comment 6 mathieu saby 2012-11-21 10:18:28 UTC
author, title and unititle are mediumtext, they can accept 16 millions of characters.


>> 
I know, I was just reacting to David Cook's remark "After all, it's not good practice to be putting multiple values in a single field, right? "

But if it is only a "good practice", I suppose it is not harmfull to have several 200$a (several titles by the same author, in collective works) mapped in biblio.title ?

Mathieu
Comment 7 Galen Charlton 2013-07-03 23:31:18 UTC
(In reply to David Cook from comment #3)
> That said, a new table might be overkill, because ISBN might be the only
> field in biblioitems that contains multiple values. I'm not 100% sure on
> this one, as I just took a quick look glance through some sample data.

Well, there are a *lot* of fields that are repeatable and where you might well want to display and search on all repeats for a record.  Off the top of my head:

- EAN/UPCs, as mentioned by Mathieu
- arbitrary record identifiers (e.g., from the MARC21 035 field)
- chapter and part titles
- languages of the work
Comment 8 David Cook 2013-07-04 00:25:48 UTC
(In reply to Galen Charlton from comment #7)
> (In reply to David Cook from comment #3)
> > That said, a new table might be overkill, because ISBN might be the only
> > field in biblioitems that contains multiple values. I'm not 100% sure on
> > this one, as I just took a quick look glance through some sample data.
> 
> Well, there are a *lot* of fields that are repeatable and where you might
> well want to display and search on all repeats for a record.  Off the top of
> my head:
> 
> - EAN/UPCs, as mentioned by Mathieu
> - arbitrary record identifiers (e.g., from the MARC21 035 field)
> - chapter and part titles
> - languages of the work

Good point, Galen. When I first wrote my comment, I wasn't aware that there were other cases of repeatable MARC fields being put in a single database field with a pipe separator. However, after reading the other comments and taking a more thorough look, I see that more and more. 

Perhaps a new table, following the model you suggested in 2010, would be the best idea.

However, if we were to have a "biblio_attributes" table, might it make sense to drop the biblioitems table and reduce the scope of the biblio table?

That is to say, all bibliographic attributes (not only 'isbn' and 'issn' but also 'title', 'author', 'notes', and other attributes from both the biblio and biblioitems tables) would be stored in "biblio_attributes", while the "biblio" table would still exist with system specific attributes/data such as the 'biblionumber', 'frameworkcode', 'timestamp', and 'datecreated' fields. 

Conceptually, I like that separation of data. In terms of killing MARC, it might be an idea to make yet another table called "biblio_records" where the record is stored in a field (i.e. blob) and a metadata descriptor is recorded in another field.

However, it would take a massive re-write to the Koha code to make that a reality, no? 

Also, by putting all that data in to one table, might we suffer some large performance hits for databases with huge amounts of bib records? 

--

These are just musings on my part. I haven't designed enough databases to know what the best practice is in terms of performance. 

I wonder sometimes about how much we gain from using the relational database to store bibliographic data. Currently, are we not storing bibliographic data in 4+ places? The biblio table, the biblioitems table, the MARC blob in the biblioitems table, the MARCXML blob in the biblioitems table, and finally the Zebra database? 

Both in terms of killing MARC and improving data integrity (and perhaps performance), might it not make sense to reduce the number of places we're storing data? 

Is the idea of storing data in the relational database to improve the speed of retrieval? Do we actually gain speed? No pattern has emerged to me in terms of deciding when to use the MARC record and when to use the database as the source of data in a given script. I know we have the "mod" scripts which ensure data integrity, but I can't help but think that there must be a better way of storing and retrieving bibliographic data...

Sorry for the long comment! I would almost tl;dr it, but I think they're valid questions. 

"biblio", "biblio_attributes", and "biblio_records"?
"biblio", "biblioitems", "biblio_attributes"?
"biblio","biblio_records"?
Comment 9 Kyle M Hall 2014-02-19 21:57:36 UTC Comment hidden (obsolete)
Comment 10 David Cook 2014-02-19 22:34:27 UTC
(In reply to Kyle M Hall from comment #9)
> Created attachment 25469 [details] [review] [review]
> Bug 5377 - Database fields too small for multiple ISBN and ISSN
> 
> Test Plan:
> 1) Apply this patch
> 2) Create a record with 5 ISBNs and 5 ISSNs
> 3) Create a new report from the following SQL, or execute it from the
>    mysql console:
>    SELECT isbn, issn FROM biblioitems ORDER BY biblionumber DESC LIMIT 1
> 4) Note that all your ISBNs and ISSNs are listed, separated by the pipe
>    character ( | )

I'll have to try this out when I have some time. I wonder if the ISSN will display properly in Koha itself though if it's used to only having a single ISSN in that field.

Still, happy to see movement on this one.
Comment 11 Chris Cormack 2014-02-20 03:02:48 UTC Comment hidden (obsolete)
Comment 12 Chris Cormack 2014-02-20 03:03:55 UTC
We never actually use biblioitems.issn for much of anything, certainly not if using XSLT
Comment 13 Jonathan Druart 2014-02-20 11:22:20 UTC
The DB structure changes should be done on the deletedbiblioitems table too.

Marked as Failed QA.

Note: I got the following error
   "BLOB/TEXT column 'issn' used in key specification without a key length"
My issn index was called issn_idx, I have absolutely no idea why...
Comment 14 Kyle M Hall 2014-02-20 11:27:16 UTC Comment hidden (obsolete)
Comment 15 Kyle M Hall 2014-02-20 11:30:36 UTC Comment hidden (obsolete)
Comment 16 Kyle M Hall 2014-02-20 11:34:04 UTC
The latest followup only alters the isbn index for deletedbiblioitems. The previous iteration from the patch added an issn index that didn't previously exist. I added it to make it make biblioitems, but now I'm removing it because I see no benefit from having it, and at the least having it will cause cost a bit more cpu time and disk space.

QA'ers, if you feel for some reason the deletedbiblioitems table *should* have an issn index, just deprecate the followup, and restore the previous iteration of it.
Comment 17 Jonathan Druart 2014-02-20 11:43:06 UTC
Created attachment 25493 [details] [review]
Bug 5377 - Database fields too small for multiple ISBN and ISSN

Test Plan:
1) Apply this patch
2) Create a record with 5 ISBNs and 5 ISSNs
3) Create a new report from the following SQL, or execute it from the
   mysql console:
   SELECT isbn, issn FROM biblioitems ORDER BY biblionumber DESC LIMIT 1
4) Note that all your ISBNs and ISSNs are listed, separated by the pipe
   character ( | )

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>

This might be slow to run on big databases, because of the 2 index
rebuilds, however it changes no functionality just increases the field
size which is safe enough (we store multiple now already)

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 18 Jonathan Druart 2014-02-20 11:43:36 UTC
Created attachment 25494 [details] [review]
Bug 5377 [QA Followup] - Alter deletedbiblioitems to match biblioitems

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 19 Jonathan Druart 2014-02-20 11:45:19 UTC
Created attachment 25495 [details] [review]
Bug 5377: No index on deletedbiblioitems.issn

This patch removes a trailing space and remove the issn index deletion
(it does not exist on deletedbiblioitems).

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 20 Jonathan Druart 2014-02-20 11:53:16 UTC
(In reply to Kyle M Hall from comment #16)
> QA'ers, if you feel for some reason the deletedbiblioitems table *should*
> have an issn index, just deprecate the followup, and restore the previous
> iteration of it.

I didn't see your comment when I submitted my QA patches.
In my opinion, if the index is not in use, it is useless to add it.
Comment 21 Galen Charlton 2014-05-05 18:00:23 UTC
Pushed to master.  Thanks, Kyle!
Comment 22 Doug Dearden 2014-06-06 18:34:59 UTC
updatedatabase.pl at $DBversion = "3.15.00.049" seems to be implementing the changes related to this bug, but references Bug 11268.

I got the following error messages during an update to 3.16.0 from upgradedatabase.pl regarding this section:

[Fri May 30 16:09:09 2014] [error] [client 192.168.30.196] [Fri May 30 16:09:09 2014] install.pl: [Fri May 30 16:09:09 2014] updatedatabase.pl: DBD::mysql::db do failed: BLOB/TEXT column 'issn' used in key specification without a key length at /usr/share/koha/intranet/cgi-bin/installer/data/mysql/updatedatabase.pl line 8420., referer: http://192.168.30.13:8080/cgi-bin/koha/installer/install.pl
[Fri May 30 16:09:09 2014] [error] [client 192.168.30.196] [Fri May 30 16:09:09 2014] install.pl: , referer: http://192.168.30.13:8080/cgi-bin/koha/installer/install.pl
[Fri May 30 16:09:09 2014] [error] [client 192.168.30.196] [Fri May 30 16:09:09 2014] install.pl: [Fri May 30 16:09:09 2014] updatedatabase.pl: DBD::mysql::db do failed: Incorrect prefix key; the used key part isn't a string, the used length is longer than the key part, or the storage engine doesn't support unique prefix keys at /usr/share/koha/intranet/cgi-bin/installer/data/mysql/updatedatabase.pl line 8424., referer: http://192.168.30.13:8080/cgi-bin/koha/installer/install.pl
Comment 23 Doug Dearden 2014-06-06 21:29:29 UTC
Further investigation on this showed something interesting.  I have three databases - main, image, and archive.  The original DB is main, later I created image and still later archive - so archive was created on a newer version of Koha than the other two.  The update to archive did not throw these errors.  When I look at a pre-update version of the biblioitems table in the main and image DBs there are two indexes for issn - one called issn, the other issn_idx .  Looking at a post-update version there is only issn_idx - no issn or isbn indexes, and the isbn and issn fields are still varchar(30) and varchar(9).

I think adding 
    $dbh->do("ALTER TABLE biblioitems DROP INDEX issn_idx");
to the right spot in updatedatabase.pl probably fixes this.  I don't know why that old index was there on my main and image databases.
Comment 24 Jorge de Cardenas 2014-10-02 14:42:53 UTC
I am also getting the same errors as Doug Dearden. Did a package install of Koha 3.16.03 drop the database, created empty database and imported data from 3.10.
Then updated the schema.
Comment 25 Jorge de Cardenas 2014-10-02 21:01:07 UTC
In addition to the issn_idx index I also have isbnidx index. I will be dropping these and later retest.