Bug 26684

Summary: Remove 'marc' column from auth_header table
Product: Koha Reporter: David Cook <dcook>
Component: MARC Authority data supportAssignee: Bugs List <koha-bugs>
Status: Signed Off --- QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: david, didier.gautheron, januszop, jonathan.druart, nick
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:
Attachments: Bug 26684: Remove marc from auth_header for new installs
Bug 26684: Remove marc column from existing databases
Bug 26684: [Do not push] Remember to update Koha::Schema::Result::AuthHeader
Bug 26684: Remove marc from auth_header for new installs
Bug 26684: Remove marc from auth_header for new installs
Bug 26684: Remove marc column from existing databases
Bug 26684: Remove marc from auth_header for new installs
Bug 26684: Remove marc column from existing databases
Bug 26684: [Do not push] Remember to update Koha::Schema::Result::AuthHeader
Bug 26684: Fix atomic update
Bug 26684: Remove marc from auth_header for new installs
Bug 26684: Remove marc column from existing databases
Bug 26684: [Do not push] Remember to update Koha::Schema::Result::AuthHeader
Bug 26684: Fix atomic update
Bug 26684: Remove marc from Koha::Authority::to_api_mapping
Bug 26684: Remove misc/exportauth.pl as a consequence of removing auth_header.marc
Bug 26684: Remove marc from auth_header for new installs
Bug 26684: Remove marc column from existing databases
Bug 26684: [Do not push] Remember to update Koha::Schema::Result::AuthHeader
Bug 26684: Fix atomic update
Bug 26684: Remove marc from Koha::Authority::to_api_mapping
Bug 26684: Remove misc/exportauth.pl as a consequence of removing auth_header.marc
Test data (one auth record) of size 92771 B

Description David Cook 2020-10-15 00:14:56 UTC
There's no need to have a marc column in the auth_header table anymore.

We'll need to change at least the following files:
./installer/data/mysql/kohastructure.sql
./C4/AuthoritiesMarc.pm
(AddAuthority, )
t/db_dependent/Linker_FirstMatch.t

We'll also need to drop the column using updatedatabase.pl:
./installer/data/mysql/updatedatabase.pl
Comment 1 David Cook 2020-10-15 01:00:54 UTC
Actually, we'll also need to re-generate Koha/Schema/Result/AuthHeader.pm
Comment 2 David Cook 2020-10-15 01:04:52 UTC
Created attachment 111670 [details] [review]
Bug 26684: Remove marc from auth_header for new installs
Comment 3 David Cook 2020-10-15 01:04:56 UTC
Created attachment 111671 [details] [review]
Bug 26684: Remove marc column from existing databases
Comment 4 David Cook 2020-10-15 01:05:00 UTC
Created attachment 111672 [details] [review]
Bug 26684: [Do not push] Remember to update Koha::Schema::Result::AuthHeader
Comment 5 Didier Gautheron 2020-10-20 06:38:15 UTC
Created attachment 112016 [details] [review]
Bug 26684: Remove marc from auth_header for new installs

Signed-off-by: Didier Gautheron <didier.gautheron@biblibre.com>
Comment 6 Didier Gautheron 2020-10-20 06:50:01 UTC
Created attachment 112018 [details] [review]
Bug 26684: Remove marc from auth_header for new installs

Signed-off-by: Didier Gautheron <didier.gautheron@biblibre.com>
Comment 7 Didier Gautheron 2020-10-20 06:50:06 UTC
Created attachment 112019 [details] [review]
Bug 26684: Remove marc column from existing databases

Signed-off-by: Didier Gautheron <didier.gautheron@biblibre.com>
Comment 8 Nick Clemens (kidclamp) 2020-10-20 13:38:11 UTC
Qa tools complaints:
 FAIL	installer/data/mysql/atomicupdate/001-26684-drop_marc_column_from_auth_header.pl
   FAIL	  critic
		Code before strictures are enabled at line 1, column 1. See page 429 of PBP.
   FAIL	  file permissions
		File must have the exec flag
   FAIL	  valid
		Name "main::dbh" used only once: possible typo 


if you move the file to 'perl' many disappear, however, the bug number is missing from the update so it fails right now, also we should check the existence of the column before droppping
Comment 9 David Cook 2020-10-21 04:50:15 UTC
Created attachment 112087 [details] [review]
Bug 26684: Remove marc from auth_header for new installs
Comment 10 David Cook 2020-10-21 04:50:21 UTC
Created attachment 112088 [details] [review]
Bug 26684: Remove marc column from existing databases
Comment 11 David Cook 2020-10-21 04:50:25 UTC
Created attachment 112089 [details] [review]
Bug 26684: [Do not push] Remember to update Koha::Schema::Result::AuthHeader
Comment 12 David Cook 2020-10-21 04:50:29 UTC
Created attachment 112090 [details] [review]
Bug 26684: Fix atomic update
Comment 13 David Cook 2020-10-21 04:50:53 UTC
QA scripts and updatedatabase.pl should be happy now
Comment 14 David Nind 2021-04-10 23:02:23 UTC
Patch no longer applies 8-(..

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 26684: Remove marc from auth_header for new installs
Using index info to reconstruct a base tree...
M	C4/AuthoritiesMarc.pm
M	installer/data/mysql/kohastructure.sql
Falling back to patching base and 3-way merge...
Auto-merging installer/data/mysql/kohastructure.sql
CONFLICT (content): Merge conflict in installer/data/mysql/kohastructure.sql
Auto-merging C4/AuthoritiesMarc.pm
error: Failed to merge in the changes.
Patch failed at 0001 Bug 26684: Remove marc from auth_header for new installs

Also, is there a test plan? I have no idea how to test this one...
Comment 15 Janusz Kaczmarek 2025-02-02 14:33:22 UTC
Created attachment 177445 [details] [review]
Bug 26684: Remove marc from auth_header for new installs
Comment 16 Janusz Kaczmarek 2025-02-02 14:33:25 UTC
Created attachment 177446 [details] [review]
Bug 26684: Remove marc column from existing databases
Comment 17 Janusz Kaczmarek 2025-02-02 14:33:27 UTC
Created attachment 177447 [details] [review]
Bug 26684: [Do not push] Remember to update Koha::Schema::Result::AuthHeader
Comment 18 Janusz Kaczmarek 2025-02-02 14:33:30 UTC
Created attachment 177448 [details] [review]
Bug 26684: Fix atomic update
Comment 19 Janusz Kaczmarek 2025-02-02 14:56:59 UTC
The auth_header.marc column should definitely be remover. It is outdated, never used (only stored), and we have removed marc from biblio long time ago. Morover, it is of size blob (64K), which means it cannot properly host large ISO 2709 records (up to 99K). 

While saving large records we get warning:
DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Data too long for column 'marc' at row 1 at /kohadevbox/koha/Koha/Object.pm line 172

and creation / modification is not properly performed.
Comment 20 Janusz Kaczmarek 2025-02-02 15:26:05 UTC
Created attachment 177449 [details] [review]
Bug 26684: Remove marc from Koha::Authority::to_api_mapping
Comment 21 Janusz Kaczmarek 2025-02-02 17:10:51 UTC
Created attachment 177450 [details] [review]
Bug 26684: Remove misc/exportauth.pl as a consequence of removing auth_header.marc

Since we have misc/export_records.pl and we want to remove
auth_header.marc, the ancient script misc/exportauth.pl can be removed
as well (it is relied upon auth_header.marc).
Comment 22 Roman Dolny 2025-02-02 18:03:57 UTC
Created attachment 177455 [details] [review]
Bug 26684: Remove marc from auth_header for new installs

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Comment 23 Roman Dolny 2025-02-02 18:04:00 UTC
Created attachment 177456 [details] [review]
Bug 26684: Remove marc column from existing databases

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Comment 24 Roman Dolny 2025-02-02 18:04:03 UTC
Created attachment 177457 [details] [review]
Bug 26684: [Do not push] Remember to update Koha::Schema::Result::AuthHeader

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Comment 25 Roman Dolny 2025-02-02 18:04:05 UTC
Created attachment 177458 [details] [review]
Bug 26684: Fix atomic update

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Comment 26 Roman Dolny 2025-02-02 18:04:08 UTC
Created attachment 177459 [details] [review]
Bug 26684: Remove marc from Koha::Authority::to_api_mapping

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Comment 27 Roman Dolny 2025-02-02 18:04:11 UTC
Created attachment 177460 [details] [review]
Bug 26684: Remove misc/exportauth.pl as a consequence of removing auth_header.marc

Since we have misc/export_records.pl and we want to remove
auth_header.marc, the ancient script misc/exportauth.pl can be removed
as well (it is relied upon auth_header.marc).

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Comment 28 David Nind 2025-02-02 19:14:09 UTC
Please add a test plan, and I'd be happy to test. (Replicate the issue (and what the issue is), apply patches, demonstrate that the issue is fixed.)
Comment 29 David Nind 2025-02-02 19:16:24 UTC
Sorry, I thought this was waiting for sign off (has now been signed off).

A test plan would be ideal for the QA step, and a release note would be great to explain what this fixes and why.
Comment 30 Janusz Kaczmarek 2025-02-02 20:00:57 UTC
Created attachment 177461 [details]
Test data (one auth record) of size 92771 B
Comment 31 Janusz Kaczmarek 2025-02-02 20:10:40 UTC
(In reply to David Nind from comment #29)
> A test plan would be ideal for the QA step, and a release note would be
> great to explain what this fixes and why.

It is hard to say about a strict test plan. One aspect of problems causing by this column can be illustrates by the following sequence of steps.

1. Take the provided test auth record (100 Baggins, Bilbo) with lots od 670 fields, of size 92771 B: still in the limits of ISO 2709, but significantly bigger than blob size (64 KB). 
2. Try to import the record into ktd installation with:
misc/migration_tools/bulkmarcimport.pl -a -d -file Baggins_Bilbo.mrc -fk
You should see a series of warnings:
DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Data too long for column 'marc' at row 1 at /kohadevbox/koha/Koha/Object.pm line 172
Use of uninitialized value $originalid in concatenation (.) or string at ./misc/migration_tools/bulkmarcimport.pl line 520, <GEN3> line 1.
ERROR: Insert authority  failed: DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Data too long for column 'marc' at row 1 at /kohadevbox/koha/Koha/Object.pm line 172
3. Control in ktd --dbshell with:
select * from auth_header;
You should see one row with no data in marc nor in marcxml.
4. Apply the patch ; updatedatabase.
5. Repeat p. 2 and p. 3. You should get no warnings and expected (huge) result from MySQL. 

Other aspects should be evaluated by code reviewers.  The koha_header.marc column should not be used in the code anymore with this patch.
Comment 32 David Nind 2025-02-02 20:21:16 UTC
Thanks Janusz!