Bug 31716 - Enlarge the size of the authorised_value column in authorised_values
Summary: Enlarge the size of the authorised_value column in authorised_values
Status: RESOLVED WONTFIX
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-10-07 18:59 UTC by Nick Clemens (kidclamp)
Modified: 2023-06-11 15:56 UTC (History)
3 users (show)

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


Attachments
Bug 31716: Enlagre the authorised_value column in authorised_values (2.63 KB, patch)
2022-10-07 19:27 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 31716: DO NOT PUSH - Schema updates (1.51 KB, patch)
2022-10-07 19:27 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 31716: Enlarge the authorised_value column in authorised_values (4.01 KB, patch)
2022-10-12 11:22 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 31716: Prevent issues with foreign key (1.62 KB, patch)
2022-10-12 11:22 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 31716: DO NOT PUSH - Schema updates (1.51 KB, patch)
2022-10-12 11:22 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 31716: Enlarge the authorised_value column in authorised_values (4.06 KB, patch)
2022-10-14 20:29 UTC, David Nind
Details | Diff | Splinter Review
Bug 31716: Prevent issues with foreign key (1.67 KB, patch)
2022-10-14 20:29 UTC, David Nind
Details | Diff | Splinter Review
Bug 31716: DO NOT PUSH - Schema updates (1.56 KB, patch)
2022-10-14 20:29 UTC, David Nind
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens (kidclamp) 2022-10-07 18:59:40 UTC
Some libraries want to store URLs or longer values in the AVs to make entry easier. Currently there is an 80 character limit
Comment 1 Nick Clemens (kidclamp) 2022-10-07 19:27:36 UTC
Created attachment 141520 [details] [review]
Bug 31716: Enlagre the authorised_value column in authorised_values

To test:
1 - Apply patch
2 - Update database
3 - Confirm you can store a longer value in authorised_values
Comment 2 Nick Clemens (kidclamp) 2022-10-07 19:27:39 UTC
Created attachment 141521 [details] [review]
Bug 31716: DO NOT PUSH - Schema updates
Comment 3 David Nind 2022-10-08 21:44:31 UTC
I can now update an authorized value code using the database, but I can't add or edit using the staff interface as the form entry's maximum length is 80: <input type="text" id="authorised_value" name="authorised_value" value="" maxlength="80" class="focus" />.

When you do try to edit an entry, you get this message on the edit form "Please enter no more than 80 characters."

To update a value in the database:

1. koha-mysql kohadev

2. After applying the patch (I also ran dbic - not sure whether that is required) you can see that the change to the database structure has been enabled: show columns from authorised_values;

3. SQL to update an entry - where XXXX = the id of an existing entry: update authorised_values set authorised_value="This is a really really long authorized value although I am not sure why you would want such a code" where id = XXXX;
Comment 4 Marcel de Rooy 2022-10-09 06:55:32 UTC
ALTER TABLE authorised_values MODIFY COLUMN authorised_value varchar(255)

You go from 80 to 255. At some point that is too low again. We should imo go to a TEXT type: A TEXT column with a maximum length of 65,535 (216 - 1) characters.
One point: VARCHAR columns can be fully indexed. TEXT columns can only be indexed over a specified length.
Comment 5 Nick Clemens (kidclamp) 2022-10-12 11:22:49 UTC
Created attachment 141722 [details] [review]
Bug 31716: Enlarge the authorised_value column in authorised_values

This patch changes authorised_value from VARCHAR(80) to VARCHAR(255)

To test:
1 - Apply patch
2 - Update database
3 - Confirm you can store a longer value in authorised_values
Comment 6 Nick Clemens (kidclamp) 2022-10-12 11:22:53 UTC
Created attachment 141723 [details] [review]
Bug 31716: Prevent issues with foreign key

VARCHAR is now allowed to be 65535 bytes max, as opposed to 255 - when testing this,
however, I received errors about latering a column with a foreign key. Adding this to the update
to avoid problems like bug 31673.
Comment 7 Nick Clemens (kidclamp) 2022-10-12 11:22:56 UTC
Created attachment 141724 [details] [review]
Bug 31716: DO NOT PUSH - Schema updates
Comment 8 Nick Clemens (kidclamp) 2022-10-12 11:25:03 UTC
(In reply to David Nind from comment #3)
> I can now update an authorized value code using the database, but I can't
> add or edit using the staff interface as the form entry's maximum length is
> 80: <input type="text" id="authorised_value" name="authorised_value"
> value="" maxlength="80" class="focus" />.
> 
Fixed

(In reply to Marcel de Rooy from comment #4)
> You go from 80 to 255. At some point that is too low again. We should imo go
> to a TEXT type: A TEXT column with a maximum length of 65,535 (216 - 1)
> characters.
> One point: VARCHAR columns can be fully indexed. TEXT columns can only be
> indexed over a specified length.

Some versions of MySQL/Maria allow varchar up to 65535 - so we can increase in future as needed - this was the largest push without more work and sufficient for current needs
Comment 9 Marcel de Rooy 2022-10-12 11:29:46 UTC
(In reply to Nick Clemens from comment #8)
> (In reply to Marcel de Rooy from comment #4)
> > You go from 80 to 255. At some point that is too low again. We should imo go
> > to a TEXT type: A TEXT column with a maximum length of 65,535 (216 - 1)
> > characters.
> > One point: VARCHAR columns can be fully indexed. TEXT columns can only be
> > indexed over a specified length.
> 
> Some versions of MySQL/Maria allow varchar up to 65535 - so we can increase
> in future as needed - this was the largest push without more work and
> sufficient for current needs

URLs could be up to 2048 chars? Note that addressing the authorized values table is only half of the work. The receiving column should be able to store it too. But you know :)
Comment 10 Nick Clemens (kidclamp) 2022-10-12 11:38:24 UTC
(In reply to Marcel de Rooy from comment #9)
> (In reply to Nick Clemens from comment #8)
> > (In reply to Marcel de Rooy from comment #4)

> URLs could be up to 2048 chars? Note that addressing the authorized values
> table is only half of the work. The receiving column should be able to store
> it too. But you know :)

Indeed, flexibility comes with responsibility sometimes :-)
Comment 11 David Nind 2022-10-14 20:29:31 UTC
Created attachment 141904 [details] [review]
Bug 31716: Enlarge the authorised_value column in authorised_values

This patch changes authorised_value from VARCHAR(80) to VARCHAR(255)

To test:
1 - Apply patch
2 - Update database
3 - Confirm you can store a longer value in authorised_values

Signed-off-by: David Nind <david@davidnind.com>
Comment 12 David Nind 2022-10-14 20:29:36 UTC
Created attachment 141905 [details] [review]
Bug 31716: Prevent issues with foreign key

VARCHAR is now allowed to be 65535 bytes max, as opposed to 255 - when testing this,
however, I received errors about latering a column with a foreign key. Adding this to the update
to avoid problems like bug 31673.

Signed-off-by: David Nind <david@davidnind.com>
Comment 13 David Nind 2022-10-14 20:29:42 UTC
Created attachment 141906 [details] [review]
Bug 31716: DO NOT PUSH - Schema updates

Signed-off-by: David Nind <david@davidnind.com>
Comment 14 Jonathan Druart 2022-10-27 10:45:21 UTC
And you make them mismatch again. All columns must be modified IMO (and be FK ideally).
Comment 15 Nick Clemens (kidclamp) 2022-10-27 15:17:29 UTC
(In reply to Jonathan Druart from comment #14)
> And you make them mismatch again. All columns must be modified IMO (and be
> FK ideally).

What is mismatching? Can you provide some detail
Comment 16 Katrin Fischer 2022-10-27 17:49:40 UTC
(In reply to Nick Clemens from comment #15)
> (In reply to Jonathan Druart from comment #14)
> > And you make them mismatch again. All columns must be modified IMO (and be
> > FK ideally).
> 
> What is mismatching? Can you provide some detail

Actually this concerns me too. When you allow to have longer values for the code, we will have the situation, that people are likely to create longer descriptions that won't in most places.

Think: items.ccode, items.location, asort1/asort2 in aqorders etc.

May I make a different suggestion?

If these are for cataloguing, maybe we could make it an option in the frameworks so that not the code but the description will be stored. That might prove nice in other situations as well.
Comment 17 Katrin Fischer 2022-10-27 17:50:20 UTC
longer descriptions should be 'longer codes'! I don't mind the long descriptions...
Comment 18 Katrin Fischer 2022-10-27 19:51:40 UTC
I feel it would be worth to learn more about the use case here.
Comment 19 Nick Clemens (kidclamp) 2022-10-28 14:52:07 UTC
Resolved using hardcoded JS, not going to keep working on this one
Comment 20 Katrin Fischer 2023-06-11 15:56:22 UTC
(In reply to Nick Clemens from comment #19)
> Resolved using hardcoded JS, not going to keep working on this one