Created attachment 54910 [details] [review] Bug 17196: Move marcxml out of the biblioitems table Two discussions on koha-devel lead to the same conclusion: biblioitems.marcxml should be moved out this table - biblio and biblioitems http://lists.koha-community.org/pipermail/koha-devel/2013-April/039239.html - biblioitems.marcxml & biblioitems.marc / HUGE performance issue ! http://lists.koha-community.org/pipermail/koha-devel/2016-July/042821.html There are several goals to do it: - Performance As Paul Poulain wrote, a simple query like SELECT publicationyear, count(publicationyear) FROM biblioitems GROUP BY publicationyear; takes more than 10min on a DB with more than 1M bibliographic records but only 3sec (!) on the same DB without the biblioitems.marcxml field Note that priori to this patch set, the biblioitems.marcxml was not retrieved systematically, but was, at least, in C4::Acquisition::GetOrdersByBiblionumber and C4::Acquisition::GetOrders - Flexibility Storing the marcxml in a specific table would allow use to store several kind of metadata (USMARC, MARCXML, MIJ, etc.) and different formats (marcflavour) - Clean code It would be a first step toward Koha::MetadataRecord for bibliographic records (not done in this patch set). Test plan: - Update the DBIC Schema - Add / Edit / Delete / Import / Export bibliographic records - Add items - Reindex records using ES - Confirm that the following scripts still work: * misc/cronjobs/delete_records_via_leader.pl * misc/migration_tools/build_oai_sets.pl - Look at the reading history at the OPAC (opac-readingrecord.pl) - At the OPAC, click on a tag, you must see the result Note: Changes in Koha/OAI/Server/ListRecords.pm is planned on bug 15108.
Created attachment 54911 [details] [review] Bug 17196: Fix query builder for item search The item search needs to join on biblio_metadata to allow search on marcxml field Test plan: Launch complex item searches (using marc fields).
Created attachment 54912 [details] [review] Bug 17196: Update misc/maintenance/sanitize_records.pl Test plan: Use the sanitize_records.pl maintenance script with the --auto-search option
Created attachment 54913 [details] [review] Bug 17196: Update switch_marc21_series_info.pl Not sure if this script is still used, could someone confirm? Test plan: If you know how to test it, please do
Created attachment 54914 [details] [review] Bug 17196: DB Changes Remove biblioitems.marcxml and deletedbiblioitems.marcxml and adds 2 table biblio_metadata and deletedbiblio_metadata
Created attachment 54918 [details] [review] Bug 17196: DB Changes Remove biblioitems.marcxml and deletedbiblioitems.marcxml and adds 2 table biblio_metadata and deletedbiblio_metadata
Hi Jonathan I attempted to test this patchset, but I had a big problem trying to 'git bz apply' the 1st patch, on master (0254fd7) 0254fd7 Bug 17135 - DBRev 16.06.00.028 would you take another look at the patchset please? --------------- mason@xen1:~/g/k/Koha$ git bz apply 17196 Bug 17196 - Move marcxml out of the biblioitems table 54910 - Bug 17196: Move marcxml out of the biblioitems table 54911 - Bug 17196: Fix query builder for item search 54912 - Bug 17196: Update misc/maintenance/sanitize_records.pl 54913 - Bug 17196: Update switch_marc21_series_info.pl 54918 - Bug 17196: DB Changes Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 17196: Move marcxml out of the biblioitems table fatal: sha1 information is lacking or useless (C4/Biblio.pm). Repository lacks necessary blobs to fall back on 3-way merge. Cannot fall back to three-way merge. Patch failed at 0001 Bug 17196: Move marcxml out of the biblioitems table The copy of the patch that failed is found in: /home/mason/g/k/Koha/.git/rebase-apply/patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-17196-Move-marcxml-out-of-the-biblioitems-tabl-jCnYTS.patch
(In reply to Mason James from comment #7) > Hi Jonathan > > I attempted to test this patchset, but I had a big problem trying to 'git bz > apply' the 1st patch, on master (0254fd7) > > 0254fd7 Bug 17135 - DBRev 16.06.00.028 > > would you take another look at the patchset please? Everything is applying ok for me, please check the dependencies.
> > I attempted to test this patchset, but I had a big problem trying to 'git bz > > apply' the 1st patch, on master (0254fd7) git bz didn't apply this successfully when using the auto-inclusion of dependent patches, but I don't know why. Applying the patches manually works without error.
(In reply to Jonathan Druart from comment #8) > (In reply to Mason James from comment #7) > > Hi Jonathan > > > > I attempted to test this patchset, but I had a big problem trying to 'git bz > > apply' the 1st patch, on master (0254fd7) > > > > 0254fd7 Bug 17135 - DBRev 16.06.00.028 > > > > would you take another look at the patchset please? > > Everything is applying ok for me, please check the dependencies. afaik (In reply to Jonathan Druart from comment #8) > (In reply to Mason James from comment #7) > > Hi Jonathan > > > > I attempted to test this patchset, but I had a big problem trying to 'git bz > > apply' the 1st patch, on master (0254fd7) > > > > 0254fd7 Bug 17135 - DBRev 16.06.00.028 > > > > would you take another look at the patchset please? > > Everything is applying ok for me, please check the dependencies. i have all listed dependencies applied on commit 56da7be (10455 17193 17197) the patch still fails for me
(In reply to Mason James from comment #10) > (In reply to Jonathan Druart from comment #8) > > (In reply to Mason James from comment #7) > > > Hi Jonathan > > > > > > I attempted to test this patchset, but I had a big problem trying to 'git bz > > > apply' the 1st patch, on master (0254fd7) > > > > > > 0254fd7 Bug 17135 - DBRev 16.06.00.028 > > > > > > would you take another look at the patchset please? > > > > Everything is applying ok for me, please check the dependencies. > > afaik (In reply to Jonathan Druart from comment #8) > > (In reply to Mason James from comment #7) > > > Hi Jonathan > > > > > > I attempted to test this patchset, but I had a big problem trying to 'git bz > > > apply' the 1st patch, on master (0254fd7) > > > > > > 0254fd7 Bug 17135 - DBRev 16.06.00.028 > > > > > > would you take another look at the patchset please? > > > > Everything is applying ok for me, please check the dependencies. > > i have all listed dependencies applied on commit 56da7be (10455 17193 17197) > > the patch still fails for me here's the transcript... mason@xen1:~/g/k/Koha$ git log --oneline' 56da7be Bug 17352 - Patron search type is hard coded to 'contain' in circ/circulation.pl 5eb4b06 Bug 16273 - DBRev 16.06.00.031 3d4dc58 Bug 16273: Add the ability not to display the login details for newly registered patrons 8d0b412 Bug 16273: Add the new pref PatronSelfRegistrationPrefillForm d85e80b Bug 17301: Follow-up - Standardize headings mason@xen1:~/g/k/Koha$ git bz apply 10455 Bug 10455 - remove redundant 'biblioitems.marc' field 55844 - Bug 10455: Remove biblioitems.marc - DB changes 55845 - Bug 10455: Remove biblioitems.marc 55846 - Bug 10455: Remove biblioitems.marc - rm MARC21_parse_test.pl 55847 - Bug 10455: Remove biblioitems.marc - update delete_records_via_leader.pl 55848 - Bug 10455: (follow-up) Remove biblioitems.marc 55849 - Bug 10455: (follow-up 2) Remove biblioitems.marc 55850 - Bug 10455: (follow-up 3) Remove biblioitems.marc 55851 - Bug 10455: Remove MARC21_utf8_flag_fix.pl 55852 - Bug 10455: Fix tests 55853 - Bug 10455 (QA Followup) Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 10455: Remove biblioitems.marc - DB changes Applying: Bug 10455: Remove biblioitems.marc Applying: Bug 10455: Remove biblioitems.marc - rm MARC21_parse_test.pl Applying: Bug 10455: Remove biblioitems.marc - update delete_records_via_leader.pl Applying: Bug 10455: (follow-up) Remove biblioitems.marc Applying: Bug 10455: (follow-up 2) Remove biblioitems.marc Applying: Bug 10455: (follow-up 3) Remove biblioitems.marc Applying: Bug 10455: Remove MARC21_utf8_flag_fix.pl Applying: Bug 10455: Fix tests Applying: Bug 10455 (QA Followup) mason@xen1:~/g/k/Koha$ mason@xen1:~/g/k/Koha$ git bz apply 17193 Bug 17193 - C4::Search::SearchAcquisitions is not used 54962 - [PASSED QA] Bug 17193: Remove C4::Search::SearchAcquisitions Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 17193: Remove C4::Search::SearchAcquisitions Using index info to reconstruct a base tree... M C4/Search.pm Falling back to patching base and 3-way merge... No changes -- Patch already applied. mason@xen1:~/g/k/Koha$ git bz apply 17197 Bug 17197 - misc/batchupdateISBNs.pl is certainly no longer in use 55478 - [PASSED QA] Bug 17197: Remove misc/batchupdateISBNs.pl Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 17197: Remove misc/batchupdateISBNs.pl Using index info to reconstruct a base tree... A misc/batchupdateISBNs.pl Falling back to patching base and 3-way merge... No changes -- Patch already applied. mason@xen1:~/g/k/Koha$ mason@xen1:~/g/k/Koha$ git log --oneline' 4f6b1ee Bug 10455 (QA Followup) 98af430 Bug 10455: Fix tests a5e6f98 Bug 10455: Remove MARC21_utf8_flag_fix.pl fbb280d Bug 10455: (follow-up 3) Remove biblioitems.marc 81a0fb2 Bug 10455: (follow-up 2) Remove biblioitems.marc 4e3520d Bug 10455: (follow-up) Remove biblioitems.marc 339d2bc Bug 10455: Remove biblioitems.marc - update delete_records_via_leader.pl d572063 Bug 10455: Remove biblioitems.marc - rm MARC21_parse_test.pl dccc79a Bug 10455: Remove biblioitems.marc 558f6b1 Bug 10455: Remove biblioitems.marc - DB changes 56da7be Bug 17352 - Patron search type is hard coded to 'contain' in circ/circulation.pl 5eb4b06 Bug 16273 - DBRev 16.06.00.031 mason@xen1:~/g/k/Koha$ git bz apply 17196 Bug 17196 - Move marcxml out of the biblioitems table 54910 - Bug 17196: Move marcxml out of the biblioitems table 54911 - Bug 17196: Fix query builder for item search 54912 - Bug 17196: Update misc/maintenance/sanitize_records.pl 54913 - Bug 17196: Update switch_marc21_series_info.pl 54918 - Bug 17196: DB Changes Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 17196: Move marcxml out of the biblioitems table fatal: sha1 information is lacking or useless (C4/Biblio.pm). Repository lacks necessary blobs to fall back on 3-way merge. Cannot fall back to three-way merge. Patch failed at 0001 Bug 17196: Move marcxml out of the biblioitems table The copy of the patch that failed is found in: /home/mason/g/k/Koha/.git/rebase-apply/patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-17196-Move-marcxml-out-of-the-biblioitems-tabl-HBdRv1.patch mason@xen1:~/g/k/Koha$ git apply /tmp/Bug-17196-Move-marcxml-out-of-the-biblioitems-tabl-HBdRv1.patch error: patch failed: C4/Biblio.pm:42 error: C4/Biblio.pm: patch does not apply error: patch failed: C4/ILSDI/Services.pm:406 error: C4/ILSDI/Services.pm: patch does not apply error: patch failed: misc/cronjobs/delete_records_via_leader.pl:68 error: misc/cronjobs/delete_records_via_leader.pl: patch does not apply
(In reply to Owen Leonard from comment #9) > > > I attempted to test this patchset, but I had a big problem trying to 'git bz > > > apply' the 1st patch, on master (0254fd7) > > git bz didn't apply this successfully when using the auto-inclusion of > dependent patches, but I don't know why. Applying the patches manually works > without error. hi Owen, would you paste your command workflow to the ticket, please?
Created attachment 55856 [details] [review] Bug 17196: Move marcxml out of the biblioitems table Two discussions on koha-devel lead to the same conclusion: biblioitems.marcxml should be moved out this table - biblio and biblioitems http://lists.koha-community.org/pipermail/koha-devel/2013-April/039239.html - biblioitems.marcxml & biblioitems.marc / HUGE performance issue ! http://lists.koha-community.org/pipermail/koha-devel/2016-July/042821.html There are several goals to do it: - Performance As Paul Poulain wrote, a simple query like SELECT publicationyear, count(publicationyear) FROM biblioitems GROUP BY publicationyear; takes more than 10min on a DB with more than 1M bibliographic records but only 3sec (!) on the same DB without the biblioitems.marcxml field Note that priori to this patch set, the biblioitems.marcxml was not retrieved systematically, but was, at least, in C4::Acquisition::GetOrdersByBiblionumber and C4::Acquisition::GetOrders - Flexibility Storing the marcxml in a specific table would allow use to store several kind of metadata (USMARC, MARCXML, MIJ, etc.) and different formats (marcflavour) - Clean code It would be a first step toward Koha::MetadataRecord for bibliographic records (not done in this patch set). Test plan: - Update the DBIC Schema - Add / Edit / Delete / Import / Export bibliographic records - Add items - Reindex records using ES - Confirm that the following scripts still work: * misc/cronjobs/delete_records_via_leader.pl * misc/migration_tools/build_oai_sets.pl - Look at the reading history at the OPAC (opac-readingrecord.pl) - At the OPAC, click on a tag, you must see the result Note: Changes in Koha/OAI/Server/ListRecords.pm is planned on bug 15108.
Created attachment 55857 [details] [review] Bug 17196: Fix query builder for item search The item search needs to join on biblio_metadata to allow search on marcxml field Test plan: Launch complex item searches (using marc fields).
Created attachment 55858 [details] [review] Bug 17196: Update misc/maintenance/sanitize_records.pl Test plan: Use the sanitize_records.pl maintenance script with the --auto-search option
Created attachment 55859 [details] [review] Bug 17196: Update switch_marc21_series_info.pl Not sure if this script is still used, could someone confirm? Test plan: If you know how to test it, please do
Created attachment 55860 [details] [review] Bug 17196: DB Changes Remove biblioitems.marcxml and deletedbiblioitems.marcxml and adds 2 table biblio_metadata and deletedbiblio_metadata
Created attachment 55904 [details] [review] Bug 17196: Move marcxml out of the biblioitems table Two discussions on koha-devel lead to the same conclusion: biblioitems.marcxml should be moved out this table - biblio and biblioitems http://lists.koha-community.org/pipermail/koha-devel/2013-April/039239.html - biblioitems.marcxml & biblioitems.marc / HUGE performance issue ! http://lists.koha-community.org/pipermail/koha-devel/2016-July/042821.html There are several goals to do it: - Performance As Paul Poulain wrote, a simple query like SELECT publicationyear, count(publicationyear) FROM biblioitems GROUP BY publicationyear; takes more than 10min on a DB with more than 1M bibliographic records but only 3sec (!) on the same DB without the biblioitems.marcxml field Note that priori to this patch set, the biblioitems.marcxml was not retrieved systematically, but was, at least, in C4::Acquisition::GetOrdersByBiblionumber and C4::Acquisition::GetOrders - Flexibility Storing the marcxml in a specific table would allow use to store several kind of metadata (USMARC, MARCXML, MIJ, etc.) and different formats (marcflavour) - Clean code It would be a first step toward Koha::MetadataRecord for bibliographic records (not done in this patch set). Test plan: - Update the DBIC Schema - Add / Edit / Delete / Import / Export bibliographic records - Add items - Reindex records using ES - Confirm that the following scripts still work: * misc/cronjobs/delete_records_via_leader.pl * misc/migration_tools/build_oai_sets.pl - Look at the reading history at the OPAC (opac-readingrecord.pl) - At the OPAC, click on a tag, you must see the result Note: Changes in Koha/OAI/Server/ListRecords.pm is planned on bug 15108. Signed-off-by: Mason James <mtj@kohaaloha.com>
Created attachment 55905 [details] [review] Bug 17196: Fix query builder for item search The item search needs to join on biblio_metadata to allow search on marcxml field Test plan: Launch complex item searches (using marc fields). Signed-off-by: Mason James <mtj@kohaaloha.com>
Created attachment 55906 [details] [review] Bug 17196: Update misc/maintenance/sanitize_records.pl Test plan: Use the sanitize_records.pl maintenance script with the --auto-search option Signed-off-by: Mason James <mtj@kohaaloha.com>
Created attachment 55907 [details] [review] Bug 17196: Update switch_marc21_series_info.pl Not sure if this script is still used, could someone confirm? Test plan: If you know how to test it, please do Signed-off-by: Mason James <mtj@kohaaloha.com>
Created attachment 55908 [details] [review] Bug 17196: DB Changes Remove biblioitems.marcxml and deletedbiblioitems.marcxml and adds 2 table biblio_metadata and deletedbiblio_metadata Signed-off-by: Mason James <mtj@kohaaloha.com>
Hi Folks I have made some good progress testing this patch-set I did hit a problem, when attempting to save a bib record via the cataloguing editor I added a "DBI->trace(2,'/tmp/some.log')" to the start of the ModBiblio() sub When saving a bib, I noticed that there is no SQL in the log to UPDATE the 'biblio_metadata' table fyi: before testing, i ran 'update_dbix_class_files.pl' to update the schemas
(In reply to Mason James from comment #23) > Hi Folks > I have made some good progress testing this patch-set > > I did hit a problem, when attempting to save a bib record via the > cataloguing editor > I added a "DBI->trace(2,'/tmp/some.log')" to the start of the ModBiblio() sub > > When saving a bib, I noticed that there is no SQL in the log to UPDATE the > 'biblio_metadata' table a quick grep thru the dbi.log confirms this (i think?) $ grep UPDATE /tmp/some.log -> prepare for DBD::mysql::db (DBI::db=HASH(0x55203c8)~0x552f888 'UPDATE biblio SET frameworkcode=? WHERE biblionumber=?') thr#1a20010 -> dbd_st_prepare MYSQL_VERSION_ID 50550, SQL statement: UPDATE biblio SET frameworkcode=? WHERE biblionumber=? >count_params statement UPDATE biblio SET frameworkcode=? WHERE biblionumber=? >parse_params statement UPDATE biblio SET frameworkcode=? WHERE biblionumber=? Binding parameters: UPDATE biblio SET frameworkcode='BKS' WHERE biblionumber='3449' UPDATE biblio UPDATE biblio UPDATE biblio UPDATE biblio Binding parameters: UPDATE biblio -> prepare for DBD::mysql::db (DBI::db=HASH(0x55203c8)~0x552f888 'UPDATE biblioitems -> dbd_st_prepare MYSQL_VERSION_ID 50550, SQL statement: UPDATE biblioitems >count_params statement UPDATE biblioitems >parse_params statement UPDATE biblioitems Binding parameters: UPDATE biblioitems
(In reply to Mason James from comment #23) > Hi Folks > I have made some good progress testing this patch-set > > I did hit a problem, when attempting to save a bib record via the > cataloguing editor could someone else confirm this bug, please?
Created attachment 55921 [details] [review] Bug 17196: Simplify tests for the different marc flavours centralize the way MARC field are set.
Created attachment 55922 [details] [review] Bug 17196: Fix - Update metadata on update
(In reply to Mason James from comment #25) > (In reply to Mason James from comment #23) > > Hi Folks > > I have made some good progress testing this patch-set > > > > I did hit a problem, when attempting to save a bib record via the > > cataloguing editor > > could someone else confirm this bug, please? Confirmed! Sorry about that, I should have written these tests before...
Created attachment 55950 [details] [review] Bug 17196: Simplify tests for the different marc flavours centralize the way MARC field are set. Signed-off-by: Mason James <mtj@kohaaloha.com>
Created attachment 55951 [details] [review] Bug 17196: Fix - Update metadata on update Signed-off-by: Mason James <mtj@kohaaloha.com>
(In reply to Jonathan Druart from comment #28) > (In reply to Mason James from comment #25) > > (In reply to Mason James from comment #23) > > > Hi Folks > > > I have made some good progress testing this patch-set > > > > > > I did hit a problem, when attempting to save a bib record via the > > > cataloguing editor > > > > could someone else confirm this bug, please? > > Confirmed! > Sorry about that, I should have written these tests before... Hi Jonathan, your last 2 patches fixed the 'save' problem. I signed-off on them I realise that there is still some ES testing, for the patch-set. I'll try to do that
Created attachment 56038 [details] [review] Bug 17196: follow-up for bug 17088 If bug 17088 is pushed first, we will need this patch.
So.. I'm in favour of this in general.. But.. I'm wondering about the wisdom of the schema a bit. I would settle on a serialisation format (JSON, XML) and abstract the inflation/deflation of it into the dbic class using InflateColumn and friends. This would allow us to take advantage of either postgres's jsonb store or mysql's in built xml handling consistently allowing those tools to be fully utilised in reports. Without doing so, would mean reports would end up much more limited. So, I would ditch the 'format' field, fix the serialisation format in the metadata field and use dbic to inflate into perl data-structures consistently. We could still happily bind to the 'marcflavour' field to build objects of different types from the inflated perl structures (though I would change marcflavour to just flavour in case we want to support non-marc later ;) Just my two pence.
CONFLICT (content): Merge conflict in koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt
sorry, I published the conflict in the wrong bug
Created attachment 57835 [details] [review] Bug 17196: Move marcxml out of the biblioitems table Two discussions on koha-devel lead to the same conclusion: biblioitems.marcxml should be moved out this table - biblio and biblioitems http://lists.koha-community.org/pipermail/koha-devel/2013-April/039239.html - biblioitems.marcxml & biblioitems.marc / HUGE performance issue ! http://lists.koha-community.org/pipermail/koha-devel/2016-July/042821.html There are several goals to do it: - Performance As Paul Poulain wrote, a simple query like SELECT publicationyear, count(publicationyear) FROM biblioitems GROUP BY publicationyear; takes more than 10min on a DB with more than 1M bibliographic records but only 3sec (!) on the same DB without the biblioitems.marcxml field Note that priori to this patch set, the biblioitems.marcxml was not retrieved systematically, but was, at least, in C4::Acquisition::GetOrdersByBiblionumber and C4::Acquisition::GetOrders - Flexibility Storing the marcxml in a specific table would allow use to store several kind of metadata (USMARC, MARCXML, MIJ, etc.) and different formats (marcflavour) - Clean code It would be a first step toward Koha::MetadataRecord for bibliographic records (not done in this patch set). Test plan: - Update the DBIC Schema - Add / Edit / Delete / Import / Export bibliographic records - Add items - Reindex records using ES - Confirm that the following scripts still work: * misc/cronjobs/delete_records_via_leader.pl * misc/migration_tools/build_oai_sets.pl - Look at the reading history at the OPAC (opac-readingrecord.pl) - At the OPAC, click on a tag, you must see the result Note: Changes in Koha/OAI/Server/ListRecords.pm is planned on bug 15108. Signed-off-by: Mason James <mtj@kohaaloha.com>
Created attachment 57836 [details] [review] Bug 17196: Fix query builder for item search The item search needs to join on biblio_metadata to allow search on marcxml field Test plan: Launch complex item searches (using marc fields). Signed-off-by: Mason James <mtj@kohaaloha.com>
Created attachment 57837 [details] [review] Bug 17196: Update misc/maintenance/sanitize_records.pl Test plan: Use the sanitize_records.pl maintenance script with the --auto-search option Signed-off-by: Mason James <mtj@kohaaloha.com>
Created attachment 57838 [details] [review] Bug 17196: Update switch_marc21_series_info.pl Not sure if this script is still used, could someone confirm? Test plan: If you know how to test it, please do Signed-off-by: Mason James <mtj@kohaaloha.com>
Created attachment 57839 [details] [review] Bug 17196: DB Changes Remove biblioitems.marcxml and deletedbiblioitems.marcxml and adds 2 table biblio_metadata and deletedbiblio_metadata Signed-off-by: Mason James <mtj@kohaaloha.com>
Created attachment 57840 [details] [review] Bug 17196: Simplify tests for the different marc flavours centralize the way MARC field are set. Signed-off-by: Mason James <mtj@kohaaloha.com>
Created attachment 57841 [details] [review] Bug 17196: Fix - Update metadata on update Signed-off-by: Mason James <mtj@kohaaloha.com>
Created attachment 57842 [details] [review] Bug 17196: follow-up for bug 17088 If bug 17088 is pushed first, we will need this patch.
(In reply to Hugo Agud from comment #35) > sorry, I published the conflict in the wrong bug But they needed a rebase anyway :)
Hi Johathan, I'm testig the patches in this bug. I think there is a problem in "Bug 17196: follow-up for bug 17088". Now is mandatory, bug 17088 is into the source. The result of "prove Record.t" is: Record.t .. DBIx::Class::Row::store_column(): No such column 'format' on Koha::Schema::Result::Biblioitem at /production/koha/install/bug17196/Koha/Object.pm line 71 # Looks like your test exited with 255 before it could output anything. Record.t .. Dubious, test returned 255 (wstat 65280, 0xff00) Failed 4/4 subtests In fact a'format' column is not present in table biblioitems. Bye
Created attachment 58000 [details] [review] Bug 17196: follow-up for bug 17088 If bug 17088 is pushed first, we will need this patch.
Hi Zeno, Yes, thanks for reporting. Last patch should fix the tests.
So, I started to test this, it looks good so far only few thing: 1. build_oai_sets.pl script does return this error on every biblio: (biblio 3) Error while creating record from marcxml: Empty String at /usr/share/perl5/MARC/File/XML.pm line 448. 2. When I click on tag in opac, I get ISE, the log says: Can't locate object method "subfield" via package "<?xml version="1.0" encoding="UTF-8"?><record .. whole xml record" at C4/Search.pm line 1925 3. The usage text for --auto-search paramater in sanitize_records.pl still says: "Automatically search records containing "&" in biblioitems.marcxml or in the specified fields."
(In reply to Martin Renvoize from comment #33) No response to this :( Personally, I'd use JSON as both Postgres (https://www.postgresql.org/docs/current/static/datatype-json.html) and MySQL (https://dev.mysql.com/doc/refman/5.7/en/json.html) have pretty extensive support for marked performance gains using JSON typed fields.
(In reply to Martin Renvoize from comment #49) > (In reply to Martin Renvoize from comment #33) > > No response to this :( > > Personally, I'd use JSON as both Postgres > (https://www.postgresql.org/docs/current/static/datatype-json.html) and > MySQL (https://dev.mysql.com/doc/refman/5.7/en/json.html) have pretty > extensive support for marked performance gains using JSON typed fields. The first goal of these patches is to move the data out of this table to bring speed improvement. Afterwards we will think about what we can do with this new table :) As you know, we are not ready yet to support another DBMS.
Created attachment 58007 [details] [review] Bug 17196: Fix build_oai_sets.pl The wrong value was retrieved.
Created attachment 58008 [details] [review] Bug 17196: Update POD of sanitise_records.pl
(In reply to Josef Moravec from comment #48) > So, I started to test this, it looks good so far > > only few thing: > > 1. build_oai_sets.pl script does return this error on every biblio: > (biblio 3) Error while creating record from marcxml: Empty String at > /usr/share/perl5/MARC/File/XML.pm line 448. Will be fixed by Bug 17196: Fix build_oai_sets.pl > 2. When I click on tag in opac, I get ISE, the log says: > Can't locate object method "subfield" via package "<?xml version="1.0" > encoding="UTF-8"?><record .. whole xml record" at C4/Search.pm line 1925 It's a bug from master, please open a new bug report. Note that it only happens when SearchEngine=Elastic > 3. The usage text for --auto-search paramater in sanitize_records.pl still > says: "Automatically search records containing "&" in > biblioitems.marcxml or in the specified fields." Will be fixed by Bug 17196: Update POD of sanitise_records.pl
Hi Jonathan, I think there problems on "Fix query builder for item search". I try to do a search with a personalized index based on 245$c. The search doesn't use the index based on 245$c. But it use the standard indexes (based on table 'items'). No errors in intranet logs Bye
(In reply to Zeno Tajoli from comment #54) > Hi Jonathan, > > I think there problems on "Fix query builder for item search". > I try to do a search with a personalized index based on 245$c. > The search doesn't use the index based on 245$c. > But it use the standard indexes (based on table 'items'). > > No errors in intranet logs > > Bye Hi Zeno, I am not sure to understand correctly what you mean. Which index did you create? Did you confirm that what you describe is not the same behaviour as in master?
Hi, I go here:Home › Administration › Item search fields and I add a index based on tag 245$c. But I check the default here: http://kohastaffsandbox.ptfs-europe.co.uk and also on master it doesn't work. So it is an other bug. Bye
(In reply to Zeno Tajoli from comment #56) > I go here:Home › Administration › Item search fields and I add a index based > on tag 245$c. > But I check the default here: > http://kohastaffsandbox.ptfs-europe.co.uk and also on master it doesn't work. > So it is an other bug. Ok thanks, so please report it on another bug report.
(In reply to Jonathan Druart from comment #53) > (In reply to Josef Moravec from comment #48) > > > 2. When I click on tag in opac, I get ISE, the log says: > > Can't locate object method "subfield" via package "<?xml version="1.0" > > encoding="UTF-8"?><record .. whole xml record" at C4/Search.pm line 1925 > > It's a bug from master, please open a new bug report. > Note that it only happens when SearchEngine=Elastic Done in bug 17739 as you have seen ;)
Created attachment 58023 [details] [review] [SIGNED-OFF] Bug 17196: Move marcxml out of the biblioitems table Two discussions on koha-devel lead to the same conclusion: biblioitems.marcxml should be moved out this table - biblio and biblioitems http://lists.koha-community.org/pipermail/koha-devel/2013-April/039239.html - biblioitems.marcxml & biblioitems.marc / HUGE performance issue ! http://lists.koha-community.org/pipermail/koha-devel/2016-July/042821.html There are several goals to do it: - Performance As Paul Poulain wrote, a simple query like SELECT publicationyear, count(publicationyear) FROM biblioitems GROUP BY publicationyear; takes more than 10min on a DB with more than 1M bibliographic records but only 3sec (!) on the same DB without the biblioitems.marcxml field Note that priori to this patch set, the biblioitems.marcxml was not retrieved systematically, but was, at least, in C4::Acquisition::GetOrdersByBiblionumber and C4::Acquisition::GetOrders - Flexibility Storing the marcxml in a specific table would allow use to store several kind of metadata (USMARC, MARCXML, MIJ, etc.) and different formats (marcflavour) - Clean code It would be a first step toward Koha::MetadataRecord for bibliographic records (not done in this patch set). Test plan: - Update the DBIC Schema - Add / Edit / Delete / Import / Export bibliographic records - Add items - Reindex records using ES - Confirm that the following scripts still work: * misc/cronjobs/delete_records_via_leader.pl * misc/migration_tools/build_oai_sets.pl - Look at the reading history at the OPAC (opac-readingrecord.pl) - At the OPAC, click on a tag, you must see the result Note: Changes in Koha/OAI/Server/ListRecords.pm is planned on bug 15108. Signed-off-by: Mason James <mtj@kohaaloha.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 58024 [details] [review] [SIGNED-OFF] Bug 17196: Fix query builder for item search The item search needs to join on biblio_metadata to allow search on marcxml field Test plan: Launch complex item searches (using marc fields). Signed-off-by: Mason James <mtj@kohaaloha.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 58025 [details] [review] [SIGNED-OFF] Bug 17196: Update misc/maintenance/sanitize_records.pl Test plan: Use the sanitize_records.pl maintenance script with the --auto-search option Signed-off-by: Mason James <mtj@kohaaloha.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 58026 [details] [review] [SIGNED-OFF] Bug 17196: Update switch_marc21_series_info.pl Not sure if this script is still used, could someone confirm? Test plan: If you know how to test it, please do Signed-off-by: Mason James <mtj@kohaaloha.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 58027 [details] [review] [SIGNED-OFF] Bug 17196: DB Changes Remove biblioitems.marcxml and deletedbiblioitems.marcxml and adds 2 table biblio_metadata and deletedbiblio_metadata Signed-off-by: Mason James <mtj@kohaaloha.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 58028 [details] [review] [SIGNED-OFF] Bug 17196: Simplify tests for the different marc flavours centralize the way MARC field are set. Signed-off-by: Mason James <mtj@kohaaloha.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 58029 [details] [review] [SIGNED-OFF] Bug 17196: Fix - Update metadata on update Signed-off-by: Mason James <mtj@kohaaloha.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 58030 [details] [review] [SIGNED-OFF] Bug 17196: follow-up for bug 17088 If bug 17088 is pushed first, we will need this patch. Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 58031 [details] [review] [SIGNED-OFF] Bug 17196: Fix build_oai_sets.pl The wrong value was retrieved. Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 58032 [details] [review] [SIGNED-OFF] Bug 17196: Update POD of sanitise_records.pl Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 58037 [details] [review] Move marcxml out of the biblioitems table
Created attachment 58038 [details] Fix query builder for item search
Created attachment 58039 [details] [review] [SIGNED-OFF] Bug 17196: Move marcxml out of the biblioitems table
Created attachment 58040 [details] [review] [SIGNED-OFF] Bug 17196: Fix query builder for item search
Created attachment 58041 [details] [review] [SIGNED-OFF] Bug 17196: Update misc/maintenance/sanitize_records.pl
Created attachment 58042 [details] [review] [SIGNED-OFF] Bug 17196: Update switch_marc21_series_info.pl
Created attachment 58043 [details] [review] [SIGNED-OFF] Bug 17196: DB Changes
Created attachment 58044 [details] [review] [SIGNED-OFF] Bug 17196: Simplify tests for the different marc flavours
Created attachment 58045 [details] [review] [SIGNED-OFF] Bug 17196: Fix - Update metadata on update
Created attachment 58046 [details] [review] [SIGNED-OFF] Bug 17196: follow-up for bug 17088
Created attachment 58047 [details] [review] [SIGNED-OFF] Bug 17196: Fix build_oai_sets.pl
Created attachment 58048 [details] [review] [SIGNED-OFF] Bug 17196: Update POD of sanitise_records.pl
I have done a second sign-off and written notes about this develop.
Testing this I found one things: When trying delete_records_via_leader.pl: kohadev-koha@kohadevbox:/home/vagrant/kohaclone$ perl misc/cronjobs/delete_records_via_leader.pl -v -c -i RECORD: 288 Can't use string ("Koha::Biblioitem") as a HASH ref while "strict refs" in use at /home/vagrant/kohaclone/Koha/Object.pm line 219.
Created attachment 58504 [details] [review] Bug 17196: Fix delete_records_via_leader.pl
Created attachment 58505 [details] [review] Bug 17196: Move marcxml out of the biblioitems table Two discussions on koha-devel lead to the same conclusion: biblioitems.marcxml should be moved out this table - biblio and biblioitems http://lists.koha-community.org/pipermail/koha-devel/2013-April/039239.html - biblioitems.marcxml & biblioitems.marc / HUGE performance issue ! http://lists.koha-community.org/pipermail/koha-devel/2016-July/042821.html There are several goals to do it: - Performance As Paul Poulain wrote, a simple query like SELECT publicationyear, count(publicationyear) FROM biblioitems GROUP BY publicationyear; takes more than 10min on a DB with more than 1M bibliographic records but only 3sec (!) on the same DB without the biblioitems.marcxml field Note that priori to this patch set, the biblioitems.marcxml was not retrieved systematically, but was, at least, in C4::Acquisition::GetOrdersByBiblionumber and C4::Acquisition::GetOrders - Flexibility Storing the marcxml in a specific table would allow use to store several kind of metadata (USMARC, MARCXML, MIJ, etc.) and different formats (marcflavour) - Clean code It would be a first step toward Koha::MetadataRecord for bibliographic records (not done in this patch set). Test plan: - Update the DBIC Schema - Add / Edit / Delete / Import / Export bibliographic records - Add items - Reindex records using ES - Confirm that the following scripts still work: * misc/cronjobs/delete_records_via_leader.pl * misc/migration_tools/build_oai_sets.pl - Look at the reading history at the OPAC (opac-readingrecord.pl) - At the OPAC, click on a tag, you must see the result Note: Changes in Koha/OAI/Server/ListRecords.pm is planned on bug 15108. Signed-off-by: Mason James <mtj@kohaaloha.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Zeno Tajoli <z.tajoli@cineca.it> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 58506 [details] [review] Bug 17196: Fix query builder for item search The item search needs to join on biblio_metadata to allow search on marcxml field Test plan: Launch complex item searches (using marc fields). Signed-off-by: Mason James <mtj@kohaaloha.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Zeno Tajoli <z.tajoli@cineca.it> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 58507 [details] [review] Bug 17196: Update misc/maintenance/sanitize_records.pl Test plan: Use the sanitize_records.pl maintenance script with the --auto-search option Signed-off-by: Mason James <mtj@kohaaloha.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Zeno Tajoli <z.tajoli@cineca.it> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 58508 [details] [review] Bug 17196: Update switch_marc21_series_info.pl Not sure if this script is still used, could someone confirm? Test plan: If you know how to test it, please do Signed-off-by: Mason James <mtj@kohaaloha.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Zeno Tajoli <z.tajoli@cineca.it> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 58509 [details] [review] Bug 17196: DB Changes Remove biblioitems.marcxml and deletedbiblioitems.marcxml and adds 2 table biblio_metadata and deletedbiblio_metadata Signed-off-by: Mason James <mtj@kohaaloha.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Zeno Tajoli <z.tajoli@cineca.it> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 58510 [details] [review] Bug 17196: Simplify tests for the different marc flavours centralize the way MARC field are set. Signed-off-by: Mason James <mtj@kohaaloha.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Zeno Tajoli <z.tajoli@cineca.it> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 58511 [details] [review] Bug 17196: Fix - Update metadata on update Signed-off-by: Mason James <mtj@kohaaloha.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Zeno Tajoli <z.tajoli@cineca.it> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 58512 [details] [review] Bug 17196: follow-up for bug 17088 If bug 17088 is pushed first, we will need this patch. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Zeno Tajoli <z.tajoli@cineca.it> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 58513 [details] [review] Bug 17196: Fix build_oai_sets.pl The wrong value was retrieved. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Zeno Tajoli <z.tajoli@cineca.it> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 58514 [details] [review] Bug 17196: Update POD of sanitise_records.pl Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Zeno Tajoli <z.tajoli@cineca.it> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 58516 [details] [review] Bug 17196: Fix delete_records_via_leader.pl Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 58517 [details] [review] Bug 17196 - (QA Followup) Add items() to Koha::Biblioitems Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Tested and provided a small update
Still applies. If it does on early Friday morning, I will have a look.
QA: Working on this one now.
Created attachment 58923 [details] [review] Bug 17196: Move marcxml out of the biblioitems table Two discussions on koha-devel lead to the same conclusion: biblioitems.marcxml should be moved out this table - biblio and biblioitems http://lists.koha-community.org/pipermail/koha-devel/2013-April/039239.html - biblioitems.marcxml & biblioitems.marc / HUGE performance issue ! http://lists.koha-community.org/pipermail/koha-devel/2016-July/042821.html There are several goals to do it: - Performance As Paul Poulain wrote, a simple query like SELECT publicationyear, count(publicationyear) FROM biblioitems GROUP BY publicationyear; takes more than 10min on a DB with more than 1M bibliographic records but only 3sec (!) on the same DB without the biblioitems.marcxml field Note that priori to this patch set, the biblioitems.marcxml was not retrieved systematically, but was, at least, in C4::Acquisition::GetOrdersByBiblionumber and C4::Acquisition::GetOrders - Flexibility Storing the marcxml in a specific table would allow use to store several kind of metadata (USMARC, MARCXML, MIJ, etc.) and different formats (marcflavour) - Clean code It would be a first step toward Koha::MetadataRecord for bibliographic records (not done in this patch set). Test plan: - Update the DBIC Schema - Add / Edit / Delete / Import / Export bibliographic records - Add items - Reindex records using ES - Confirm that the following scripts still work: * misc/cronjobs/delete_records_via_leader.pl * misc/migration_tools/build_oai_sets.pl - Look at the reading history at the OPAC (opac-readingrecord.pl) - At the OPAC, click on a tag, you must see the result Note: Changes in Koha/OAI/Server/ListRecords.pm is planned on bug 15108. Signed-off-by: Mason James <mtj@kohaaloha.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Zeno Tajoli <z.tajoli@cineca.it> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 58924 [details] [review] Bug 17196: Fix query builder for item search The item search needs to join on biblio_metadata to allow search on marcxml field Test plan: Launch complex item searches (using marc fields). Signed-off-by: Mason James <mtj@kohaaloha.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Zeno Tajoli <z.tajoli@cineca.it> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 58925 [details] [review] Bug 17196: Update misc/maintenance/sanitize_records.pl Test plan: Use the sanitize_records.pl maintenance script with the --auto-search option Signed-off-by: Mason James <mtj@kohaaloha.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Zeno Tajoli <z.tajoli@cineca.it> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 58926 [details] [review] Bug 17196: Update switch_marc21_series_info.pl Not sure if this script is still used, could someone confirm? Test plan: If you know how to test it, please do Signed-off-by: Mason James <mtj@kohaaloha.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Zeno Tajoli <z.tajoli@cineca.it> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 58927 [details] [review] Bug 17196: DB Changes Remove biblioitems.marcxml and deletedbiblioitems.marcxml and adds 2 table biblio_metadata and deletedbiblio_metadata Signed-off-by: Mason James <mtj@kohaaloha.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Zeno Tajoli <z.tajoli@cineca.it> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 58928 [details] [review] Bug 17196: Simplify tests for the different marc flavours centralize the way MARC field are set. Signed-off-by: Mason James <mtj@kohaaloha.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Zeno Tajoli <z.tajoli@cineca.it> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 58929 [details] [review] Bug 17196: Fix - Update metadata on update Signed-off-by: Mason James <mtj@kohaaloha.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Zeno Tajoli <z.tajoli@cineca.it> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 58930 [details] [review] Bug 17196: follow-up for bug 17088 If bug 17088 is pushed first, we will need this patch. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Zeno Tajoli <z.tajoli@cineca.it> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 58931 [details] [review] Bug 17196: Fix build_oai_sets.pl The wrong value was retrieved. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Zeno Tajoli <z.tajoli@cineca.it> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 58932 [details] [review] Bug 17196: Update POD of sanitise_records.pl Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Zeno Tajoli <z.tajoli@cineca.it> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 58933 [details] [review] Bug 17196: Fix delete_records_via_leader.pl Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 58934 [details] [review] Bug 17196 - (QA Followup) Use items from Koha::Biblio Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Amended: Using items from Koha::Biblio seems better :)
Created attachment 58935 [details] [review] Bug 17196: [QA Follow-up] Correct POD in ILSDI/Services.pm QA tools complains about: Apparent command =cut not preceded by blank line. Trivial fix. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 58936 [details] [review] Bug 17196: [QA Follow-up] DBIx schema changes Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 58937 [details] [review] Bug 17196: [QA Follow-up] Wrong module in Record.t Should be the singular version instead of the plural one. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 58938 [details] [review] Bug 17196: [QA Follow-up] Missing unique key in kohastructure See the atomic update. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Tested on command line.
Still working on it little bit, but looks good. Please wait..
Some preliminary findings: opac/opac-readingrecord.pl: Calling GetXmlBiblio seems useless here. Probably should be GetMarcBiblio. opac-search.pl: map { (C4::Context->config('zebra_bib_index_mode') eq 'dom')? $_->{marcxml}: $_->{marc}; } This marc reference should not have been there anymore? C4/Items.pm: changes aimed for catalogue/itemsearch.pl, but I am not sure if they are needed. Does item search actually looks into the xml? Seems not. New report??
(In reply to Marcel de Rooy from comment #116) > Some preliminary findings: > > opac/opac-readingrecord.pl: Calling GetXmlBiblio seems useless here. > Probably should be GetMarcBiblio. GetMarcBiblio does much more stuffs than GetXmlBiblio. Since we just want the metadata it's better like that. > opac-search.pl: map { (C4::Context->config('zebra_bib_index_mode') eq > 'dom')? $_->{marcxml}: $_->{marc}; } This marc reference should not have > been there anymore? Nope, the column does no longer exist. > C4/Items.pm: changes aimed for catalogue/itemsearch.pl, but I am not sure if > they are needed. Does item search actually looks into the xml? Seems not. > New report?? I don't think so, it uses it.
(In reply to Jonathan Druart from comment #117) > > C4/Items.pm: changes aimed for catalogue/itemsearch.pl, but I am not sure if > > they are needed. Does item search actually looks into the xml? Seems not. > > New report?? > > I don't think so, it uses it. See C4::Items::_SearchItems_build_where_fragment
> C4/Items.pm: changes aimed for catalogue/itemsearch.pl, but I am not sure if > they are needed. Does item search actually looks into the xml? Seems not. > New report?? So that is wrong. You can do it via dmin/items_search_fields.pl. Works fine.
dmin -> admin
Created attachment 58939 [details] [review] Bug 17196: Remove occurrence of marcxml in columns.def Also remove a wrong ref to the marc column
Created attachment 58943 [details] [review] Bug 17196: Remove occurrence of marcxml in columns.def Also remove a wrong ref to the marc column Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 58946 [details] [review] Bug 17196: [QA Follow-up] Adjust some text on marcxml No code changes here, just text. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 58947 [details] [review] Bug 17196: [QA Follow-up] Additional fix on acqui/basketgroup acqui/basketgroup calls GetOrders and expects marcxml in the results. Fixing it by an additional call of GetXmlBiblio. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Last comments: > Changes in Koha/OAI/Server/ListRecords.pm is planned on bug 15108. Failed QA several weeks ago. The author did not respond until now. [OK] t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t [OK] t/db_dependent/Holds/HoldFulfillmentPolicy.t [OK] t/db_dependent/Holds/HoldItemtypeLimit.t [OK} t/db_dependent/HoldsQueue.t [OK] t/db_dependent/UsageStats.t [OK] t/db_dependent/Members/GetAllIssues.t [OK] t/db_dependent/Items.t [OK] t/ImportBatch.t [OK] t/db_dependent/OAI/Server.t [OK] t/db_dependent/Record/Record.t [OK] t/db_dependent/Record.t [OK] t/db_dependent/Search.t [OK] t/db_dependent/Acquisition/GetOrdersByBiblionumber.t [OK] t/db_dependent/Acquisition/TransferOrder.t misc/migration_tools/koha-svc.pl: Did not verify it this still works. koha-svc.pl http://koha-dev:8080/cgi-bin/koha/svc svc-user svc-password $biblionumber [bib-42.xml] Did not check REST API. Well finally, we know about Reports. Should we still warn somewhere about that ?
(In reply to Marcel de Rooy from comment #125) > Last comments: > > > Changes in Koha/OAI/Server/ListRecords.pm is planned on bug 15108. > Failed QA several weeks ago. The author did not respond until now. > Ere Maijala is primerly working on VuFind development, so probably we could try to contact them by e-mail directly to ask if he is plannig to work a bit on this OAI enhancement.
(In reply to Josef Moravec from comment #126) > (In reply to Marcel de Rooy from comment #125) > > Last comments: > > > > > Changes in Koha/OAI/Server/ListRecords.pm is planned on bug 15108. > > Failed QA several weeks ago. The author did not respond until now. > > > > Ere Maijala is primerly working on VuFind development, so probably we could > try to contact them by e-mail directly to ask if he is plannig to work a bit > on this OAI enhancement. Josef, If you know him, could you please ask him for rebase and feedback. Thanks
Pushed to master for 17.05, thanks Jonathan, Nick, Marcel!
This won't get ported back to 16.11.x as it is an enhancement.
Comment on attachment 58927 [details] [review] Bug 17196: DB Changes Review of attachment 58927 [details] [review]: ----------------------------------------------------------------- ::: installer/data/mysql/kohastructure.sql @@ +3926,5 @@ > + > +CREATE TABLE biblio_metadata ( > + `id` INT(11) NOT NULL AUTO_INCREMENT, > + `biblionumber` INT(11) NOT NULL, > + `format` VARCHAR(16) NOT NULL, I know this has already been pushed, but maybe the RM could add some comments to this SQL? In the comments, I think format was "USMARC, MARCXML, MiJ"? I'd maybe add a comment saying something like "ISO2709, XML, JSON", although maybe I'm misunderstanding how the field will actually be used... @@ +3927,5 @@ > +CREATE TABLE biblio_metadata ( > + `id` INT(11) NOT NULL AUTO_INCREMENT, > + `biblionumber` INT(11) NOT NULL, > + `format` VARCHAR(16) NOT NULL, > + `marcflavour` VARCHAR(16) NOT NULL, It seems unfortunate that we're using marcflavour here after divorcing this table from MARC in every other way. Although I don't know how else we'd do this. Both UNIMARC and USMARC share the same MARCXML schema, and if we can have non-XML data, we couldn't use "schema" instad of "marcflavour". Although if it were Dublin Core, you'd want to be able to specify that schema I reckon. Maybe a person could cheat and use a format of XML and a schema of UNIMARC or USMARC, and you'd use the actual MARCXML schema for both but you'd know you're dealing with one standard or the other. I don't know. I'm guessing all the work's actually be done on this one anyway, so I'm probably too late with these comments.
Hi, > +CREATE TABLE biblio_metadata ( > + `id` INT(11) NOT NULL AUTO_INCREMENT, > + `biblionumber` INT(11) NOT NULL, > + `format` VARCHAR(16) NOT NULL, > + `marcflavour` VARCHAR(16) NOT NULL, in fact I think it is a case of not perfect selection of names. 'format' = 'physical_format' [ISO2709, XML, JSON, etc.] 'marcflavour' = 'logical_format' [MARC21, UNIMARC, Dublin Core, etc.]
I am willing to change the names of these fields if you have a good suggestion. It's the beginning of the end of MARC, so I think I can spend 10 min to rename the fields. (In reply to David Cook from comment #130) > I don't know. I'm guessing all the work's actually be done on this one > anyway, so I'm probably too late with these comments. Indeed, I asked for feedbacks on these patches for the last 6 months... But better late than never, isn't it?
I don't want to beat a dead horse, but can't resist: > There are several goals to do it: > - Performance > As Paul Poulain wrote, a simple query like > SELECT publicationyear, count(publicationyear) FROM biblioitems GROUP BY > publicationyear; > takes more than 10min on a DB with more than 1M bibliographic records > but only 3sec (!) on the same DB without the biblioitems.marcxml field All our test environments: MariaDB [koha]> SELECT COUNT(*) FROM biblioitems; +----------+ | COUNT(*) | +----------+ | 1424537 | +----------+ 1 row in set (0.34 sec) On Ubuntu14.04 MariaDB using a normal 10k HDD with Intel Xeon 3GHz: innodb_buffer_pool_size = 128MB MariaDB [koha]> SELECT publicationyear, count(publicationyear) FROM biblioitems GROUP BY publicationyear; +-----------------+------------------------+ | publicationyear | count(publicationyear) | +-----------------+------------------------+ | NULL | 0 | +-----------------+------------------------+ 1 row in set (39.64 sec) The search is arguably slow. I wonder how the performance inspiration server (10 min search time) is configured? On Ubuntu14.04 MariaDB using RAID-1 SSD with Intel Xeon 3GHz 32GB RAM innodb_buffer_pool_size = 24G (Percona autoconfigurer) MariaDB [koha]> SELECT publicationyear, count(publicationyear) FROM biblioitems GROUP BY publicationyear; +-----------------+------------------------+ | publicationyear | count(publicationyear) | +-----------------+------------------------+ | NULL | 0 | +-----------------+------------------------+ 1 row in set (5.56 sec) RAM and SSDs are really cheap nowadays. See also Bug 18265
(In reply to Olli-Antti Kivilahti from comment #134) > I don't want to beat a dead horse, but can't resist: Try to be around when we ask for feedbacks and tests, *before* things are pushed. I am sure it would be more useful for everybody. I cannot count how many times I have asked for help on this one. > The search is arguably slow. > I wonder how the performance inspiration server (10 min search time) is > configured? "This is a test DB, not optimized, but we were surprised by the results." http://lists.koha-community.org/pipermail/koha-devel/2016-July/042821.html
Just noticed while doing DB changes, that biblio_metadata is linked to the biblio table instead of the biblioitems-table! This will be fixed in the followup. >Try to be around when we ask for feedbacks and tests, *before* things are pushed. >I am sure it would be more useful for everybody. >I cannot count how many times I have asked for help on this one. Unfortunately that is not possible at the moment. I can only bark later.
Followup Bug 18265 is ready to be SO'd. Regardless of the bugs found, I think this change is welcome. I hope we get the fixes in asap. http://quoteinvestigator.com/2014/12/16/no-mistakes/
(In reply to Olli-Antti Kivilahti from comment #137) > Followup Bug 18265 is ready to be SO'd. I have opened and submitted a patch on bug 18284 to deal with the move of the biblio metadata and the data lost.
*** Bug 15192 has been marked as a duplicate of this bug. ***