Zebra sorting options are hardcoded - but trying to add local-number failed 375 elsif ( $sort eq "local-number_asc" ) { 376 $sort_by .= "1=12 <i "; 377 } 378 elsif ( $sort eq "local-number_dsc" ) { 379 $sort_by .= "1=12 >i "; 380 } I think it is because we index both as word and number 5 <!--record.abs line 24: melm 001 Local-number,Local-number:n--> 6 <index_control_field tag="001"> 7 <target_index>Local-number:w</target_index> 8 </index_control_field> 9 <index_control_field tag="001"> 10 <target_index>Local-number:n</target_index> 11 </index_control_field> I am not sure how to specify sorting by number index, and not word
(In reply to Nick Clemens from comment #0) > I am not sure how to specify sorting by number index, and not word The short answer is that you don't. If you look at ./etc/zebradb/marc_defs/marc21/biblios/biblio-zebra-indexdefs.xsl, you'll see that "pubdate" has a word, number, year, and sort index. You would just need to add Local-number:s for the target index. <z:index name="pubdate:w"> <xslo:value-of select="substring(., 8, 4)"/> </z:index> <z:index name="pubdate:n"> <xslo:value-of select="substring(., 8, 4)"/> </z:index> <z:index name="pubdate:y"> <xslo:value-of select="substring(., 8, 4)"/> </z:index> <z:index name="pubdate:s"> <xslo:value-of select="substring(., 8, 4)"/> </z:index>
Created attachment 136255 [details] [review] Bug 30879: Add option to sort components by biblionumber WIP This works for ES, but zebra sorts the biblionumber as a string, s=109 was my attempt based on https://software.indexdata.com/yaz/doc/tools.html#ccl.syntax which I don't fully understand
(In reply to Nick Clemens from comment #2) > Created attachment 136255 [details] [review] [review] > Bug 30879: Add option to sort components by biblionumber > > WIP > > This works for ES, but zebra sorts the biblionumber as a string, s=109 was > my attempt based on > https://software.indexdata.com/yaz/doc/tools.html#ccl.syntax > which I don't fully understand Sorry but I think you gotten it a bit backwards [U+1F605]. First, it looks like you've updated the UNIMARC files for biblio-koha-indexdefs.xml and biblio-zebra-indexdefs.xsl instead of the MARC21 files. Second, you don't need that change to ccl.properties. -- And now that I'm looking more closely... it looks like we do already index 999$c into the following indexes/registers for MARC 21: Local-Number:n Local-Number:w Local-Number:s So I think you might be going down the wrong path...
Ok I understand what you're saying now... It's not that the sort fails completely. Rather, it's doing a lexicographic sort... so it'll go 1, 10, 2 for ascending 1=12 sort. Yeah that's annoying... Btw, one way to do tests for this is to use "yaz-client". Search on something like "e" in koha-testing-docker, and then run "sort 1=4 >i"
I think this is just a flaw in Zebra. I tried sorting by "itemnumber" which is just defined in the "n" and "s" registers, and it just lexically sorts as well. I've tried reading through the YAZ code which seems to be responsible for sorting, but the sort code is from 1998 and very difficult to understand. It looks like sorting updates the query... but I don't see anything that handles the actual "sorting".
(In reply to David Cook from comment #5) > I've tried reading through the YAZ code which seems to be responsible for > sorting, but the sort code is from 1998 and very difficult to understand. It > looks like sorting updates the query... but I don't see anything that > handles the actual "sorting". Ah I was looking at the wrong Zebra folder which is why my grepping was failing :| According to https://software.indexdata.com/zebra/doc/features.html#features-sort-rank: "Sorting on the basis of alpha-numeric and numeric data is supported." "Sorting on the basis of combined sorts e.g. combinations of ascending/descending sorts of lexicographical/numeric/date field data is supported" "All ordering operations are based on a lexicographical ordering, except when the structure attribute numeric (109) is used. In this case, ordering is numerical. See Section 2.4.3, “Structure Attributes (type 4)”." And that's why you've used that s=109 there... Their documentation sure is ambiguous...
So I think that suggestion with the 4=109 probably only works when providing the 7 attribute: Ascending order: find @or @attr 1=1016 e @attr 7=1 @attr 1=Local-Number @attr 4=109 0 Descending order: find @or @attr 1=1016 e @attr 7=2 @attr 1=Local-Number @attr 4=109 0 (Highest bib is 389 in this case) -- But in this case we're using the ZOOM "sort" method to run the sort query separately from the main query, so I don't think it'll work, as the sort spec can't support it currently. I tried adding 4=109 to "Local-number" in ccl.properties and just sending "Local-number" via the sort and hoping it might do something but nope... -- So ultimately I'm thinking a bug/shortcoming in Zebra.
https://github.com/indexdata/idzebra/issues/37
Thanks for taking a look at this, David!
Our IDs all seem to be INT(11) in the DB - would it be reasonable to zero pad our ids into a new column to fix the sorting?
(In reply to Nick Clemens from comment #10) > Our IDs all seem to be INT(11) in the DB - would it be reasonable to zero > pad our ids into a new column to fix the sorting? Zero padding the ID is smart thinking I reckon. In theory, we wouldn't need a new DB column. We'd just need to add something like the following to our Zebra indexing XSLT for a new custom index (e.g. Local-number-sort): <xsl:value-of select='format-number(1, "00000000000")' /> I think to update "./etc/zebradb/marc_defs/marc21/biblios/biblio-zebra-indexdefs.xsl", we'd need to update the template "handle-index-subfields" in ./etc/zebradb/xsl/koha-indexdefs-to-zebra.xsl
Created attachment 136425 [details] [review] Bug 30879: Add biblionumber as a sorting option in MARC21 This patch updates the Local-Number indexing by adding a zeropad option to Zebra indexing and adding this to the mapping files It also updates C4/Search.pm to allow biblionumber as an option To test: 1 - Apply patches 2 - copy etc/zebradb/marc_defs/marc21/biblios/biblio-zebra-indexdefs.xsl to /etc/koha/zebradb/marc_defs/marc21/biblios/biblio-zebra-indexdefs.xsl 3 - Restart all, reindex zebra 4 - Browse to: http://localhost:8081/cgi-bin/koha/catalogue/search.pl?idx=kw&q=a&sort_by=biblionumber_dsc&count=20 5 - Confirm records sorted correctly 6 - Browse to http://localhost:8081/cgi-bin/koha/catalogue/search.pl?idx=kw&q=a&sort_by=biblionumber_asc&count=20 7 - Confirm records sorted correctly
Created attachment 136426 [details] [review] Bug 30879: Allow biblionumber as sort option in Elasticsearch Repeat previous tests with Elasticsearch engine You will need to reindex and reset mappings to pickup the changes form the file
Created attachment 136427 [details] [review] Bug 30879: Handle index/sorting for UNIMARC Same as before, but test with UNIMARC setup
Created attachment 136428 [details] [review] Bug 30879: Add option to syspref and update display To test: 1 - Update ComponentSortField system preference and verify biblionumber is an option 2 - Repeat test plan from 30327 and confirm sorting with biblionumber works
Can't really test this at the moment since bug 30879 can't apply on master.
However, I'm pretty keen to test this Zebra indexing change, so I'll do a subtest. My test plan: 1. Apply patches for 30879 (w/o 30327 and nixing merge conflict due to missing 30327) 2. cp etc/zebradb/marc_defs/marc21/biblios/biblio-zebra-indexdefs.xsl /etc/koha/zebradb/marc_defs/marc21/biblios/biblio-zebra-indexdefs.xsl 3. restart_all 4. koha-rebuild-zebra -b -f -v kohadev 5. yaz-client unix:/var/run/koha/kohadev/bibliosocket 5b. find e 5c. sort 1=12 >i 6. show (through whole 64 record result set to confirm desc sort) -- It works 100% as you'd expect, so that's awesome. Great job, Nick! I'm so pleased how we got here using teamwork!
Since 30327 has been updated... My test plan: 1 - Apply patches 2 - cp etc/zebradb/marc_defs/marc21/biblios/biblio-zebra-indexdefs.xsl /etc/koha/zebradb/marc_defs/marc21/biblios/biblio-zebra-indexdefs.xsl 3a - restart_all 3b - koha-rebuild-zebra -b -f -v kohadev 3c - koha-upgrade-schema kohadev 4 - Browse to: http://localhost:8081/cgi-bin/koha/catalogue/search.pl?idx=kw&q=a&sort_by=biblionumber_dsc&count=20 5 - Confirm records sorted correctly 6 - Browse to http://localhost:8081/cgi-bin/koha/catalogue/search.pl?idx=kw&q=a&sort_by=biblionumber_asc&count=20 7 - Confirm records sorted correctly I don't know how to test the Elasticsearch or UNIMARC changes. I've changed "SearchEngine" to "Elasticsearch" and run "koha-elasticsearch --rebuild kohadev" which spat out the following: Cannot determine authority type for record: 1 at /kohadevbox/koha/Koha/SearchEngine/Elasticsearch.pm line 564. In the logs I'm seeing this error: [2022/06/24 02:25:42] [WARN] [Request] ** [http://es:9200]-[400] [query_shard_exception] No mapping found for [local-number__sort] in order to sort on, with: {"index":"koha_kohadev_biblios","index_uuid":"CVVpzugoRfqw7zsFft1pnQ"} Will need more specific instructions to test this one. I'll leave it as "Needs Signoff" rather than "Failed QA" in the event that someone else can test it correctly. -- The test plan for bug 30327 wasn't very specific in its instruction "Add some components to a record by control number or title depending on above", so I tried a lot of things... many which didn't work. Would've been nice to have more explicit steps there. Additional test plan: 1 - Update ComponentSortField system preference and verify biblionumber is an option 2 - Repeat test plan from 30327 and confirm sorting with biblionumber works This looks OK with Zebra.
Created attachment 136525 [details] [review] Bug 30879: Add biblionumber as a sorting option in MARC21 This patch updates the Local-Number indexing by adding a zeropad option to Zebra indexing and adding this to the mapping files It also updates C4/Search.pm to allow biblionumber as an option To test: 1 - Apply patches 2 - copy etc/zebradb/marc_defs/marc21/biblios/biblio-zebra-indexdefs.xsl to /etc/koha/zebradb/marc_defs/marc21/biblios/biblio-zebra-indexdefs.xsl 3 - Restart all, reindex zebra 4 - Browse to: http://localhost:8081/cgi-bin/koha/catalogue/search.pl?idx=kw&q=a&sort_by=biblionumber_dsc&count=20 5 - Confirm records sorted correctly 6 - Browse to http://localhost:8081/cgi-bin/koha/catalogue/search.pl?idx=kw&q=a&sort_by=biblionumber_asc&count=20 7 - Confirm records sorted correctly Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 136526 [details] [review] Bug 30879: Allow biblionumber as sort option in Elasticsearch Repeat previous tests with Elasticsearch engine You will need to reindex and reset mappings to pickup the changes form the file Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 136527 [details] [review] Bug 30879: Handle index/sorting for UNIMARC Same as before, but test with UNIMARC setup Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 136528 [details] [review] Bug 30879: Add option to syspref and update display To test: 1 - Update ComponentSortField system preference and verify biblionumber is an option 2 - Repeat test plan from 30327 and confirm sorting with biblionumber works Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Awesome teamwork here guys, thankyou very much. All tested and working as expected from my side.. signing off.
We need a rebase here please! Applying: Bug 30327: Add options for sorting components Using index info to reconstruct a base tree... M Koha/Biblio.pm M catalogue/detail.pl M installer/data/mysql/mandatory/sysprefs.sql M koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/cataloguing.pref M koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt M koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt M opac/opac-detail.pl M t/db_dependent/Koha/Biblio.t Falling back to patching base and 3-way merge... Auto-merging installer/data/mysql/mandatory/sysprefs.sql CONFLICT (content): Merge conflict in installer/data/mysql/mandatory/sysprefs.sql CONFLICT (add/add): Merge conflict in installer/data/mysql/atomicupdate/bug_30327_add_sortComponents_syspref.pl Auto-merging installer/data/mysql/atomicupdate/bug_30327_add_sortComponents_syspref.pl Auto-merging Koha/Biblio.pm CONFLICT (content): Merge conflict in Koha/Biblio.pm error: Failed to merge in the changes. Patch failed at 0001 Bug 30327: Add options for sorting components
Created attachment 136545 [details] [review] Bug 30879: Add biblionumber as a sorting option in MARC21 This patch updates the Local-Number indexing by adding a zeropad option to Zebra indexing and adding this to the mapping files It also updates C4/Search.pm to allow biblionumber as an option To test: 1 - Apply patches 2 - copy etc/zebradb/marc_defs/marc21/biblios/biblio-zebra-indexdefs.xsl to /etc/koha/zebradb/marc_defs/marc21/biblios/biblio-zebra-indexdefs.xsl 3 - Restart all, reindex zebra 4 - Browse to: http://localhost:8081/cgi-bin/koha/catalogue/search.pl?idx=kw&q=a&sort_by=biblionumber_dsc&count=20 5 - Confirm records sorted correctly 6 - Browse to http://localhost:8081/cgi-bin/koha/catalogue/search.pl?idx=kw&q=a&sort_by=biblionumber_asc&count=20 7 - Confirm records sorted correctly Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 136546 [details] [review] Bug 30879: Allow biblionumber as sort option in Elasticsearch Repeat previous tests with Elasticsearch engine You will need to reindex and reset mappings to pickup the changes form the file Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 136547 [details] [review] Bug 30879: Handle index/sorting for UNIMARC Same as before, but test with UNIMARC setup Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 136548 [details] [review] Bug 30879: Add option to syspref and update display To test: 1 - Update ComponentSortField system preference and verify biblionumber is an option 2 - Repeat test plan from 30327 and confirm sorting with biblionumber works Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Happy to add a motivational sign-off, but: I wonder how we can make sure this feature works correctly for updated installations. 1) We should add a note to the release notes about this requiring to reindex. 2) For Zebra the update will usually just work, but what about Elasticsearch? We recently noticed that bug 15851 doesn't work for people because of the missing cni index. I'd like to avoid that here. Could we add a database update for updating the mappings? 3) Unit tests?
No reply from Nick yet, I think we still need the release notes note, but dropping the other requirement from my side. I believe this is worth having.
(In reply to Katrin Fischer from comment #30) > No reply from Nick yet, I think we still need the release notes note, but > dropping the other requirement from my side. I believe this is worth having. I think I drafted and never posted We need to solve the problem of differing settings in a larger way - if we are letting people define their own mappings we really can't update them without feedback - bug 30836 is the place to figure that out
Pushed to master for 22.11. Nice work everyone, thanks!
Backported to 22.05.x for 22.05.06
depends on Bug30327 which i'm still waiting a valid patch for 21.11.x. Will only backport if I get an update on the dependancy. (don't know the status... on hold?)
So I've got an update from Indexdata... It turns out that we could've just patched our Perl code to the following: 375 elsif ( $sort eq "local-number_asc" ) { 376 $sort_by .= "1=12,4=109 <i "; 377 } 378 elsif ( $sort eq "local-number_dsc" ) { 379 $sort_by .= "1=12,4=109 >i "; 380 } I've already tested it using yaz-client and it worked a treat. It's an undocumented feature that Adam told me about in the Github issues: https://github.com/indexdata/idzebra/issues/37 I don't know if we want to undo the XSLT changes or keep them and do the Perl change as well. It might be worth doing if only to document what the change would be. It might be useful in other sorting contexts as well...
(In reply to Lucas Gass from comment #33) > Backported to 22.05.x for 22.05.06 I don't think that this has gone out yet, so we could actually still revert these changes too?
(In reply to Katrin Fischer from comment #29) > Happy to add a motivational sign-off, but: > > I wonder how we can make sure this feature works correctly for updated > installations. > > 1) We should add a note to the release notes about this requiring to reindex. If we replace the workaround using bug 31756, we won't need to require a reindex!
(In reply to David Cook from comment #37) > (In reply to Katrin Fischer from comment #29) > > Happy to add a motivational sign-off, but: > > > > I wonder how we can make sure this feature works correctly for updated > > installations. > > > > 1) We should add a note to the release notes about this requiring to reindex. > > If we replace the workaround using bug 31756, we won't need to require a > reindex! Actually, it looks like that may just be true for MARC21. It looks like "Local-number:s" was added for UNIMARC, so they'll probably still need to re-index...
(In reply to David Cook from comment #38) > (In reply to David Cook from comment #37) > > (In reply to Katrin Fischer from comment #29) > > > Happy to add a motivational sign-off, but: > > > > > > I wonder how we can make sure this feature works correctly for updated > > > installations. > > > > > > 1) We should add a note to the release notes about this requiring to reindex. > > > > If we replace the workaround using bug 31756, we won't need to require a > > reindex! > > Actually, it looks like that may just be true for MARC21. It looks like > "Local-number:s" was added for UNIMARC, so they'll probably still need to > re-index... It makes sense, but since this has been pushed to stable and we are already close to release we need to hurry up if we want to revert and replace with an alternative solution.