|
Description
Marcel de Rooy
2023-06-16 09:51:18 UTC
<marcelr> we should prevent the indexing crash very simple by testing what we push to biblio_ids We could extend publishercode to TEXT 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 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 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 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 (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. (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. (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 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> Please change to TEXT 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 Created attachment 152648 [details] [review] Bug 34029: (QA follow-up) Change from longtext to text Can be squashed if needed. (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. 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> RM: Please add DBIC changes too. 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> Pushed to master for 23.11. Nice work everyone, thanks! 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 Created attachment 152866 [details] [review] Bug 34029: (QA follow-up) Fix slipped broken char Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> (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 :) I think it was me after all, thx for catching it! Thanks for all the hard work! Pushed to 23.05.x for the next release Nice work everyone! Pushed to 22.11.x for next release 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 (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 |