Created attachment 57658 [details] 490 $a Harry Bosch mysteries; $v 3 -- whitespace after semicolon in 490$a There are two marc files attached; 'a.marc' and 'b.marc' Each contains the string "Harry Bosch mysteries;" in 490$a. In a.marc, this is followed by a trailing space character, in b.marc, it is not. If you import both records and do a keyword search for 'Harry Bosch mysteries', you will see that Harry Bosch mysteries shows twice in the Series facet.
Created attachment 57659 [details] 490 $a Harry Bosch mysteries; $v 13 -- no space after semicolon
Created attachment 68897 [details] [review] Bug 17661 - Ending punctuation causes duplicate facets WIP
Should expand this patch to cover whitespace, also need ES coverage, also better bug title
Correcting this display problem would really improve user experience. Currently users need to discover by trial and error that the two entries "Brunkus, Denise," and Brunkus, Denise." run the same search. Our librarians assume they run different searches, and it makes them distrust the catalog. This affects all facets that come from MARC: authors, series, places, topics.
(In reply to Sarah Cornell from comment #4) > Correcting this display problem would really improve user experience. > > Currently users need to discover by trial and error that the two entries > "Brunkus, Denise," and Brunkus, Denise." run the same search. Our > librarians assume they run different searches, and it makes them distrust > the catalog. > > This affects all facets that come from MARC: authors, series, places, topics. I completely agree with Sarah. The duplication pushes other authors, series, places, topics, etc. further down the list and users have to click "Show Me More" to see them.
+1
*** Bug 13231 has been marked as a duplicate of this bug. ***
My library would also be glad to see a resolution for this bug. The quality of our OPAC is part of the customer service we offer to our patrons. Misleading duplication in the OPAC because of how the 490 is read negatively impacts the patron experience rather than helping us make our OPAC as effective as possible. Thank you!
Hi Nick, I thouhgt it would be very nice to revive this bug and looked at your WIP patch. I think you were on the right track there, but I'd like to suggest some changes to the regex. You got: $data =~ s/[.\-,]$//; I've tested with authors and made some changes on my data: Without patch: Aristophanes, Aristophanes. Beckett, Samuel, Celan, Paul. Date, C. J. With patch: Aristophanes. Beckett Samuel, Celan Paul. Date C. J. 1) Your regex always removes the first occurence, but we need the last. 2) . is not removed as it's not escaped in the regex (I think). 2) In case of "Date, C. J." we don't want the last, when it's preceded by a capital letter (very likely to be an initial).
Created attachment 106637 [details] [review] Bug 17661 - Ending punctuation causes duplicate facets To test: 1 - Do search and pull up some records 2 - Edit some of the records to have authors like: Date, C.J. Date, C.j. Date, C.J . 3 - Edit the records to have some series statments like: 830 $aDate, C.J. ;$v5 830 $aDate, C.J. ; $v5 830 $aDate, C.J.; $v5 4 - Search again and note you have 3 facets each for authr and series 5 - Apply patch 6 - Repeat 7 - Now you get 2 facets for author, period not removed when following Upper case immediately, is otherwise 8 - Now you should have a single series facet 9 - Switch search engine to ES (index before applying patch) 10 - Note facets are separate again 11 - Reset mappings and reindex perl misc/search_tools/rebuild_elasticsearch -v -r 12 - Repeat search, facets combined as above
(In reply to Katrin Fischer from comment #9) > 1) Your regex always removes the first occurence, but we need the last. > 2) . is not removed as it's not escaped in the regex (I think). > 2) In case of "Date, C. J." we don't want the last, when it's preceded by a > capital letter (very likely to be an initial). 1 - I had two different regexes, one was wrong 2 - period does not need to be escaped 3 - Done I added eliminating spaces on either side of punctuation as well Tested and played here to find cases: https://regex101.com/ This is the base right now: \s*(?<![A-Z])[.\-,;]\s*$ Let me know how that works
Created attachment 108742 [details] [review] Bug 17661 - Ending punctuation causes duplicate facets The current code for facets doesn't pull strip ending punctuation from facets This causes duplicate facets for terms that should be combined Sometimes series can have different punctuation depending on the field they are in Author initials punctuation should be preserved To test: 1 - Do search and pull up some records 2 - Edit some of the records to have authors like: Date, C.J. Date, C.j. Date, C.J . 3 - Edit the records to have some series statments like: 830 $aDate, C.J. ;$v5 830 $aDate, C.J. ; $v5 830 $aDate, C.J.; $v5 4 - Add some 490s to the record with first indicator 1 and series like: You wouldn't want to-- You wouldn't want to You wouldn't want to.. 5 - Search again and note you have 3 facets each for author and series 6 - Apply patch 7 - Repeat 8 - Now you get 2 facets for author, period not removed when following Upper case immediately, is otherwise 9 - Now you should have a single series facet 10 - Switch search engine to ES (index before applying patch) 11 - Note facets are separate again 12 - Reset mappings and reindex perl misc/search_tools/rebuild_elasticsearch -v -r 13 - Repeat search, facets combined as above
Created attachment 109117 [details] [review] Bug 17661 - Ending punctuation causes duplicate facets The current code for facets doesn't pull strip ending punctuation from facets This causes duplicate facets for terms that should be combined Sometimes series can have different punctuation depending on the field they are in Author initials punctuation should be preserved To test: 1 - Do search and pull up some records 2 - Edit some of the records to have authors like: Date, C.J. Date, C.j. Date, C.J . 3 - Edit the records to have some series statments like: 830 $aDate, C.J. ;$v5 830 $aDate, C.J. ; $v5 830 $aDate, C.J.; $v5 4 - Add some 490s to the record with first indicator 1 and series like: You wouldn't want to-- You wouldn't want to You wouldn't want to.. 5 - Search again and note you have 3 facets each for author and series 6 - Apply patch 7 - Repeat 8 - Now you get 2 facets for author, period not removed when following Upper case immediately, is otherwise 9 - Now you should have a single series facet 10 - Switch search engine to ES (index before applying patch) 11 - Note facets are separate again 12 - Reset mappings and reindex perl misc/search_tools/rebuild_elasticsearch -v -r 13 - Repeat search, facets combined as above Signed-off-by: Sarah Cornell <sbcornell@cityofportsmouth.com>
This is a really great improvement and works really well with our sample data. The only thing that I've been wondering about: seeing the A-Z as part of the regex: What will happen with non-latin and letters with diacritics? At the moment Ж. (Cyrillic) will lose the '.' and not be recognized as an initial. Is there a way we could make this work maybe? Could we check for the characters before being a space followed by a single character maybe?
I found some information here, that is promising: https://perldoc.perl.org/perlunicode.html For instance, \p{Uppercase} matches any single character with the Unicode "Uppercase" property, while \p{L} matches any character with a General_Category of "L" (letter) property So maybe we could just switch out [A-Z] for the \p{Uppercase}.
This is interesting. I'm not surprised that "_get_facets_data_from_record" works in an undesirable way, but I'm surprised that this would be necessary for "_get_facet_from_result_set" using Zebra facets, since Zebra returns the data in a normalized format. For example: Z> elements zebra::facet::author:p Z> show 1 Sent presentRequest (1+1). Records: 1 Record type: XML <record xmlns="http://www.indexdata.com/zebra/"> <facet type="p" index="author"> <term coccur="21" occur="11">berryman faye</term> <term coccur="11" occur="11">dale rae</term> <term coccur="11" occur="11">dale rae 1945</term> <term coccur="11" occur="11">fitzroy programs</term> <term coccur="12" occur="11">o carroll philip</term> <term coccur="10" occur="10">o carroll philip 1945</term> <term coccur="6" occur="6">powell jonathon</term> <term coccur="6" occur="6">powell jonathon illustrator</term> <term coccur="9" occur="6">reynolds kate e</term> <term coccur="12" occur="6">shapiro lawrence e</term> <term coccur="6" occur="5">carter emily</term> <term coccur="5" occur="3">caramagna joe</term> <term coccur="6" occur="3">schaefer charles e</term> <term coccur="4" occur="2">digeronimo theresa foy</term> <term coccur="4" occur="2">hughes edward e</term> <term coccur="4" occur="2">kazdin alan e</term> <term coccur="4" occur="2">reeve christine e</term> <term coccur="4" occur="2">renton n e nicholas edwin 1931</term> <term coccur="4" occur="2">snell martha e</term> <term coccur="4" occur="1">attwood tony</term> </facet> </record> nextResultSetPosition = 2 Elapsed: 0.030227 Now in this case there are some apparent "duplicates" where the author's date is included in some records but not others. I would argue that is an authority data issue though, since "John Smith", "John Smith 1945", and "John Smith 1995" are all different authors. I don't know how Elasticsearch handles its facets, so I can't comment there.
Yet, if I look at http://localhost:8081/cgi-bin/koha/catalogue/search.pl?q=test I'm not seeing data that looks like that facet data I showed before (which was from a different catalogue). I'll dig a bit more...
Ah, that example was using CHR indexing.
Ok still using CHR indexing but using the unnormalized 0 register... Previous results used "elements zebra::facet::author:p" These results use "elements zebra::facet::au:0" <record xmlns="http://www.indexdata.com/zebra/"> <facet type="0" index="au"> <term coccur="10" occur="10">O'Carroll, Philip,</term> <term coccur="9" occur="9">Berryman, Faye</term> <term coccur="7" occur="7">Dale, Rae,</term> <term coccur="6" occur="6">Powell, Jonathon,</term> <term coccur="4" occur="4">Dale, Rae</term> <term coccur="4" occur="4">Shapiro, Lawrence E</term> <term coccur="3" occur="3">Reynolds, Kate E.,</term> <term coccur="2" occur="2">Anderson, Vicki</term> <term coccur="2" occur="2">Betts, Dion E.</term> <term coccur="2" occur="2">Brennan, Tom</term> <term coccur="2" occur="2">Caramagna, Joe</term> <term coccur="2" occur="2">Foxx, Richard M</term> <term coccur="2" occur="2">Gutstein, Steven E</term> <term coccur="2" occur="2">Medlen, Joan E. Guthrie.</term> <term coccur="2" occur="2">Renton, N. E. (Nicholas Edwin), 1931</term> <term coccur="2" occur="2">Reynolds, Kate E.</term> <term coccur="2" occur="2">Rosemann, Bill</term> <term coccur="2" occur="2">Schaefer, Charles E</term> <term coccur="2" occur="2">Snell, Martha E.</term> <term coccur="2" occur="1">Attwood, Tony</term> </facet> </record> I see that duplication now: <term coccur="7" occur="7">Dale, Rae,</term> <term coccur="4" occur="4">Dale, Rae</term>
One downside of post-processing facet results is that your facet list length will vary depending on its contents... It could be more reliable to handle this issue at index-time rather than search-time. Since we're using DOM indexing with Zebra, we have a lot of power at index-time (beyond the use of CHR and ICU mechanisms) which could handle whitespace and punctuation for facet purposes. That being said... post-processing is probably easiest to have the most consistent behaviour across 3 different facet providers...
Created attachment 109242 [details] [review] Bug 17661: (follow-up) Update regex to support Unicode characters Rather than limiting initials to [A-Z] we should test for a broad range of uppercase letters. The ES/Zebra changes are slightly different because of Perl vs Java regex conventions. POerl may support either, but I found 'Uppercase' to be a bit more explicit More info here: https://perldoc.perl.org/perlunicode.html TO test: Same plan as before but use Ж. as the ending initial Confirm the period is preserved and other punctuation removed
Created attachment 109259 [details] [review] Bug 17661 - Ending punctuation causes duplicate facets The current code for facets doesn't pull strip ending punctuation from facets This causes duplicate facets for terms that should be combined Sometimes series can have different punctuation depending on the field they are in Author initials punctuation should be preserved To test: 1 - Do search and pull up some records 2 - Edit some of the records to have authors like: Date, C.J. Date, C.j. Date, C.J . 3 - Edit the records to have some series statments like: 830 $aDate, C.J. ;$v5 830 $aDate, C.J. ; $v5 830 $aDate, C.J.; $v5 4 - Add some 490s to the record with first indicator 1 and series like: You wouldn't want to-- You wouldn't want to You wouldn't want to.. 5 - Search again and note you have 3 facets each for author and series 6 - Apply patch 7 - Repeat 8 - Now you get 2 facets for author, period not removed when following Upper case immediately, is otherwise 9 - Now you should have a single series facet 10 - Switch search engine to ES (index before applying patch) 11 - Note facets are separate again 12 - Reset mappings and reindex perl misc/search_tools/rebuild_elasticsearch -v -r 13 - Repeat search, facets combined as above Signed-off-by: Sarah Cornell <sbcornell@cityofportsmouth.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 109260 [details] [review] Bug 17661: (follow-up) Update regex to support Unicode characters Rather than limiting initials to [A-Z] we should test for a broad range of uppercase letters. The ES/Zebra changes are slightly different because of Perl vs Java regex conventions. POerl may support either, but I found 'Uppercase' to be a bit more explicit More info here: https://perldoc.perl.org/perlunicode.html TO test: Same plan as before but use Ж. as the ending initial Confirm the period is preserved and other punctuation removed Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 109368 [details] [review] Bug 17661: Trivial simplification using += operator
Pushed to master for 20.11, thanks to everybody involved!
backported to 20.05.x for 20.05.04
backported to 19.11.x for 19.11.10
Not backported to oldoldstable (19.05.x). Feel free to ask if it's needed.