It looks like we are not sorting cn_sort as a raw field, but sorting the analyzed version. To recreate: 0 - Be using ES with Koha 1 - On records with single item, add callnumbers: VA65 E7 R63 1984 VA65 E7 T35 1990 VA65 E45 R67 1985 2 - Add public note 'shrimp' or something to make them easily searchable as a group 3 - Search for 'shrimp' 4 - Note E45 comes last, it should come first Problem may be here: 1122 sub _sort_field { 1123 my ($self, $f) = @_; 1124 1125 my $mappings = $self->get_elasticsearch_mappings(); 1126 my $textField = defined $mappings->{data}{properties}{$f}{type} && $mappings->{data}{properties}{$f}{type} eq 'text'; 1127 if (!defined $self->sort_fields()->{$f} || $self->sort_fields()->{$f}) { 1128 $f .= '__sort'; 1129 } else { 1130 # We need to add '.raw' to text fields without a sort field, 1131 # otherwise it'll sort based on the tokenised form. 1132 $f .= '.raw' if $textField; 1133 } 1134 return $f; 1135 } We need to enforce raw for cn-sort, or we need to index it unanalyzed
Noting that I recreated this workflow on Koha version 20.11. My results returned step 4 as the second result. So, in my testing, Koha returned results in the following order: VA65 E7 T35 1990, VA65 E45 R67 1985, VA65 E7 R63 1984.
We're having this problem on 21.05 and can be seen in our OPAC: https://keys.bywatersolutions.com/cgi-bin/koha/opac-search.pl?q=callnum%2Cphr%3Ava65&offset=80&sort_by=call_number_asc You'll see that VA65 E45 is still sorting *after* VA65 E7. It's a big problem for us to continue to have the LC call numbers not sorted correctly--the staff can't sort correctly for paging lists, for example, and being a closed stack library, this impacts us daily, with every service request, where they're operating from Carts, searches, lists, and often generating their own paging/pull lists from various parts of Koha.
Created attachment 136682 [details] [review] Bug 29632: Don't sort cn-sort numerically When defining our sort fields in we defined all as 'numeric' For other string containing numbers this is likely correct, however, for callnumbers it is not. e.g. E45 should sort before E7 This patch adds a new 'callnumber' type and deifnes this for cn-sort and adds to the field maping a sort without numeric set To test: 0 - Be using ES with Koha 1 - On records with single item, add callnumbers: VA65 E7 R63 1984 VA65 E7 T35 1990 VA65 E45 R67 1985 2 - Add public note 'shrimp' or something to make them easily searchable as a group 3 - Search for 'shrimp', sort by callnumber 4 - Note E45 comes last, it should come first 5 - Apply patch 6 - Reset ES mappings 7 - Reindex ES 8 - Repeat search 9 - Sorting should be correct when set to callnumber
Created attachment 136754 [details] [review] Bug 29632: Don't sort cn-sort numerically When defining our sort fields in we defined all as 'numeric' For other string containing numbers this is likely correct, however, for callnumbers it is not. e.g. E45 should sort before E7 This patch adds a new 'callnumber' type and deifnes this for cn-sort and adds to the field maping a sort without numeric set To test: 0 - Be using ES with Koha 1 - On records with single item, add callnumbers: VA65 E7 R63 1984 VA65 E7 T35 1990 VA65 E45 R67 1985 2 - Add public note 'shrimp' or something to make them easily searchable as a group 3 - Search for 'shrimp', sort by callnumber 4 - Note E45 comes last, it should come first 5 - Apply patch 6 - Reset ES mappings 7 - Reindex ES 8 - Repeat search 9 - Sorting should be correct when set to callnumber Signed-off-by: David Nind <david@davidnind.com>
Testing notes (using koha-testing-docker): - After applying the patch (step 5) you need to run a database update (updatedatabase0
Created attachment 137182 [details] [review] Bug 29632: Don't sort cn-sort numerically When defining our sort fields in we defined all as 'numeric' For other string containing numbers this is likely correct, however, for callnumbers it is not. e.g. E45 should sort before E7 This patch adds a new 'callnumber' type and deifnes this for cn-sort and adds to the field maping a sort without numeric set To test: 0 - Be using ES with Koha 1 - On records with single item, add callnumbers: VA65 E7 R63 1984 VA65 E7 T35 1990 VA65 E45 R67 1985 2 - Add public note 'shrimp' or something to make them easily searchable as a group 3 - Search for 'shrimp', sort by callnumber 4 - Note E45 comes last, it should come first 5 - Apply patch 6 - Reset ES mappings 7 - Reindex ES 8 - Repeat search 9 - Sorting should be correct when set to callnumber Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Michal Urban <michalurban177@gmail.com>
Looks like we're missing a unit test here for the Koha/SearchEngine/Elasticsearch.pm change.
Created attachment 137539 [details] [review] Bug 29632: Unit tests This patch adds unit tests, as well as changing existing test to use a mock and read the data as passed in tests, rather than relying on what exists in the DB
Created attachment 137605 [details] [review] Bug 29632: Don't sort cn-sort numerically When defining our sort fields in we defined all as 'numeric' For other string containing numbers this is likely correct, however, for callnumbers it is not. e.g. E45 should sort before E7 This patch adds a new 'callnumber' type and deifnes this for cn-sort and adds to the field maping a sort without numeric set To test: 0 - Be using ES with Koha 1 - On records with single item, add callnumbers: VA65 E7 R63 1984 VA65 E7 T35 1990 VA65 E45 R67 1985 2 - Add public note 'shrimp' or something to make them easily searchable as a group 3 - Search for 'shrimp', sort by callnumber 4 - Note E45 comes last, it should come first 5 - Apply patch 6 - Reset ES mappings 7 - Reindex ES 8 - Repeat search 9 - Sorting should be correct when set to callnumber Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Michal Urban <michalurban177@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 137606 [details] [review] Bug 29632: Unit tests This patch adds unit tests, as well as changing existing test to use a mock and read the data as passed in tests, rather than relying on what exists in the DB Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Thanks Nick, Passing QA now :)
Shouldn't we add the index to the configuration on the update?
Pushed to master for 22.11. Nice work everyone, thanks!
Created attachment 137823 [details] [review] Bug 29632: (QA follow-up) Add ENUM value to kohastructure.sql :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 138611 [details] [review] Bug 29632: (QA follow-up) Fix COMMENT discrepancy on upgrade Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Backported to 22.05.x for 22.05.05
conflicts in templates when trying to apply on 21.11. Provide a backport if needed.
Created attachment 139827 [details] [review] Bug 29632: [21.11.X] Don't sort cn-sort numerically When defining our sort fields in we defined all as 'numeric' For other string containing numbers this is likely correct, however, for callnumbers it is not. e.g. E45 should sort before E7 This patch adds a new 'callnumber' type and deifnes this for cn-sort and adds to the field maping a sort without numeric set To test: 0 - Be using ES with Koha 1 - On records with single item, add callnumbers: VA65 E7 R63 1984 VA65 E7 T35 1990 VA65 E45 R67 1985 2 - Add public note 'shrimp' or something to make them easily searchable as a group 3 - Search for 'shrimp', sort by callnumber 4 - Note E45 comes last, it should come first 5 - Apply patch 6 - Reset ES mappings 7 - Reindex ES 8 - Repeat search 9 - Sorting should be correct when set to callnumber Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Michal Urban <michalurban177@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Bug 29632: Unit tests This patch adds unit tests, as well as changing existing test to use a mock and read the data as passed in tests, rather than relying on what exists in the DB Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Bug 29632: (QA follow-up) Add ENUM value to kohastructure.sql :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Bug 29632: (QA follow-up) Fix COMMENT discrepancy on upgrade Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
(In reply to Arthur Suzuki from comment #17) > conflicts in templates when trying to apply on 21.11. > Provide a backport if needed. Patch supplied