Bug 26684 - Remove 'marc' column from auth_header table
Summary: Remove 'marc' column from auth_header table
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: MARC Authority data support (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: David Cook
QA Contact: Nick Clemens (kidclamp)
URL:
Keywords: additional_work_needed, release-notes-needed
Depends on:
Blocks: 30888
  Show dependency treegraph
 
Reported: 2020-10-15 00:14 UTC by David Cook
Modified: 2025-02-21 10:00 UTC (History)
6 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:
25.05.00
Circulation function:


Attachments
Bug 26684: Remove marc from auth_header for new installs (2.23 KB, patch)
2020-10-15 01:04 UTC, David Cook
Details | Diff | Splinter Review
Bug 26684: Remove marc column from existing databases (1.00 KB, patch)
2020-10-15 01:04 UTC, David Cook
Details | Diff | Splinter Review
Bug 26684: [Do not push] Remember to update Koha::Schema::Result::AuthHeader (1.01 KB, patch)
2020-10-15 01:05 UTC, David Cook
Details | Diff | Splinter Review
Bug 26684: Remove marc from auth_header for new installs (2.29 KB, patch)
2020-10-20 06:38 UTC, Didier Gautheron
Details | Diff | Splinter Review
Bug 26684: Remove marc from auth_header for new installs (2.29 KB, patch)
2020-10-20 06:50 UTC, Didier Gautheron
Details | Diff | Splinter Review
Bug 26684: Remove marc column from existing databases (1.06 KB, patch)
2020-10-20 06:50 UTC, Didier Gautheron
Details | Diff | Splinter Review
Bug 26684: Remove marc from auth_header for new installs (2.23 KB, patch)
2020-10-21 04:50 UTC, David Cook
Details | Diff | Splinter Review
Bug 26684: Remove marc column from existing databases (1.01 KB, patch)
2020-10-21 04:50 UTC, David Cook
Details | Diff | Splinter Review
Bug 26684: [Do not push] Remember to update Koha::Schema::Result::AuthHeader (1.01 KB, patch)
2020-10-21 04:50 UTC, David Cook
Details | Diff | Splinter Review
Bug 26684: Fix atomic update (1.15 KB, patch)
2020-10-21 04:50 UTC, David Cook
Details | Diff | Splinter Review
Bug 26684: Remove marc from auth_header for new installs (1.76 KB, patch)
2025-02-02 14:33 UTC, Janusz Kaczmarek
Details | Diff | Splinter Review
Bug 26684: Remove marc column from existing databases (1.00 KB, patch)
2025-02-02 14:33 UTC, Janusz Kaczmarek
Details | Diff | Splinter Review
Bug 26684: [Do not push] Remember to update Koha::Schema::Result::AuthHeader (1.01 KB, patch)
2025-02-02 14:33 UTC, Janusz Kaczmarek
Details | Diff | Splinter Review
Bug 26684: Fix atomic update (1.14 KB, patch)
2025-02-02 14:33 UTC, Janusz Kaczmarek
Details | Diff | Splinter Review
Bug 26684: Remove marc from Koha::Authority::to_api_mapping (764 bytes, patch)
2025-02-02 15:26 UTC, Janusz Kaczmarek
Details | Diff | Splinter Review
Bug 26684: Remove misc/exportauth.pl as a consequence of removing auth_header.marc (1.34 KB, patch)
2025-02-02 17:10 UTC, Janusz Kaczmarek
Details | Diff | Splinter Review
Bug 26684: Remove marc from auth_header for new installs (1.81 KB, patch)
2025-02-02 18:03 UTC, Roman Dolny
Details | Diff | Splinter Review
Bug 26684: Remove marc column from existing databases (1.05 KB, patch)
2025-02-02 18:04 UTC, Roman Dolny
Details | Diff | Splinter Review
Bug 26684: [Do not push] Remember to update Koha::Schema::Result::AuthHeader (1.06 KB, patch)
2025-02-02 18:04 UTC, Roman Dolny
Details | Diff | Splinter Review
Bug 26684: Fix atomic update (1.20 KB, patch)
2025-02-02 18:04 UTC, Roman Dolny
Details | Diff | Splinter Review
Bug 26684: Remove marc from Koha::Authority::to_api_mapping (816 bytes, patch)
2025-02-02 18:04 UTC, Roman Dolny
Details | Diff | Splinter Review
Bug 26684: Remove misc/exportauth.pl as a consequence of removing auth_header.marc (1.40 KB, patch)
2025-02-02 18:04 UTC, Roman Dolny
Details | Diff | Splinter Review
Test data (one auth record) of size 92771 B (1.76 KB, application/x-zip-compressed)
2025-02-02 20:00 UTC, Janusz Kaczmarek
Details
Bug 26684: Remove marc from auth_header for new installs (1.87 KB, patch)
2025-02-07 14:15 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 26684: Remove marc column from existing databases (1.11 KB, patch)
2025-02-07 14:15 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 26684: [Do not push] Remember to update Koha::Schema::Result::AuthHeader (1.12 KB, patch)
2025-02-07 14:15 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 26684: Fix atomic update (1.25 KB, patch)
2025-02-07 14:15 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 26684: Remove marc from Koha::Authority::to_api_mapping (874 bytes, patch)
2025-02-07 14:15 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 26684: Remove misc/exportauth.pl as a consequence of removing auth_header.marc (1.45 KB, patch)
2025-02-07 14:15 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 26684: Remove marc from auth_header for new installs (1.98 KB, patch)
2025-02-18 14:50 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 26684: Remove marc column from existing databases (1.15 KB, patch)
2025-02-18 14:50 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 26684: [Do not push] Remember to update Koha::Schema::Result::AuthHeader (1.16 KB, patch)
2025-02-18 14:51 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 26684: Fix atomic update (1.29 KB, patch)
2025-02-18 14:51 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 26684: Remove marc from Koha::Authority::to_api_mapping (912 bytes, patch)
2025-02-18 14:51 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 26684: Remove misc/exportauth.pl as a consequence of removing auth_header.marc (1.50 KB, patch)
2025-02-18 14:51 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 26684: {DBIx] Schema changes for AuthHeader.pm (1.43 KB, patch)
2025-02-18 15:40 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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!
Comment 33 Nick Clemens (kidclamp) 2025-02-07 14:15:41 UTC
Created attachment 177622 [details] [review]
Bug 26684: Remove marc from auth_header for new installs

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 34 Nick Clemens (kidclamp) 2025-02-07 14:15:43 UTC
Created attachment 177623 [details] [review]
Bug 26684: Remove marc column from existing databases

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 35 Nick Clemens (kidclamp) 2025-02-07 14:15:45 UTC
Created attachment 177624 [details] [review]
Bug 26684: [Do not push] Remember to update Koha::Schema::Result::AuthHeader

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 36 Nick Clemens (kidclamp) 2025-02-07 14:15:47 UTC
Created attachment 177625 [details] [review]
Bug 26684: Fix atomic update

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 37 Nick Clemens (kidclamp) 2025-02-07 14:15:49 UTC
Created attachment 177626 [details] [review]
Bug 26684: Remove marc from Koha::Authority::to_api_mapping

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 38 Nick Clemens (kidclamp) 2025-02-07 14:15:52 UTC
Created attachment 177627 [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>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 39 Katrin Fischer 2025-02-12 13:48:11 UTC
I agree about removing the column, but I don't believe this is a bug fix. Setting to enhancement (moving back in queue a little).

Also: setting Assignee and QA contact...
Comment 40 Janusz Kaczmarek 2025-02-15 22:37:50 UTC
(In reply to Katrin Fischer from comment #39)
> I agree about removing the column, but I don't believe this is a bug fix.
> Setting to enhancement (moving back in queue a little).
> 
> Also: setting Assignee and QA contact...

I wouldn't agree that it is just enhancement. IMO it is a real and quite serious bug. Please see comments # 19 and # 31. The limit of the size of blob makes auth_table and the whole authorities infrastructure fail in case of authority records of the size >= 64 KB (rare but real cases).
Comment 41 Katrin Fischer 2025-02-17 08:02:00 UTC
It's still quite a big change, so I am not sure it should be backported.
Comment 42 Janusz Kaczmarek 2025-02-17 22:46:04 UTC
(In reply to Katrin Fischer from comment #41)
> It's still quite a big change, so I am not sure it should be backported.

I get the point. I do not insist. It seems serious and it is but it shouldn't hurt anybody. On the contrary, it could save maybe not the life, but data ;)
Comment 43 Marcel de Rooy 2025-02-18 14:49:01 UTC
Applying: Bug 26684: Remove misc/exportauth.pl as a consequence of removing auth_header.marc
Using index info to reconstruct a base tree...
M       misc/exportauth.pl
Falling back to patching base and 3-way merge...
CONFLICT (modify/delete): misc/exportauth.pl deleted in Bug 26684: Remove misc/exportauth.pl as a consequence of removing auth_header.marc and modified in HEAD. Version HEAD of misc/exportauth.pl left in tree.
error: Failed to merge in the changes.

Please rebase. I need to rebase another one on top..
Comment 44 Marcel de Rooy 2025-02-18 14:50:56 UTC
Created attachment 178277 [details] [review]
Bug 26684: Remove marc from auth_header for new installs

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 45 Marcel de Rooy 2025-02-18 14:50:59 UTC
Created attachment 178278 [details] [review]
Bug 26684: Remove marc column from existing databases

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 46 Marcel de Rooy 2025-02-18 14:51:01 UTC
Created attachment 178279 [details] [review]
Bug 26684: [Do not push] Remember to update Koha::Schema::Result::AuthHeader

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 47 Marcel de Rooy 2025-02-18 14:51:04 UTC
Created attachment 178280 [details] [review]
Bug 26684: Fix atomic update

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 48 Marcel de Rooy 2025-02-18 14:51:07 UTC
Created attachment 178281 [details] [review]
Bug 26684: Remove marc from Koha::Authority::to_api_mapping

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 49 Marcel de Rooy 2025-02-18 14:51:10 UTC
Created attachment 178282 [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>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 50 Marcel de Rooy 2025-02-18 14:51:44 UTC
Well, it was a trivial rebase after all
Comment 51 Marcel de Rooy 2025-02-18 15:14:28 UTC
DBIx::Class::Schema::Loader::make_schema_at(): Checksum mismatch in './/Koha/Schema/Result/AuthHeader.pm', the auto-generated part of the file has been modified outside of this loader.  Aborting.

Please run misc/devel/update_dbix_class_files.pl to prevent that when generating the new schema file.
Comment 52 Marcel de Rooy 2025-02-18 15:40:19 UTC
Created attachment 178285 [details] [review]
Bug 26684: {DBIx] Schema changes for AuthHeader.pm
Comment 53 Katrin Fischer 2025-02-21 09:13:39 UTC
I can see these are really old patches still using the old atomicupdate template. I will port them, but if you see such things, please help and save some RM time.
Comment 54 Katrin Fischer 2025-02-21 09:42:13 UTC
Pushed for 25.05!

Well done everyone, thank you!
Comment 55 Katrin Fischer 2025-02-21 10:00:34 UTC
This change requires a change to our sample data used by ktd (reset_all) and Jenkins: auth_header.sql

I've filed:
https://gitlab.com/koha-community/koha-misc4dev/-/issues/86

We need an urgent fix please.