We had a lot of auth_header (~700.000). Then we decided to import a lot more (10.000.000). We used --update together with a match-rule (based on 024) to prevent duplicates. Which all worked well. Until... We discovered that ~30k of our old auths where overwritten with "random" data (some other auths). What used to be "Graz" was now "Centro Editoriale Valtortiano". After a bit of panic we were able to restore our old auths from a backup. But what happend? The problem is that eg "Graz" had authid=777317. But "Valtortiano" has 001=00777317X. If you squint a bit (and ignore the leading 0s and the X at the end) that's the same number! And it seems that C4::AuthoritiesMarc::GuessAuthId($record) returns the value of 001, which somehow causes the confusion. I'm not entirely sure yet, where the X is stripped, but the effect is that auth-record 777317 is overwritten. An easy fix would be to add a new command line param to bulkmarcimport to either disable the call to GuessAuthId (which keeps with backwards compat, but makes it easy for other people to also run into this problem); or to only call GuessAuthId if the new param is set (saver, but changes behaviour) If you agree that this is a problem, I can prepare a patch. If we did something stupid, please enlighten us!
Overall this doesn't sound like intended behavior. Koha did not always replace 001 with the authid, in the distant past it kept the imported 001. I could imagine the behavior made more sense back then, but does much less now. I see 2 different bugs here: * It breaks importing for everyone that imports data with an existing 001 that is somewhat numerical and can clash with the authids in Koha that are added to 001 on import. * The guess is not even exact. I assume this is because of using the search index, but maybe there should be an additional Perl check if they are really equivalent before overlay? I would not be opposed to change the behavior here or make it a legacy switch that needs to be chosen actively. The current behavior appears quite dangerous. Some other thoughts: Could it make sense to forgo GuessAuthId when another match-rule is used? And maybe have a separate bug for making the match more exact/smarter.
Created attachment 161303 [details] handcrafted auth marcxml with controlnumber 1402
Created attachment 161304 [details] auth_1402_pre_import.png - screenshot before import
Created attachment 161305 [details] auth_1402_post_import.png - screenshot after import
Here's some more info and tests for this bug: * twin_040682714.xml is this GND record http://d-nb.info/gnd/4068271-7 ("Zwilling", i.e. twin), but modified to have <controlfield tag="001">1402</controlfield>, which matches the authid 1402 "Aristotle" in koha-testing-docker * auth_1402_pre_import.png is a screenshot of authid 1402 from koha-testing-docker * copy twin_040682714.xml to the root of your koha-testing-docker (so it will be available inside the container) * run: misc/migration_tools/bulkmarcimport.pl -m MARCXML -c utf8 -a --file twin_040682714.xml --all --match isbn,001 (you can also run with --match, makes no difference, as long as --match does not actually hit a match) * auth_1402_post_import.png is a screenshot of authid 1402 after running bulkmarcimport, showing that the authid 1402 now contains the data from twin_040682714.xml
Created attachment 161306 [details] [review] Bug 35591: Prevent bulkmarcimport from replacing local auth_header If you run bulkmarcimport,pl on authorities with --update (or --all) it will overwrite a local auth_header if the auth_id of a local auth_header matches the control-number (field 001) of an imported MARC record. This patch adds a new option --guessauthid (which is still enabled by default to keep the script backwards compatible, but broken), which can turn off this behaviour if called as --noguessauthid. To test with koha-testing-docker: * Inspect cgi-bin/koha/authorities/detail.pl?authid=1402 which should be PERSO_NAME of Aristotles. * get the file twin_040682714.xml from Bugzilla and make it available inside of koha-testing-docker * run: misc/migration_tools/bulkmarcimport.pl -m MARCXML -c utf8 -a --file twin_040682714.xml --all --match isbn,001 * Check cgi-bin/koha/authorities/detail.pl?authid=1402, which now is a TOPIC_TERM "Zwilling" (twin...) Reset koha-testing-docker, apply the patch * run with the new flag misc/migration_tools/bulkmarcimport.pl -m MARCXML -c utf8 -a --file twin_040682714.xml --all --match isbn,001 --noguessauthid * cgi-bin/koha/authorities/detail.pl?authid=1402 still is "Aristotle" * "Zwilling" is cgi-bin/koha/authorities/detail.pl?authid=1708 (at least on my ktd, the actual id might vary)
Sponsored-by: Steiermärkische Landesbibliothek
Created attachment 162409 [details] [review] Bug 35591: Prevent bulkmarcimport from replacing local auth_header If you run bulkmarcimport,pl on authorities with --update (or --all) it will overwrite a local auth_header if the auth_id of a local auth_header matches the control-number (field 001) of an imported MARC record. This patch adds a new option --guessauthid (which is still enabled by default to keep the script backwards compatible, but broken), which can turn off this behaviour if called as --noguessauthid. To test with koha-testing-docker: * Inspect cgi-bin/koha/authorities/detail.pl?authid=1402 which should be PERSO_NAME of Aristotles. * get the file twin_040682714.xml from Bugzilla and make it available inside of koha-testing-docker * run: misc/migration_tools/bulkmarcimport.pl -m MARCXML -c utf8 -a --file twin_040682714.xml --all --match isbn,001 * Check cgi-bin/koha/authorities/detail.pl?authid=1402, which now is a TOPIC_TERM "Zwilling" (twin...) Reset koha-testing-docker, apply the patch * run with the new flag misc/migration_tools/bulkmarcimport.pl -m MARCXML -c utf8 -a --file twin_040682714.xml --all --match isbn,001 --noguessauthid * cgi-bin/koha/authorities/detail.pl?authid=1402 still is "Aristotle" * "Zwilling" is cgi-bin/koha/authorities/detail.pl?authid=1708 (at least on my ktd, the actual id might vary) Sponsored-by: Steiermärkische Landesbibliothek Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
It works :)
Well done, it's a nasty bug. (In reply to Katrin Fischer from comment #1) > I would not be opposed to change the behavior here or make it a legacy > switch that needs to be chosen actively. The current behavior appears quite > dangerous. +1 for legacy switch. The current patch is a fix switch. Default is still dangerous.
I believe bug 29440 fixed or altered this - the patch doesn't apply, nor do I recreate the behaviour following the test plan - GuessAuthId is no longer used in the script
Confirmed! I can reproduce on the commit before bug 29440. And I can't with the commits of bug 29440. Is there a worry to have about the broken legacy behavior that is "lost".