[Bug 17885] New: Koha::SearchEngine::Elasticsearch-> reset_elasticsearch_mappings throws DBD::mysql Duplicate entry exceptions
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17885 Bug ID: 17885 Summary: Koha::SearchEngine::Elasticsearch->reset_elasticsearch _mappings throws DBD::mysql Duplicate entry exceptions Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Searching Assignee: gmcharlt@gmail.com Reporter: olli-antti.kivilahti@jns.fi QA Contact: testopia@bugs.koha-community.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17885 --- Comment #1 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- Created attachment 58825 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58825&action=edit Bug 17885 - Koha::SearchEngine::Elasticsearch->reset_elasticsearch_mappings throws DBD::mysql Duplicate entry exceptions When executing Koha::SearchEngine::Elasticsearch->reset_elasticsearch_mappings() against populated search engine mappings tables, one gets this exception: DBD::mysql::st execute failed: Duplicate entry '388-73' for key 'PRIMARY' [for Statement \"INSERT INTO `search_marc_to_field` ( `facet`, `search_field_id`, `search_marc_map_id`, `sort`, `suggestible`) VALUES ( ?, ?, ?, ?, ? )\" with ParamValues: 0='', 1='73', 2='388', 3=undef, 4=''] at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1834.\nDBIx::Class::Storage::DBI::_dbh_execute(): Duplicate entry '388-73' for key 'PRIMARY' at /home/koha/Koha/Koha/SearchField.pm line 38" This patch fixes this, by upserting those entries instead of adding them. Also adds a Koha::SearchMappingManager to manage the complexities of CRUD:ing these search mappings Also adds nice tests to make sure the Manager and the reset_elasticsearch_mappings() keep working as expected. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17885 Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |17255 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17255 [Bug 17255] Upgrade Elastic Search code to work with version 5.1 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17885 Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17885 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org --- Comment #2 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- What are the steps to recreate? select count(*) from search_marc_to_field; 476 Hit /cgi-bin/koha/admin/searchengine/elasticsearch/mappings.pl?op=reset&i_know_what_i_am_doing=1 => no error. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17885 --- Comment #3 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- Hi. You can recreate the problem with this: perl -e 'use Koha::SearchEngine::Elasticsearch; Koha::SearchEngine::Elasticsearch->reset_elasticsearch_mappings' Your solution looks very simple :) Unfortunately there is a bunch of business logic clumped together in the /admin/searchengine/elasticsearch/mappings.pl And I don't think business logic belongs to a controller module. Simply because that functionality is needed when writing tests. When writing tests it is better to call that one subroutine reset_elasticsearch_mappings instead of the bunch of code in mappings.pl If a subroutine says reset_elasticsearch_mappings(), I expect it to reset the elasticsearch mappings without errors. A simple solution would be to move: Koha::SearchMarcMaps->search->delete; inside Koha::SearchEngine::Elasticsearch->reset_elasticsearch_mappings; which might be a better one than the DBIC-monster I created. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17885 --- Comment #4 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- My modification preserves existing changes. The way it works in the mappings.pl, is to drop all changes and add the defaults. I guess the original one is the desired behaviour. But I strongly feel that the reset_elasticsearch_mappings()-subroutine needs to perform on its own. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17885 --- Comment #5 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Ok got it, so I guess it would be better to simply move the delete inside the reset_elasticsearch_mappings. And move the refactoring to another bug report. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17885 --- Comment #6 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- Any suggestions for the title of the new refactorings bug? It is all closely related to this if you take a loot at the tests. Refactorings have been done so I can test this change. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17885 --- Comment #7 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- I am not sure that separating them to separate bugs is the correct solution here. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17885 Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #58825|0 |1 is obsolete| | --- Comment #8 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- Created attachment 58940 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58940&action=edit Bug 17885 - Koha::SearchEngine::Elasticsearch->reset_elasticsearch_mappings throws DBD::mysql Duplicate entry exceptions When executing Koha::SearchEngine::Elasticsearch->reset_elasticsearch_mappings() against populated search engine mappings tables, one gets this exception: DBD::mysql::st execute failed: Duplicate entry '388-73' for key 'PRIMARY' [for Statement \"INSERT INTO `search_marc_to_field` ( `facet`, `search_field_id`, `search_marc_map_id`, `sort`, `suggestible`) VALUES ( ?, ?, ?, ?, ? )\" with ParamValues: 0='', 1='73', 2='388', 3=undef, 4=''] at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1834.\nDBIx::Class::Storage::DBI::_dbh_execute(): Duplicate entry '388-73' for key 'PRIMARY' at /home/koha/Koha/Koha/SearchField.pm line 38" This patch fixes this, by first deleting all entries, then adding the default ones. If a subroutine says reset_elasticsearch_mappings(), I expect it to reset the elasticsearch mappings without errors. When writing tests it is better to call that one subroutine reset_elasticsearch_mappings instead of the bunch of code in admin/searchengine/elasticsearch/mappings.pl?op=reset&i_know_what_i_am_doing=1 Also this promotes code reuse. TEST PLAN: a. perl -e 'use Koha::SearchEngine::Elasticsearch; Koha::SearchEngine::Elasticsearch->reset_elasticsearch_mappings' b. Run misc/devel/populate_db.pl on an already populated DB and you eventually face this problem. After this patch, step a. works. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17885 Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #58940|0 |1 is obsolete| | --- Comment #9 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- Created attachment 58941 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58941&action=edit Bug 17885 - Koha::SearchEngine::Elasticsearch->reset_elasticsearch_mappings throws DBD::mysql Duplicate entry exceptions When executing Koha::SearchEngine::Elasticsearch->reset_elasticsearch_mappings() against populated search engine mappings tables, one gets this exception: DBD::mysql::st execute failed: Duplicate entry '388-73' for key 'PRIMARY' [for Statement \"INSERT INTO `search_marc_to_field` ( `facet`, `search_field_id`, `search_marc_map_id`, `sort`, `suggestible`) VALUES ( ?, ?, ?, ?, ? )\" with ParamValues: 0='', 1='73', 2='388', 3=undef, 4=''] at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1834.\nDBIx::Class::Storage::DBI::_dbh_execute(): Duplicate entry '388-73' for key 'PRIMARY' at /home/koha/Koha/Koha/SearchField.pm line 38" This patch fixes this, by first deleting all entries, then adding the default ones. If a subroutine says reset_elasticsearch_mappings(), I expect it to reset the elasticsearch mappings without errors. When writing tests it is better to call that one subroutine reset_elasticsearch_mappings instead of the bunch of code in admin/searchengine/elasticsearch/mappings.pl?op=reset&i_know_what_i_am_doing=1 Also this promotes code reuse. TEST PLAN: a. perl -e 'use Koha::SearchEngine::Elasticsearch; Koha::SearchEngine::Elasticsearch->reset_elasticsearch_mappings' b. Run misc/devel/populate_db.pl on an already populated DB and you eventually face this problem. After this patch, step a. works. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17885 Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |17897 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17897 [Bug 17897] Koha::SearchMappingManager - Simplify search mapping modifications -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17885 Zoe Schoeler <crazy.mental.onion@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|gmcharlt@gmail.com |crazy.mental.onion@gmail.co | |m CC| |crazy.mental.onion@gmail.co | |m -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17885 Bug 17885 depends on bug 17255, which changed state. Bug 17255 Summary: Upgrade Elastic Search code to work with version 5.1 https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17255 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17885 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit | |y.org | Component|Searching |Searching - Elasticsearch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17885 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|crazy.mental.onion@gmail.co |olli-antti.kivilahti@jns.fi |m | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17885 David Bourgault <david.bourgault@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17885 David Bourgault <david.bourgault@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #58941|0 |1 is obsolete| | --- Comment #10 from David Bourgault <david.bourgault@inlibro.com> --- Created attachment 69338 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=69338&action=edit Bug 17885 - Koha::SearchEngine::Elasticsearch->reset_elasticsearch_mappings throws DBD::mysql Duplicate entry exceptions TEST PLAN: a. perl -e 'use Koha::SearchEngine::Elasticsearch; Koha::SearchEngine::Elasticsearch->reset_elasticsearch_mappings' b. Run misc/devel/populate_db.pl on an already populated DB and you eventually face this problem. After this patch, step a. works. Signed-off-by: David Bourgault <david.bourgault@inlibro.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17885 Alex Arnaud <alex.arnaud@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact| |alex.arnaud@biblibre.com CC| |alex.arnaud@biblibre.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17885 --- Comment #11 from Alex Arnaud <alex.arnaud@biblibre.com> --- Hello Oli, Could you please add unit tests for sub reset_elasticsearch_mappings? At least to check that it deletes mappings as it is what you change in the behavior. But more tests are welcome. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17885 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@bywatersolutions.com --- Comment #12 from Nick Clemens <nick@bywatersolutions.com> --- Should be QAed together with 17897 which contains the tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17885 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de Status|Signed Off |BLOCKED --- Comment #13 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Thx Nick for taking a look. Moving to BLOCKED for now, move back to Signed off once dependency is resolved please. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17885 Joonas Kylmälä <joonas.kylmala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #69338|0 |1 is obsolete| | CC| |joonas.kylmala@helsinki.fi --- Comment #14 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Created attachment 78327 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=78327&action=edit Bug 17885: Koha::SearchEngine::Elasticsearch->reset_elasticsearch_mappings throws DBD::mysql Duplicate entry exceptions I modified the patch to work with the recent Elasticsearch changes. Please sign-off if you think it is fine. I will check up next the status of bug #17897. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17885 Joonas Kylmälä <joonas.kylmala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|BLOCKED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17885 Johanna Räisä <johanna.raisa@koha-suomi.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #78327|0 |1 is obsolete| | CC| |johanna.raisa@koha-suomi.fi --- Comment #15 from Johanna Räisä <johanna.raisa@koha-suomi.fi> --- Created attachment 91308 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=91308&action=edit Bug 17885: Koha::SearchEngine::Elasticsearch->reset_elasticsearch_mappings throws DBD::mysql Duplicate entry exceptions When executing Koha::SearchEngine::Elasticsearch->reset_elasticsearch_mappings() against populated search engine mappings tables, one gets this exception: DBD::mysql::st execute failed: Duplicate entry '388-73' for key 'PRIMARY' [for Statement \"INSERT INTO `search_marc_to_field` ( `facet`, `search_field_id`, `search_marc_map_id`, `sort`, `suggestible`) VALUES ( ?, ?, ?, ?, ? )\" with ParamValues: 0='', 1='73', 2='388', 3=undef, 4=''] at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1834.\nDBIx::Class::Storage::DBI::_dbh_execute(): Duplicate entry '388-73' for key 'PRIMARY' at /home/koha/Koha/Koha/SearchField.pm line 38" This patch fixes this, by first deleting all entries, then adding the default ones. If a subroutine says reset_elasticsearch_mappings(), I expect it to reset the elasticsearch mappings without errors. When writing tests it is better to call that one subroutine reset_elasticsearch_mappings instead of the bunch of code in admin/searchengine/elasticsearch/mappings.pl?op=reset_confirmed Also this promotes code reuse. TEST PLAN: a. perl -e 'use Koha::SearchEngine::Elasticsearch; Koha::SearchEngine::Elasticsearch->reset_elasticsearch_mappings' b. Run misc/devel/populate_db.pl on an already populated DB and you eventually face this problem. After this patch, step a. works. This patch is adapted from the original patch made by Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>. Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17885 Johanna Räisä <johanna.raisa@koha-suomi.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17885 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |normal CC| |martin.renvoize@ptfs-europe | |.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17885 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17885 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #91308|0 |1 is obsolete| | --- Comment #16 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 97198 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=97198&action=edit Bug 17885: Koha::SearchEngine::Elasticsearch->reset_elasticsearch_mappings throws DBD::mysql Duplicate entry exceptions When executing Koha::SearchEngine::Elasticsearch->reset_elasticsearch_mappings() against populated search engine mappings tables, one gets this exception: DBD::mysql::st execute failed: Duplicate entry '388-73' for key 'PRIMARY' [for Statement \"INSERT INTO `search_marc_to_field` ( `facet`, `search_field_id`, `search_marc_map_id`, `sort`, `suggestible`) VALUES ( ?, ?, ?, ?, ? )\" with ParamValues: 0='', 1='73', 2='388', 3=undef, 4=''] at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1834.\nDBIx::Class::Storage::DBI::_dbh_execute(): Duplicate entry '388-73' for key 'PRIMARY' at /home/koha/Koha/Koha/SearchField.pm line 38" This patch fixes this, by first deleting all entries, then adding the default ones. If a subroutine says reset_elasticsearch_mappings(), I expect it to reset the elasticsearch mappings without errors. When writing tests it is better to call that one subroutine reset_elasticsearch_mappings instead of the bunch of code in admin/searchengine/elasticsearch/mappings.pl?op=reset_confirmed Also this promotes code reuse. TEST PLAN: a. perl -e 'use Koha::SearchEngine::Elasticsearch; Koha::SearchEngine::Elasticsearch->reset_elasticsearch_mappings' b. Run misc/devel/populate_db.pl on an already populated DB and you eventually face this problem. After this patch, step a. works. This patch is adapted from the original patch made by Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>. Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Test plan is wrong, but changes make sense -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17885 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |20.05.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17885 --- Comment #17 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Nice work everyone! Pushed to master for 20.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17885 Joy Nelson <joy@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |Pushed to stable CC| |joy@bywatersolutions.com Version(s)|20.05.00 |20.05.00, 19.11.03 released in| | --- Comment #18 from Joy Nelson <joy@bywatersolutions.com> --- Pushed to 19.11.x branch for 19.11.03 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17885 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com Status|Pushed to stable |Pushed to oldstable Version(s)|20.05.00, 19.11.03 |20.05.00, 19.11.03, released in| |19.05.08 --- Comment #19 from Lucas Gass <lucas@bywatersolutions.com> --- backported to 19.05.x for 19.05.08 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org