Bug 20882 - URI column in the items table is limited to 255 characters
Summary: URI column in the items table is limited to 255 characters
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Marcel de Rooy
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-06-05 10:52 UTC by Nick Clemens
Modified: 2020-12-02 14:40 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Medium patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.05.00, 19.11.04, 19.05.09


Attachments
Bug 20882: Move items.uri to mediumtext (3.05 KB, patch)
2020-02-10 14:31 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 20882: Move items.uri to mediumtext (3.06 KB, patch)
2020-02-10 14:37 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 20882: Move items.uri to mediumtext (3.12 KB, patch)
2020-02-14 17:48 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2018-06-05 10:52:30 UTC
This should be 'text' to match the URL colum in biblioitems
Comment 1 David Cook 2020-02-07 01:36:03 UTC
biblioitems.url now uses MEDIUMTEXT

However, I don't think items.uri necessarily need to be the same as biblioitems.url.

With biblioitems.url, multiple URLs are concatenated in the same field, but items.uri should only ever store 1 URL.

This link presents a number of different length limits for 1 URL:
https://stackoverflow.com/questions/417142/what-is-the-maximum-length-of-a-url-in-different-browsers

The largest CloudFlare seems to support is 32Kb and a TEXT field can store 64KB. 

I suppose using MEDIUMTEXT could provide some future proofing, but TEXT is probably good enough for items.uri.
Comment 2 Marcel de Rooy 2020-02-10 13:45:07 UTC
(In reply to David Cook from comment #1)
> biblioitems.url now uses MEDIUMTEXT
> 
> However, I don't think items.uri necessarily need to be the same as
> biblioitems.url.

Since it is variable length, I dont think making the distinction is needed. The link you gave talks about keeping it under 2048 btw. I would go for MEDIUMTEXT here too for consistency.

> With biblioitems.url, multiple URLs are concatenated in the same field, but
> items.uri should only ever store 1 URL.

Why only ever 1? Our 952$u is derived from 852$u which is a repeatable field. Unfortunately our editor does not yet support it [1]. But the Koha to MARC mapping underneath supports url1 | url2 | url3 etc., like in biblioitems.url.


[1] It does not support adding a subfield directly. But if you save url1 | url2 in $u and reopen the item record, the field is cloned for you. Editing and savin g again is supported now too. Which is something that we wanted to prevent on bug 21800 but luckily we did not yet so far ;)
Comment 3 Marcel de Rooy 2020-02-10 13:58:18 UTC
(In reply to Marcel de Rooy from comment #2)
> [1] It does not support adding a subfield directly. But if you save url1 |
> url2 in $u and reopen the item record, the field is cloned for you. Editing
> and savin g again is supported now too. Which is something that we wanted to
> prevent on bug 21800 but luckily we did not yet so far ;)

Not even completely true. In master you can set the item field repeatable and you have a clone button. It acts a bit weird but works. See bug 13574.
Comment 4 Marcel de Rooy 2020-02-10 14:31:49 UTC
Created attachment 98658 [details] [review]
Bug 20882: Move items.uri to mediumtext

Applies to items and deleteditems.

Test plan:
Run new install or upgrade. Check field size.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 5 Nick Clemens 2020-02-10 14:37:49 UTC
Created attachment 98661 [details] [review]
Bug 20882: Move items.uri to mediumtext

Applies to items and deleteditems.

Test plan:
Run new install or upgrade. Check field size.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 6 Marcel de Rooy 2020-02-10 15:29:37 UTC
(In reply to Nick Clemens from comment #5)
> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Thx for very fast SO !
Comment 7 Katrin Fischer 2020-02-14 17:48:41 UTC
Created attachment 98995 [details] [review]
Bug 20882: Move items.uri to mediumtext

Applies to items and deleteditems.

Test plan:
Run new install or upgrade. Check field size.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 8 Katrin Fischer 2020-02-14 17:49:14 UTC
(In reply to Marcel de Rooy from comment #6)
> (In reply to Nick Clemens from comment #5)
> > Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
> 
> Thx for very fast SO !

Thx for a nice clean patch and not forgetting about deleteditems :)
Comment 9 Martin Renvoize 2020-02-17 10:45:59 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 10 Joy Nelson 2020-03-05 01:23:06 UTC
Pushed to 19.11.x branch for 19.11.04
Comment 11 Lucas Gass 2020-03-09 21:16:05 UTC
backported to 19.05.x for 19.05.09
Comment 12 Jonathan Druart 2020-05-14 11:09:38 UTC
(In reply to Lucas Gass from comment #11)
> backported to 19.05.x for 19.05.09

Lucas, there is a typo in
  commit 7b3c2d287ace83ac19747e31f858e8b2beccdc2e
  Bug 20882: DBRev 19.05.08.001


-#!/usr/bin/perl
+#!/USr/bin/perl
Comment 13 Fridolin Somers 2020-12-02 14:40:23 UTC
DBIx class changes has been pushed in 19.11.08 :
58e7d0b44a Bug 20882: DBIC