Bug 34029 - Import breaks when data exceeds size of mapped database columns
Summary: Import breaks when data exceeds size of mapped database columns
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Katrin Fischer
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks: 34516 36978 34548 35180
  Show dependency treegraph
 
Reported: 2023-06-16 09:51 UTC by Marcel de Rooy
Modified: 2024-05-29 12:25 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.11.00,23.05.02,22.11.08
Circulation function:


Attachments
Bug 34029: Extend datatyps of common biblioitems.columns to longtext to avoid breaking import (7.54 KB, patch)
2023-06-16 11:45 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 34029: Extend datatyps of common biblioitems.columns to longtext to avoid breaking import (7.59 KB, patch)
2023-06-16 14:26 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 34029: Extend datatypes of biblioitems.columns to longtext to avoid breaking import (7.59 KB, patch)
2023-06-16 14:26 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 34029: Extend datatypes of biblioitems.columns to longtext to avoid breaking import (7.62 KB, patch)
2023-06-20 15:53 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 34029: Extend datatypes of biblioitems.columns to longtext to avoid breaking import (7.59 KB, patch)
2023-06-23 16:07 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 34029: (QA follow-up) Change from longtext to text (8.19 KB, patch)
2023-06-23 16:07 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 34029: Extend datatypes of biblioitems.columns to text to avoid breaking import (7.59 KB, patch)
2023-06-26 10:42 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 34029: (QA follow-up) Fix pushing undef to biblio_ids (1.39 KB, patch)
2023-06-26 10:54 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 34029: (QA follow-up) Fix slipped broken char (1.22 KB, patch)
2023-06-29 12:51 UTC, Tomás Cohen Arazi (tcohen)
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 2023-06-16 09:51:18 UTC
DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Data too long for column 'publishercode' at row 1 at /usr/share/koha/Koha/Object.pm line 170

DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Data too long for column 'publishercode' at row 1 at /usr/share/koha/Koha/Object.pm line 170

Use of uninitialized value $record_number in concatenation (.) or string at /usr/share/koha/C4/Biblio.pm line 2431.
C4::Biblio::ModZebra(): DBI Exception: DBD::mysql::st execute failed: Column 'biblio_auth_number' cannot be null at /usr/share/koha/Koha/SearchEngine/Zebra/Indexer.pm line 61
ModZebra( $record_number, $op, $server );

Whats happening here?
Two records have a too long publishercode.
Two undef's (from AddBiblio) are pushed to array @biblio_ids.
Indexing crashes after the import.
Comment 1 Marcel de Rooy 2023-06-16 09:54:17 UTC
<marcelr> we should prevent the indexing crash very simple by testing what we push to biblio_ids
Comment 2 Marcel de Rooy 2023-06-16 09:54:33 UTC
We could extend publishercode to TEXT
Comment 3 Katrin Fischer 2023-06-16 09:58:25 UTC
Bug 28328 increased several columns to longtext, biblio is almost completely longtext, so I suggest to do the same for the remaining varchars in biblioitems:

* place
* publishercode
* illus
* pages
* size
Comment 4 Katrin Fischer 2023-06-16 11:45:55 UTC
Created attachment 152404 [details] [review]
Bug 34029: Extend datatyps of common biblioitems.columns to longtext to avoid breaking import

This updated the datatype of the following columns from varchar(255) to longtext
to avoid import problems if the mapped MARC field's data exceeds 255
characters.

This is for:
* place (260$a and 264$a)
* publishercode (260$b and 264$b)
* size (300$c)
* illus (300$b)
* pages (300$a)

To test:
* Edit a bibliographic record and enter more than 255 characters into
  the MARC fields listed above. Try to save.
* Run the updatedatabase script to modify the DB structure
* restart_all
* Edit a bibliographic record and enter more than 255 characters into
  the MARC fields listed above.
* Save
* Export the record
* Import the record using staging tools, all should work well
Comment 5 Katrin Fischer 2023-06-16 14:26:09 UTC
Created attachment 152417 [details] [review]
Bug 34029: Extend datatyps of common biblioitems.columns to longtext to avoid breaking import

This updated the datatype of the following columns from varchar(255) to longtext
to avoid import problems if the mapped MARC field's data exceeds 255
characters.

This is for:
* place (260$a and 264$a)
* publishercode (260$b and 264$b)
* size (300$c)
* illus (300$b)
* pages (300$a)

To test:
* Edit a bibliographic record and enter more than 255 characters into
  the MARC fields listed above. Try to save.
* Run the updatedatabase script to modify the DB structure
* restart_all
* Edit a bibliographic record and enter more than 255 characters into
  the MARC fields listed above.
* Save
* Export the record
* Import the record using staging tools, all should work well
Comment 6 Katrin Fischer 2023-06-16 14:26:45 UTC
Created attachment 152418 [details] [review]
Bug 34029: Extend datatypes of biblioitems.columns to longtext to avoid breaking import

This updated the datatype of the following columns from varchar(255) to longtext
to avoid import problems if the mapped MARC field's data exceeds 255
characters.

This is for:
* place (260$a and 264$a)
* publishercode (260$b and 264$b)
* size (300$c)
* illus (300$b)
* pages (300$a)

To test:
* Edit a bibliographic record and enter more than 255 characters into
  the MARC fields listed above. Try to save.
* Run the updatedatabase script to modify the DB structure
* restart_all
* Edit a bibliographic record and enter more than 255 characters into
  the MARC fields listed above.
* Save
* Export the record
* Import the record using staging tools, all should work well
Comment 7 Marcel de Rooy 2023-06-19 06:44:43 UTC
(In reply to Katrin Fischer from comment #3)
> Bug 28328 increased several columns to longtext, biblio is almost completely
> longtext, so I suggest to do the same for the remaining varchars in
> biblioitems:
> 
> * place
> * publishercode
> * illus
> * pages
> * size

Might not really matter, but LONGTEXT (4G) seems to be absolute overkill for these five fields. Tinytext might be just too small.
Comment 8 Katrin Fischer 2023-06-19 18:40:42 UTC
(In reply to Marcel de Rooy from comment #7)
> (In reply to Katrin Fischer from comment #3)
> > Bug 28328 increased several columns to longtext, biblio is almost completely
> > longtext, so I suggest to do the same for the remaining varchars in
> > biblioitems:
> > 
> > * place
> > * publishercode
> > * illus
> > * pages
> > * size
> 
> Might not really matter, but LONGTEXT (4G) seems to be absolute overkill for
> these five fields. Tinytext might be just too small.

Mostly picked longtext because the majority of datatypes is already longtext if you look at biblio and biblioitems. What do you suggest? We also have a fair bit of mediumtext.
Comment 9 Marcel de Rooy 2023-06-20 11:16:21 UTC
(In reply to Katrin Fischer from comment #8)
> (In reply to Marcel de Rooy from comment #7)
> > (In reply to Katrin Fischer from comment #3)
> > > Bug 28328 increased several columns to longtext, biblio is almost completely
> > > longtext, so I suggest to do the same for the remaining varchars in
> > > biblioitems:
> > > 
> > > * place
> > > * publishercode
> > > * illus
> > > * pages
> > > * size
> > 
> > Might not really matter, but LONGTEXT (4G) seems to be absolute overkill for
> > these five fields. Tinytext might be just too small.
> 
> Mostly picked longtext because the majority of datatypes is already longtext
> if you look at biblio and biblioitems. What do you suggest? We also have a
> fair bit of mediumtext.

Comment2
Comment 10 Sam Lau 2023-06-20 15:53:35 UTC
Created attachment 152490 [details] [review]
Bug 34029: Extend datatypes of biblioitems.columns to longtext to avoid breaking import

This updated the datatype of the following columns from varchar(255) to longtext
to avoid import problems if the mapped MARC field's data exceeds 255
characters.

This is for:
* place (260$a and 264$a)
* publishercode (260$b and 264$b)
* size (300$c)
* illus (300$b)
* pages (300$a)

To test:
* Edit a bibliographic record and enter more than 255 characters into
  the MARC fields listed above. Try to save.
* Run the updatedatabase script to modify the DB structure
* restart_all
* Edit a bibliographic record and enter more than 255 characters into
  the MARC fields listed above.
* Save
* Export the record
* Import the record using staging tools, all should work well

Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 11 Marcel de Rooy 2023-06-23 07:50:44 UTC
Please change to TEXT
Comment 12 Katrin Fischer 2023-06-23 16:07:52 UTC
Created attachment 152647 [details] [review]
Bug 34029: Extend datatypes of biblioitems.columns to longtext to avoid breaking import

This updated the datatype of the following columns from varchar(255) to longtext
to avoid import problems if the mapped MARC field's data exceeds 255
characters.

This is for:
* place (260$a and 264$a)
* publishercode (260$b and 264$b)
* size (300$c)
* illus (300$b)
* pages (300$a)

To test:
* Edit a bibliographic record and enter more than 255 characters into
  the MARC fields listed above. Try to save.
* Run the updatedatabase script to modify the DB structure
* restart_all
* Edit a bibliographic record and enter more than 255 characters into
  the MARC fields listed above.
* Save
* Export the record
* Import the record using staging tools, all should work well
Comment 13 Katrin Fischer 2023-06-23 16:07:55 UTC
Created attachment 152648 [details] [review]
Bug 34029: (QA follow-up) Change from longtext to text

Can be squashed if needed.
Comment 14 Katrin Fischer 2023-06-23 16:10:13 UTC
(In reply to Marcel de Rooy from comment #11)
> Please change to TEXT

I've made the requested change - in a follow-up for easier review. Please feel free to squash later.
Comment 15 Marcel de Rooy 2023-06-26 10:42:30 UTC
Created attachment 152670 [details] [review]
Bug 34029: Extend datatypes of biblioitems.columns to text to avoid breaking import

This updated the datatype of the following columns from varchar(255) to text
to avoid import problems if the mapped MARC field's data exceeds 255
characters.

This is for:
* place (260$a and 264$a)
* publishercode (260$b and 264$b)
* size (300$c)
* illus (300$b)
* pages (300$a)

To test:
* Edit a bibliographic record and enter more than 255 characters into
  the MARC fields listed above. Try to save.
* Run the updatedatabase script to modify the DB structure
* restart_all
* Edit a bibliographic record and enter more than 255 characters into
  the MARC fields listed above.
* Save
* Export the record
* Import the record using staging tools, all should work well

Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 16 Marcel de Rooy 2023-06-26 10:42:56 UTC
RM: Please add DBIC changes too.
Comment 17 Marcel de Rooy 2023-06-26 10:54:11 UTC
Created attachment 152671 [details] [review]
Bug 34029: (QA follow-up) Fix pushing undef to biblio_ids

See comment1. Although we now fix the error on publishercode, it
is good to verify the result before pushing.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 18 Tomás Cohen Arazi (tcohen) 2023-06-26 14:41:29 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 19 Tomás Cohen Arazi (tcohen) 2023-06-29 12:45:12 UTC
Comment on attachment 152670 [details] [review]
Bug 34029: Extend datatypes of biblioitems.columns to text to avoid breaking import

Marcel:

>+  `place` text DEFAULT NULL COMMENT 'publication place (MARC21 260$a and 264§a)',

Katrin's patch didn't contain that char. You are evil :-D
Comment 20 Tomás Cohen Arazi (tcohen) 2023-06-29 12:51:49 UTC
Created attachment 152866 [details] [review]
Bug 34029: (QA follow-up) Fix slipped broken char

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 21 Marcel de Rooy 2023-06-29 13:21:14 UTC
(In reply to Tomás Cohen Arazi from comment #20)
> Created attachment 152866 [details] [review] [review]
> Bug 34029: (QA follow-up) Fix slipped broken char
> 
> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Good catch :)
Comment 22 Katrin Fischer 2023-06-30 18:56:18 UTC
I think it was me after all, thx for catching it!
Comment 23 Martin Renvoize (ashimema) 2023-07-17 14:41:37 UTC
Thanks for all the hard work!

Pushed to 23.05.x for the next release
Comment 24 Pedro Amorim 2023-07-18 11:55:09 UTC
Nice work everyone!

Pushed to 22.11.x for next release
Comment 25 Fridolin Somers 2023-07-28 19:33:04 UTC
There is a small typo in kohastructure.sql :
  `publishercode` text DEFAULT NULL COMMENT 'publisher (MARC21 260$b and 246$b)',

  246$b => 264$b

Maybe just fix in master
Comment 26 Katrin Fischer 2023-10-29 09:34:45 UTC
(In reply to Fridolin Somers from comment #25)
> There is a small typo in kohastructure.sql :
>   `publishercode` text DEFAULT NULL COMMENT 'publisher (MARC21 260$b and
> 246$b)',
> 
>   246$b => 264$b
> 
> Maybe just fix in master

As this has already been backported, I've opened a separate bug:

Bug 35180 - Fix typo in deletedbiblioitems.publishercode comment in kohastructure.sql