[Bug 12343] New: TransformKohaToMarc() is adding MARC subfields in random order
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12343 Bug ID: 12343 Summary: TransformKohaToMarc() is adding MARC subfields in random order Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: gmcharlt@gmail.com Reporter: abl@biblos.pk.edu.pl QA Contact: testopia@bugs.koha-community.org This function iterates through it's argument - which is a hashref - using 'each'. Perl is not guaranteed to return hash keys in any particular order (not to mention that in more recent perl versions, explicit hash key order randomization is to be expected). As a consequence: 1) For biblio records added via acquisition (order from a new/empty record, order from a suggestion), freshly created MARC biblio records doesn't always have 260 $b and 260 $c stored in the proper order 2) Holdings data exported for zebra indexing as 952 fields may have subfields generated in more-or-less random order. While it probably (?) does not affect zebra indexing/searching in any significant way, end result is prone to be somehow ugly (which can be a potential issue e.g. for people running Z39.50 server) and is not guaranteed to be consistent; different records - or even different items in the same record, can have 952 subfields generated in indiscriminate order. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12343 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl --- Comment #1 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- Vaguely remember some earlier discussion about this.. You did not find any references to this in older reports? :) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12343 --- Comment #2 from Jacek Ablewicz <abl@biblos.pk.edu.pl> --- (In reply to M. de Rooy from comment #1)
Vaguely remember some earlier discussion about this.. You did not find any references to this in older reports? :)
I performed several bugzilla searches, but no luck - they do not seem to yield anything relevant. Also, according to http://splitter.koha-community.org/, there are no C4/Biblio.pm patches for this issue currently awaiting in the queue (?). I'm wondering which approach might be best for fixing this: - just sort (or reverse sort) the darn hash keys - that's a simple one; results would be perhaps not particularly well-sorted, but at least they will be sorted consistently ;) - sort by field tags & subfield codes (but this function is apparently pretty sensitive performance-wise ?) - make it to accept arrayref instead of (or in addition to) hashref as an argument, so the subfield order would be preserved - but that would not be all that helpfull for item/holding data subfields which come from the database in the hashref form. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12343 Jacek Ablewicz <ablewicz@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12343 --- Comment #3 from Jacek Ablewicz <ablewicz@gmail.com> --- Created attachment 28631 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=28631&action=edit Bug 12343 - TransformKohaToMarc() is adding MARC subfields in random order TransformKohaToMarc() function in C4/Biblio.pm iterates through it's argument - which is a hashref - using 'each'. Perl is not guaranteed to return hash keys in any particular order (not to mention that in more recent perl versions, explicit hash key order randomization is to be expected). As a consequence: 1) For biblio records added via acquisition (order from a new/empty record, order from a suggestion), freshly created MARC biblio records doesn't always have 260 $b and 260 $c stored in the proper order 2) Holdings data exported for zebra indexing as 952 fields may have subfields generated in more-or-less random order. While it probably (?) does not affect zebra indexing/searching in any significant way, end result is prone to be somehow ugly (which can be a potential issue e.g. for people running Z39.50 server) and is not guaranteed to be consistent; different records - or even different items in the same record, can have 952 subfields generated in indiscriminate order. This patch fixes abovementioned issues via introducting explicit sorting (by subfiled code/letter) for subfield pairs before they are added to the MARC record. To test: 1/ Try to confirm and reproduce both issues (use perl 5.18.1 if possible for more randomly ordered results). 2/ Apply patch. 3/ Redo the tests; ensure that both issues are now fixed and that there are no apparent regressions of any kind (especially regarding to 952 fields generated for zebra [re]indexing). -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12343 Jacek Ablewicz <abl@biblos.pk.edu.pl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |abl@biblos.pk.edu.pl Assignee|gmcharlt@gmail.com |abl@biblos.pk.edu.pl -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12343 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12343 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #28631|0 |1 is obsolete| | --- Comment #4 from Chris Cormack <chris@bigballofwax.co.nz> --- Created attachment 28719 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=28719&action=edit Bug 12343 - TransformKohaToMarc() is adding MARC subfields in random order TransformKohaToMarc() function in C4/Biblio.pm iterates through it's argument - which is a hashref - using 'each'. Perl is not guaranteed to return hash keys in any particular order (not to mention that in more recent perl versions, explicit hash key order randomization is to be expected). As a consequence: 1) For biblio records added via acquisition (order from a new/empty record, order from a suggestion), freshly created MARC biblio records doesn't always have 260 $b and 260 $c stored in the proper order 2) Holdings data exported for zebra indexing as 952 fields may have subfields generated in more-or-less random order. While it probably (?) does not affect zebra indexing/searching in any significant way, end result is prone to be somehow ugly (which can be a potential issue e.g. for people running Z39.50 server) and is not guaranteed to be consistent; different records - or even different items in the same record, can have 952 subfields generated in indiscriminate order. This patch fixes abovementioned issues via introducting explicit sorting (by subfiled code/letter) for subfield pairs before they are added to the MARC record. To test: 1/ Try to confirm and reproduce both issues (use perl 5.18.1 if possible for more randomly ordered results). 2/ Apply patch. 3/ Redo the tests; ensure that both issues are now fixed and that there are no apparent regressions of any kind (especially regarding to 952 fields generated for zebra [re]indexing). Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12343 --- Comment #5 from Jonathan Druart <jonathan.druart@biblibre.com> --- Comment on attachment 28719 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=28719 Bug 12343 - TransformKohaToMarc() is adding MARC subfields in random order Review of attachment 28719: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=12343&attachment=28719) ----------------------------------------------------------------- ::: C4/Biblio.pm @@ +2194,4 @@
while ( my ($name, $value) = each %$hash ) { next unless my $dtm = $db_to_marc->{''}->{$name}; next unless ( scalar( @$dtm ) ); + my ($tag, $letter) = @$dtm; $tag .= '';
Please prefer 1 instruction by line. @@ +2198,2 @@
foreach my $value ( split(/\s?\|\s?/, $value, -1) ) { + $value eq '' && next;
Please prefer next if $value eq ''; of if ( $value eq '' ) { next; } -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12343 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12343 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #28719|0 |1 is obsolete| | --- Comment #6 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 28776 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=28776&action=edit Bug 12343 - TransformKohaToMarc() is adding MARC subfields in random order TransformKohaToMarc() function in C4/Biblio.pm iterates through it's argument - which is a hashref - using 'each'. Perl is not guaranteed to return hash keys in any particular order (not to mention that in more recent perl versions, explicit hash key order randomization is to be expected). As a consequence: 1) For biblio records added via acquisition (order from a new/empty record, order from a suggestion), freshly created MARC biblio records doesn't always have 260 $b and 260 $c stored in the proper order 2) Holdings data exported for zebra indexing as 952 fields may have subfields generated in more-or-less random order. While it probably (?) does not affect zebra indexing/searching in any significant way, end result is prone to be somehow ugly (which can be a potential issue e.g. for people running Z39.50 server) and is not guaranteed to be consistent; different records - or even different items in the same record, can have 952 subfields generated in indiscriminate order. This patch fixes abovementioned issues via introducting explicit sorting (by subfiled code/letter) for subfield pairs before they are added to the MARC record. To test: 1/ Try to confirm and reproduce both issues (use perl 5.18.1 if possible for more randomly ordered results). 2/ Apply patch. 3/ Redo the tests; ensure that both issues are now fixed and that there are no apparent regressions of any kind (especially regarding to 952 fields generated for zebra [re]indexing). Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12343 --- Comment #7 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 28777 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=28777&action=edit Bug 12343: Add regression test for TransformKohaToMarc Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12343 --- Comment #8 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 28778 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=28778&action=edit Bug 12343: QA follow-up Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12343 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |gmcharlt@gmail.com --- Comment #9 from Galen Charlton <gmcharlt@gmail.com> --- One thing to note regarding subfield order: in the general case, one cannot count on alphabetical order being the correct order for subfields in a field. For example, the MARC21 246 field puts $i before $a: 246 0# $iAdded title page title on some issues:$aAnnual report However, in the specific case, since TransformKohaToMarc is used only for creating brief records in acquisitions and embedding item fields in the bib record, this isn't a blocker IMO, but should be noted if TransformKohaToMarc gets used more generally in the future. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12343 Yohann Dufour <yohann.dufour@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=12409 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12343 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master CC| |tomascohen@gmail.com --- Comment #10 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Patches pushed to master. Thanks Jacek! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12343 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |Pushed to Stable --- Comment #11 from Galen Charlton <gmcharlt@gmail.com> --- Pushed to 3.16.x for inclusion in 3.16.1. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org