Summary: | C4::Matcher gets CCL parsing error if term contains ? (question mark) | ||
---|---|---|---|
Product: | Koha | Reporter: | David Cook <dcook> |
Component: | MARC Bibliographic record staging/import | Assignee: | David Cook <dcook> |
Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
Severity: | normal | ||
Priority: | P5 - low | CC: | brendan, f.demians, julian.maurice, olli-antti.kivilahti, patrick.robitaille |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | Small patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Attachments: |
Bug 15745 - C4::Matcher gets CCL parsing error if term contains ? (question mark)
Bug 15745 - C4::Matcher gets CCL parsing error if term contains ? (question mark) Bug 15745: C4::Matcher gets CCL parsing error if term contains ? (question mark) |
Description
David Cook
2016-02-05 05:02:35 UTC
Created attachment 47676 [details] [review] Bug 15745 - C4::Matcher gets CCL parsing error if term contains ? (question mark) I'll write a better description and test plan next week when I have more time... Comment on attachment 47676 [details] [review] Bug 15745 - C4::Matcher gets CCL parsing error if term contains ? (question mark) Review of attachment 47676 [details] [review]: ----------------------------------------------------------------- ::: C4/Matcher.pm @@ +658,5 @@ > else { > my $phr = C4::Context->preference('AggressiveMatchOnISBN') ? ',phr' : q{}; > $query = join( " or ", > + map { "$matchpoint->{'index'}$phr=\"$_\"" } @source_keys ); > + #NOTE: double-quote the values so you don't get a "Embedded truncation not supported" error when a term has a ? in it. What about the test case where you want to search for double quotes? Escaping is needed. Perhaps there is another way? (In reply to M. Tompsett from comment #3) > What about the test case where you want to search for double quotes? > Escaping is needed. Perhaps there is another way? I disagree. Why do you think escaping is needed? Can you point me to the particular test case or query in mind? Firstly, the values are coming from the MARC record, so I doubt there will be many double quotes in that data. Secondly, even if there are double quotes in the data, CCL2RPN takes them into account. We can see what happens if we double up our double quotes on the front/back of the query: Z> find id-other,st-urx="http://libris.kb.se/resource/bib/219553" Sent searchRequest. Received SearchResponse. Search was a success. Number of hits: 11, setno 31 SearchResult-1: term=http://libris.kb.se/resource/bib/219553 cnt=11 records returned: 0 Elapsed: 0.000886 BECOMES: @attrset Bib-1 @attr 1=9012 @attr 4=104 http://libris.kb.se/resource/bib/219553 Z> find id-other,st-urx=""http://libris.kb.se/resource/bib/219553"" Sent searchRequest. Received SearchResponse. Search was a success. Number of hits: 11, setno 32 SearchResult-1: term=http://libris.kb.se/resource/bib/219553 cnt=11 records returned: 0 Elapsed: 0.000792 BECOMES: @attrset Bib-1 @attr 1=9012 @attr 4=104 http://libris.kb.se/resource/bib/219553 Z> f ti,phr=""This is a test"" Sent searchRequest. Received SearchResponse. Search was a success. Number of hits: 4, setno 33 SearchResult-1: term=This cnt=4, term=is cnt=4, term=a cnt=4, term=test cnt=4 records returned: 0 Elapsed: 0.001079 BECOMES: @attrset Bib-1 @attr 1=4 @attr 4=1 "This is a test" Z> f ti,phr="This is a test" Sent searchRequest. Received SearchResponse. Search was a success. Number of hits: 4, setno 34 SearchResult-1: term=This cnt=4, term=is cnt=4, term=a cnt=4, term=test cnt=4 records returned: 0 Elapsed: 0.000961 BECOMES: @attrset Bib-1 @attr 1=4 @attr 4=1 "This is a test" -- I'm guessing you're referring to a case where the data contains a double quote and you're searching to match a double quote... but Zebra should strip out double quotes during indexing in most cases. It might not for URLs, but RFC3986 stipulates that you should be percent encoding double quotes as %22 anyway so that shouldn't be an issue. I'll look at an example of that in a moment... Interestingly enough, Zebra doesn't like it when you embed double quotes in a URL. http://prosentient.com.au?test="test" Becomes the following during indexing: http://prosentient.com.au?test=@test@ And the only way to retrieve it is to use the @ symbol instead of the double quotes in the query as well. Z> find uri,st-urx=http://prosentient.com.au?test="test" CCL ERROR: Embedded truncation not supported Z> find uri,st-urx="http://prosentient.com.au?test="test"" Sent searchRequest. Received SearchResponse. Search was a success. Number of hits: 0, setno 11 SearchResult-1: term=http://prosentient.com.au?test=test cnt=0 records returned: 0 Elapsed: 0.000750 Z> find uri,st-urx="http://prosentient.com.au?test=@test@" Sent searchRequest. Received SearchResponse. Search was a success. Number of hits: 1, setno 10 SearchResult-1: term=http://prosentient.com.au?test=@test@ cnt=1 records returned: 0 Elapsed: 0.000918 Now I'll try an example with normal word and phrase indexes... As expected, the double quote is removed during normalization during indexing and retrieval for the phrase index: 245 00 $a This is a "test" / $c by David Cook Z> find ti,phr="This is a "test"" Sent searchRequest. Received SearchResponse. Search was a success. Number of hits: 4, setno 4 SearchResult-1: term=This cnt=4, term=is cnt=4, term=a cnt=8, term=test cnt=4 records returned: 0 Elapsed: 0.001117 Z> show 1 245 00 $a This is a "test" / $c by David Cook Z> find ti,phr="This is a test" Sent searchRequest. Received SearchResponse. Search was a success. Number of hits: 4, setno 5 SearchResult-1: term=This cnt=4, term=is cnt=4, term=a cnt=8, term=test cnt=4 records returned: 0 Elapsed: 0.000991 Z> show 1 245 00 $a This is a "test" / $c by David Cook -- We can see this using "elements zebra::index" too: <index name="Title" type="p" seq="112">this is a test by david cook</index> We can see it with the word register too: <index name="Title" type="w" seq="113">this</index> <index name="Title" type="w" seq="114">is</index> <index name="Title" type="w" seq="115">a</index> <index name="Title" type="w" seq="116">test</index> <index name="Title" type="w" seq="117">by</index> <index name="Title" type="w" seq="118">david</index> <index name="Title" type="w" seq="119">cook</index> In other words, I think my patch still holds up. Created attachment 49255 [details] [review] Bug 15745 - C4::Matcher gets CCL parsing error if term contains ? (question mark) Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Also fixes ! and + Rebased to master Created attachment 49809 [details] [review] Bug 15745: C4::Matcher gets CCL parsing error if term contains ? (question mark) Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Also fixes ! and + Rebased to master Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar> It makes perfect sense and works as expected. This part of the code is too under-tested so no point requiring a regression test for such a simple change. Pushed to Master - Should be in the May 2016 release. Thanks! Patch pushed to 3.22.x, will be in 3.22.6 Pushed to 3.20.x, will be in 3.20.11. |