Bug 15745 - C4::Matcher gets CCL parsing error if term contains ? (question mark)
Summary: C4::Matcher gets CCL parsing error if term contains ? (question mark)
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: MARC Bibliographic record staging/import (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: David Cook
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-02-05 05:02 UTC by David Cook
Modified: 2017-09-08 19:49 UTC (History)
5 users (show)

See Also:
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) (1.89 KB, patch)
2016-02-05 06:16 UTC, David Cook
Details | Diff | Splinter Review
Bug 15745 - C4::Matcher gets CCL parsing error if term contains ? (question mark) (1.51 KB, patch)
2016-03-17 12:26 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 15745: C4::Matcher gets CCL parsing error if term contains ? (question mark) (1.71 KB, patch)
2016-04-01 17:51 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description David Cook 2016-02-05 05:02:35 UTC
I was using C4::Matcher via the record matching rules during an import, and I kept getting the following error: "CCL parsing error (10014) Embedded truncation not supported ZOOM".

Query:
id-other,st-urx=http://www.skovdenyheter.se?p=38004

I wondered why it was trying to do "embedded truncation" when it shouldn't have been doing any truncation by default... so I thought about wrapping the whole term in double quotation marks. 

Sure enough, the following worked and didn't produce an error:

id-other,st-urx="http://www.skovdenyheter.se?p=38004"

You can notice this same problem elsewhere when using CCL. Most noticeably directly with yaz-client:

Z> find ti=test?
CCL ERROR: Right truncation not supported

Z> find ti="test?"
Sent searchRequest.
Received SearchResponse.
Search was a success.
Number of hits: 4, setno 2
SearchResult-1: term=test cnt=4
records returned: 0
Elapsed: 0.000816

Also consider the following:

Z> find kw,rt=tes*
CCL ERROR: Right truncation not supported

Z> find kw="tes*"
Sent searchRequest.
Received SearchResponse.
Search was a success.
Number of hits: 0, setno 23
SearchResult-1: term=tes cnt=0
records returned: 0
Elapsed: 0.000679

Z> find kw,rt="tes*"
Sent searchRequest.
Received SearchResponse.
Search was a success.
Number of hits: 6, setno 24
SearchResult-1: term=tes cnt=6
records returned: 0
Elapsed: 0.001049
Comment 1 David Cook 2016-02-05 06:16:11 UTC Comment hidden (obsolete)
Comment 2 David Cook 2016-02-05 06:16:33 UTC
I'll write a better description and test plan next week when I have more time...
Comment 3 Mark Tompsett 2016-02-12 20:29:10 UTC
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?
Comment 4 David Cook 2016-02-14 22:50:12 UTC
(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...
Comment 5 David Cook 2016-02-14 23:05:32 UTC
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...
Comment 6 David Cook 2016-02-14 23:14:11 UTC
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>
Comment 7 David Cook 2016-02-14 23:14:22 UTC
In other words, I think my patch still holds up.
Comment 8 Olli-Antti Kivilahti 2016-03-17 12:26:05 UTC Comment hidden (obsolete)
Comment 9 Tomás Cohen Arazi 2016-04-01 17:51:06 UTC
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.
Comment 10 Brendan Gallagher 2016-04-01 19:13:23 UTC
Pushed to Master - Should be in the May 2016 release.  Thanks!
Comment 11 Julian Maurice 2016-04-06 14:39:18 UTC
Patch pushed to 3.22.x, will be in 3.22.6
Comment 12 Frédéric Demians 2016-04-27 15:39:58 UTC
Pushed to 3.20.x, will be in 3.20.11.