[Bug 10961] New: Error in GetMarcBiblio can cause severe data loss
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10961 Bug ID: 10961 Summary: Error in GetMarcBiblio can cause severe data loss Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: critical Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: gmcharlt@gmail.com Reporter: kyle@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org A bug in GetMarcBiblio can cause severe data loss if your database has records where the biblionumber and biblioitemnumber do not match and the script misc/batchRebuildBiblioTables.pl is run. The Biblio::GetMarcBiblio makes a kall to C4::Biblio::_koha_marc_update_bib_ids which passes the biblionumber as both the biblionumber *and the biblioitemnumber*. Thus, if your biblio and biblioitem numbers are not always equal, you will end up with a record where the biblioitemnumber is incorrect in the record! This is usually not a severe issue, but since batchRebuildBiblioTables uses that record to update the database tables, it ends up updating the wrong biblioitem row! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10961 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10961 --- Comment #1 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 21520 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=21520&action=edit Bug 10961 - Error in GetMarcBiblio can cause severe data loss A bug in GetMarcBiblio can cause severe data loss if your database has records where the biblionumber and biblioitemnumber do not match and the script misc/batchRebuildBiblioTables.pl is run. The Biblio::GetMarcBiblio makes a kall to C4::Biblio::_koha_marc_update_bib_ids which passes the biblionumber as both the biblionumber *and the biblioitemnumber*. Thus, if your biblio and biblioitem numbers are not always equal, you will end up with a record where the biblioitemnumber is incorrect in the record! This is usually not a severe issue, but since batchRebuildBiblioTables uses that record to update the database tables, it ends up updating the wrong biblioitem row! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10961 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Trivial patch Assignee|gmcharlt@gmail.com |kyle@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10961 --- Comment #2 from Galen Charlton <gmcharlt@gmail.com> --- Comment on attachment 21520 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=21520 Bug 10961 - Error in GetMarcBiblio can cause severe data loss Review of attachment 21520: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=10961&attachment=21520) ----------------------------------------------------------------- ::: C4/Biblio.pm @@ +1252,4 @@
my $biblionumber = shift; my $embeditems = shift || 0; my $dbh = C4::Context->dbh; + my $sth = $dbh->prepare("SELECT * FROM biblioitems WHERE biblionumber=? ");
You don't need * here, just "marcxml, biblioitemnumber". Since biblioitems also contains the marc column, select * just causes unnecessary network traffic for remote DB servers. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10961 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |gmcharlt@gmail.com --- Comment #3 from Galen Charlton <gmcharlt@gmail.com> --- Also, as with any change to a routine in the C4 or Koha namespace, please provide a regression test. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10961 --- Comment #4 from Galen Charlton <gmcharlt@gmail.com> --- Also, while the bug in GetMarcBiblio() is real, consideration should be given to removing misc/batchRebuildBiblioTables.pl for the following reasons: [1] It calls internal routines in C4::Biblio directly in the name of performance, but doing so is the direct cause of the bug. [2] misc/maintenance/touch_all_biblios.pl will do the same thing, but does not call private routines in C4::Biblio. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10961 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA CC| |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=10961 --- Comment #5 from stephane.delaune@biblibre.com --- Created attachment 41130 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=41130&action=edit Bug 10961 - (Followup) Error in GetMarcBiblio can cause severe data loss add correct frameworkcode to _koha_marc_update_bib_ids parameters add test, prove with : prove t/db_dependent/Biblio.t -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10961 stephane.delaune@biblibre.com changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |stephane.delaune@biblibre.c | |om Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10961 --- Comment #6 from stephane.delaune@biblibre.com --- *** Bug 14596 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10961 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10961 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #21520|0 |1 is obsolete| | --- Comment #7 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 41182 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=41182&action=edit Bug 10961 - Error in GetMarcBiblio can cause severe data loss A bug in GetMarcBiblio can cause severe data loss if your database has records where the biblionumber and biblioitemnumber do not match and the script misc/batchRebuildBiblioTables.pl is run. The Biblio::GetMarcBiblio makes a kall to C4::Biblio::_koha_marc_update_bib_ids which passes the biblionumber as both the biblionumber *and the biblioitemnumber*. Thus, if your biblio and biblioitem numbers are not always equal, you will end up with a record where the biblioitemnumber is incorrect in the record! This is usually not a severe issue, but since batchRebuildBiblioTables uses that record to update the database tables, it ends up updating the wrong biblioitem row! NOTE: What a horrible, horrible typo that was. Tested this with the second patch. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10961 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #41130|0 |1 is obsolete| | --- Comment #8 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 41183 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=41183&action=edit Bug 10961 - (Followup) Error in GetMarcBiblio can cause severe data loss add correct frameworkcode to _koha_marc_update_bib_ids parameters add test, prove with : prove t/db_dependent/Biblio.t TEST PLAN --------- 1) git checkout -b bug_10961 origin/master 2) git bz apply 10961 3) git checkout origin/master -- C4/Biblio.pm 4) prove t/db_dependent/Biblio.t -- was expecting failure, got failure. 5) git reset --hard origin/master 6) git bz apply 10961 7) prove t/db_dependent/Biblio.t -- success as expected. 8) Read over code. -- Noted it also grabs the framework code for the biblio, rather than uses default. And it also corrects an indentation issue. Test case looks like it attempts to cover the biblionumber!=biblioitemnumber case by adding 1. 9) run koha qa test tools. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10961 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mtompset@hotmail.com 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=10961 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #41182|0 |1 is obsolete| | --- Comment #9 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 41214 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=41214&action=edit [PASSED QA] Bug 10961: Error in GetMarcBiblio can cause severe data loss A bug in GetMarcBiblio can cause severe data loss if your database has records where the biblionumber and biblioitemnumber do not match and the script misc/batchRebuildBiblioTables.pl is run. The Biblio::GetMarcBiblio makes a kall to C4::Biblio::_koha_marc_update_bib_ids which passes the biblionumber as both the biblionumber *and the biblioitemnumber*. Thus, if your biblio and biblioitem numbers are not always equal, you will end up with a record where the biblioitemnumber is incorrect in the record! This is usually not a severe issue, but since batchRebuildBiblioTables uses that record to update the database tables, it ends up updating the wrong biblioitem row! NOTE: What a horrible, horrible typo that was. Tested this with the second patch. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10961 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #41183|0 |1 is obsolete| | --- Comment #10 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 41215 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=41215&action=edit [PASSED QA] Bug 10961: (followup) Make query fields explicit and add regression tests add correct frameworkcode to _koha_marc_update_bib_ids parameters add test, prove with : prove t/db_dependent/Biblio.t TEST PLAN --------- 1) git checkout -b bug_10961 origin/master 2) git bz apply 10961 3) git checkout origin/master -- C4/Biblio.pm 4) prove t/db_dependent/Biblio.t -- was expecting failure, got failure. 5) git reset --hard origin/master 6) git bz apply 10961 7) prove t/db_dependent/Biblio.t -- success as expected. 8) Read over code. -- Noted it also grabs the framework code for the biblio, rather than uses default. And it also corrects an indentation issue. Test case looks like it attempts to cover the biblionumber!=biblioitemnumber case by adding 1. 9) run koha qa test tools. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10961 Tomás Cohen Arazi <tomascohen@gmail.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=10961 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #11 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Patches pushed to master. Thanks Kyle and Stephane! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10961 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |Pushed to Stable --- Comment #12 from Chris Cormack <chris@bigballofwax.co.nz> --- Pushed to 3.20.x will be in 3.20.3 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10961 Liz Rea <wizzyrea@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |wizzyrea@gmail.com --- Comment #13 from Liz Rea <wizzyrea@gmail.com> --- Pushed to 3.18.x, will be in 3.18.10. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10961 Mason James <mtj@kohaaloha.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mtj@kohaaloha.com --- Comment #14 from Mason James <mtj@kohaaloha.com> --- Pushed to 3.16.x, will be in 3.16.14 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org