Bug 25900 - QueryWeightFields creates unnecessary whitespace
Summary: QueryWeightFields creates unnecessary whitespace
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching - Zebra (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal with 8 votes (vote)
Assignee: Fridolin Somers
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-06-30 11:50 UTC by Fridolin Somers
Modified: 2023-12-10 09:34 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 25900: trim spaces in search with Zebra and QueryWeightFields (1.56 KB, patch)
2020-06-30 13:20 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 25900: trim spaces in search with Zebra and QueryWeightFields (1.35 KB, patch)
2020-12-07 09:19 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 25900: trim spaces in search with Zebra and QueryWeightFields (1.40 KB, patch)
2021-06-01 10:27 UTC, Peter Vashchuk
Details | Diff | Splinter Review
Bug 25900: trim spaces in search with Zebra and QueryWeightFields (1.46 KB, patch)
2021-06-03 10:28 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Fridolin Somers 2020-06-30 11:50:20 UTC
Search with Zebra and system preference QueryWeightFields on builds a query with surrounding double quotes : q=tree => q="tree". Therefore existing double quotes are replaced by a space : q="tree" => q=" tree ".
Looks like the leading space breaks the search, 0 results.
Experimented on Ubuntu Xenial and Bionic with Indexdata repository so Zebra version is 2.1.4. It might be specific to this version.

You can reproduce with direct CCL query (regardless QueryWeightFields) :
https://demo.biblibre.com/cgi-bin/koha/opac-search.pl?idx=&q=ccl%3Dti%2Cphr%3D%22centre+arch%C3%A9ologique%22
https://demo.biblibre.com/cgi-bin/koha/opac-search.pl?idx=&q=ccl%3Dti%2Cphr%3D%22+centre+arch%C3%A9ologique%22
Comment 1 Fridolin Somers 2020-06-30 13:20:20 UTC Comment hidden (obsolete)
Comment 2 Katrin Fischer 2020-10-19 15:53:21 UTC
Hi Fridolin, I just tested this on master without your patch and it search works fine for me - could another pref be at play here?

I tried simple searches on staff interface like: "simon of" 

Using the standard MARC21 sample data.
Comment 3 Fridolin Somers 2020-10-20 09:32:37 UTC
Which Zebra version please ?
Comment 4 Katrin Fischer 2020-10-20 19:41:37 UTC
kohadevbox:
Zebra version: 	Zebra 2.0.59 (C) 1
Comment 5 Fridolin Somers 2020-10-21 08:40:27 UTC
Oh indeed, even Debian Buster has Zebra 2.0.59.

You can use indexdata repo to upgrade the version : http://ftp.indexdata.dk/debian/
Comment 6 David Cook 2020-10-21 23:17:33 UTC
Frido: Are you using CHR or ICU with Zebra?
Comment 7 Katrin Fischer 2020-10-22 00:36:31 UTC
I've reset the status, so maybe someone with a newer version can give it a go.
Comment 8 Fridolin Somers 2020-10-23 09:54:50 UTC
(In reply to David Cook from comment #6)
> Frido: Are you using CHR or ICU with Zebra?

Always ICU
Comment 9 David Cook 2020-10-25 23:57:32 UTC
(In reply to Fridolin SOMERS from comment #8)
> (In reply to David Cook from comment #6)
> > Frido: Are you using CHR or ICU with Zebra?
> 
> Always ICU

That's what I thought.

I suspect that Katrin and others are probably using CHR, so that might also explain the discrepancy in results.

I haven't looked into the patches though.
Comment 10 David Cook 2020-12-03 04:57:36 UTC
Using koha-testing-docker w/o patch using CHR in /etc/koha/zebradb/etc/default.idx:

QueryAutoTruncate:
Query: test
Request: @attrset Bib-1 @attr 1=1016 @attr 4=6 @attr 5=1 test

Query: "test"
Request: @attrset Bib-1 @attr 1=1016 @attr 4=6 @attr 5=1 @attr 2=102 test

QueryWeightFields:
Query: test
Request: @attrset Bib-1 @or @or @or @or @or @or @attr 1=36 @attr 4=1 @attr 6=3 @attr 9=32 @attr 2=102 test @attr 1=4 @attr 4=1 @attr 6=3 @attr 9=28 @attr 2=102 test @attr 1=
    36 @attr 4=1 @attr 9=26 @attr 2=102 test @attr 1=4 @attr 4=6 @attr 9=24 @attr 2=102 test @attr 4=6 @attr 5=103 @attr 9=16 @attr 2=102 test @attr 4=6 @attr 5=1 @attr 9=14 @attr 2=102 "test? " @attr 4=6 @attr 9=14 @attr 2=102 test

Query: "test"
Request: @attrset Bib-1 @or @or @or @or @or @or @attr 1=36 @attr 4=1 @attr 6=3 @attr 9=32 @attr 2=102 " test " @attr 1=4 @attr 4=1 @attr 6=3 @attr 9=28 @attr 2=102 " test " @att
r 1=36 @attr 4=1 @attr 9=26 @attr 2=102 " test " @attr 1=4 @attr 4=6 @attr 9=24 @attr 2=102 " test " @attr 4=6 @attr 5=103 @attr 9=16 @attr 2=102 " test " @attr 4=6 @attr 5=1 @attr 9=14 @attr 2=102 "test? " @attr 4=6 @attr 9=14 @attr 2=1
02 " test "
Comment 11 David Cook 2020-12-03 05:25:22 UTC
Using koha-testing-docker w/o patch using ICU in
/etc/koha/zebradb/etc/default.idx:

For "index w", replace "charmap word-phrase-utf.chr" with "icuchain words-icu.xml"

For "index p", replace "charmap word-phrase-utf.chr" with "icuchain phrases-icu.xml"

koha-rebuild-zebra -f -b -v kohadev

QueryAutoTruncate:
Query: test
Request: Search biblios OK 3 1 1+0 RPN @attrset Bib-1 @attr 1=1016 @attr 4=6 @attr 5=1 test

Query: "test"
Request: Search biblios OK 3 1 1+0 RPN @attrset Bib-1 @attr 1=1016 @attr 4=6 @attr 5=1 test

QueryWeightFields:
Query: test
Request: Search biblios ERROR 120 1 1+0 RPN @attrset Bib-1 @or @or @or @or @or @or @attr 1=36 @attr 4=1 @attr 6=3 @attr 9=32 @attr 2=102 test @attr 1=4 @attr 4=1 @attr 6=3 @attr 9=28 @attr 2=102 test @attr 1=3
6 @attr 4=1 @attr 9=26 @attr 2=102 test @attr 1=4 @attr 4=6 @attr 9=24 @attr 2=102 test @attr 4=6 @attr 5=103 @attr 9=16 @attr 2=102 test @attr 4=6 @attr 5=1 @attr 9=14 @attr 2=102 "test? " @attr 4=6 @attr 9=14 @attr 2=102 test

Query: "test"
Request: Search biblios ERROR 120 1 1+0 RPN @attrset Bib-1 @or @or @or @or @or @or @attr 1=36 @attr 4=1 @attr 6=3 @attr 9=32 @attr 2=102 " test " @attr 1=4 @attr 4=1 @attr 6=3 @attr 9=28 @attr 2=102 " test " @
attr 1=36 @attr 4=1 @attr 9=26 @attr 2=102 " test " @attr 1=4 @attr 4=6 @attr 9=24 @attr 2=102 " test " @attr 4=6 @attr 5=103 @attr 9=16 @attr 2=102 " test " @attr 4=6 @attr 5=1 @attr 9=14 @attr 2=102 "test? " @attr 4=6 @attr 9=14 @attr
2=102 " test "

(Via yaz-client this query yields: [120] Unsupported Truncation attribute -- v2 addinfo '103')

--

Actually, it doesn't look like this problem is related to double quotes at all.

Interesting... if I disable QueryFuzzy it will work:

Query: test
Request: Search biblios OK 3 1 1+0 RPN @attrset Bib-1 @or @or @or @or @or @attr 1=36 @attr 4=1 @attr 6=3 @attr 9=32 @attr 2=102 test @attr 1=4 @attr 4=1 @attr 6=3 @attr 9=28 @attr 2=102 test @attr 1=36 @attr 4
=1 @attr 9=26 @attr 2=102 test @attr 1=4 @attr 4=6 @attr 9=24 @attr 2=102 test @attr 4=6 @attr 5=1 @attr 9=14 @attr 2=102 "test? " @attr 4=6 @attr 9=14 @attr 2=102 test

Query: "test"
Request: 
Search biblios OK 3 1 1+0 RPN @attrset Bib-1 @or @or @or @or @or @attr 1=36 @attr 4=1 @attr 6=3 @attr 9=32 @attr 2=102 " test " @attr 1=4 @attr 4=1 @attr 6=3 @attr 9=28 @attr 2=102 " test " @attr 1=36
 @attr 4=1 @attr 9=26 @attr 2=102 " test " @attr 1=4 @attr 4=6 @attr 9=24 @attr 2=102 " test " @attr 4=6 @attr 5=1 @attr 9=14 @attr 2=102 "test? " @attr 4=6 @attr 9=14 @attr 2=102 " test "

--

If I re-enable QueryFuzzy *but* DISABLE QueryStemming it causes catastrophic problems:

Query: test
Request: Search biblios ERROR 120 1 1+0 RPN @attrset Bib-1 @or @or @or @or @or @attr 1=36 @attr 4=1 @attr 6=3 @attr 9=32 @attr 2=102 test @attr 1=4 @attr 4=1 @attr 6=3 @attr 9=28 @attr 2=102 test @attr 1=36 @a
ttr 4=1 @attr 9=26 @attr 2=102 test @attr 1=4 @attr 4=6 @attr 9=24 @attr 2=102 test @attr 4=6 @attr 5=103 @attr 9=16 @attr 2=102 test @attr 4=6 @attr 9=14 @attr 2=102 test

Query: "test"
Request:  Search biblios ERROR 120 1 1+0 RPN @attrset Bib-1 @or @or @or @or @or @attr 1=36 @attr 4=1 @attr 6=3 @attr 9=32 @attr 2=102 " test " @attr 1=4 @attr 4=1 @attr 6=3 @attr 9=28 @attr 2=102 " test " @att
r 1=36 @attr 4=1 @attr 9=26 @attr 2=102 " test " @attr 1=4 @attr 4=6 @attr 9=24 @attr 2=102 " test " @attr 4=6 @attr 5=103 @attr 9=16 @attr 2=102 " test " @attr 4=6 @attr 9=14 @attr 2=102 " test "

(Via yaz-client this query yield: [120] Unsupported Truncation attribute -- v2 addinfo '103')
Comment 12 David Cook 2020-12-03 05:26:11 UTC
(In reply to David Cook from comment #10)
> Using koha-testing-docker w/o patch using CHR in
> /etc/koha/zebradb/etc/default.idx:

Note I have a typo in the QueryAutoTruncate section. By accident, a @attr 2=102 snuck in there.
Comment 13 David Cook 2020-12-03 05:29:24 UTC
Hmm I've just noticed that I didn't have "UseICU" turned on... but that doesn't look relevant.
Comment 14 David Cook 2020-12-03 05:55:07 UTC
I'm tempted to mark this as a duplicate of Bug 9715
Comment 15 Fridolin Somers 2020-12-03 14:18:15 UTC
(In reply to David Cook from comment #13)
> Hmm I've just noticed that I didn't have "UseICU" turned on... but that
> doesn't look relevant.

Indeed, this is only used by XSLT links
Comment 16 Fridolin Somers 2020-12-03 14:21:37 UTC
(In reply to Fridolin Somers from comment #5)
> Oh indeed, even Debian Buster has Zebra 2.0.59.
> 
> You can use indexdata repo to upgrade the version :
> http://ftp.indexdata.dk/debian/

koha-testing-docker uses Debian version no ?

This change is harmless I'd say.
Comment 17 David Cook 2020-12-04 02:17:02 UTC
(In reply to Fridolin Somers from comment #16)
> This change is harmless I'd say.

It's harmless but the description is inaccurate. If you renamed this issue to "QueryWeightFields creates unnecessary whitespace", then I would happily test again and probably sign off. 

But it wouldn't be fixing any functional bug I think.
Comment 18 Fridolin Somers 2020-12-07 09:15:13 UTC
(In reply to David Cook from comment #17)
> (In reply to Fridolin Somers from comment #16)
> > This change is harmless I'd say.
> 
> It's harmless but the description is inaccurate. If you renamed this issue
> to "QueryWeightFields creates unnecessary whitespace", then I would happily
> test again and probably sign off. 
> 
> But it wouldn't be fixing any functional bug I think.

Sur, good idea
Comment 19 Fridolin Somers 2020-12-07 09:19:21 UTC
Created attachment 114228 [details] [review]
Bug 25900: trim spaces in search with Zebra and QueryWeightFields

Search with Zebra and system preference QueryWeightFields on builds a query with surrounding double quotes : q=tree => q="tree". Therefore existing double quotes are replaced by a space : q="tree" => q=" tree ".

Patch corrects by adding trim on operand after transforming quotes into spaces.

Test plan :
1) Use Zebra searchengine and enable system preference QueryWeightFields
2) Go to advanced search
3) Search on two words from a title, surrounded by double quotes. Like "Order of the Phoenix"
4) Check you get same results with an without patch
Comment 20 Peter Vashchuk 2021-06-01 10:27:29 UTC
Created attachment 121529 [details] [review]
Bug 25900: trim spaces in search with Zebra and QueryWeightFields

Search with Zebra and system preference QueryWeightFields on builds a query with surrounding double quotes : q=tree => q="tree". Therefore existing double quotes are replaced by a space : q="tree" => q=" tree ".

Patch corrects by adding trim on operand after transforming quotes into spaces.

Test plan :
1) Use Zebra searchengine and enable system preference QueryWeightFields
2) Go to advanced search
3) Search on two words from a title, surrounded by double quotes. Like "Order of the Phoenix"
4) Check you get same results with an without patch

Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>
Comment 21 Katrin Fischer 2021-06-03 10:28:03 UTC
Created attachment 121572 [details] [review]
Bug 25900: trim spaces in search with Zebra and QueryWeightFields

Search with Zebra and system preference QueryWeightFields on builds a query with surrounding double quotes : q=tree => q="tree". Therefore existing double quotes are replaced by a space : q="tree" => q=" tree ".

Patch corrects by adding trim on operand after transforming quotes into spaces.

Test plan :
1) Use Zebra searchengine and enable system preference QueryWeightFields
2) Go to advanced search
3) Search on two words from a title, surrounded by double quotes. Like "Order of the Phoenix"
4) Check you get same results with an without patch

Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 22 Katrin Fischer 2021-06-03 10:29:04 UTC
I believe this change makes sense and is harmless. 

As this is a change to Search.pm we should probably have a unit test verifying it. But I am not sure how hard/easy it would be to test. Leaving final decision to RM.
Comment 23 Katrin Fischer 2021-06-03 10:57:29 UTC
Please add a unit test!
Comment 24 Katrin Fischer 2023-12-10 09:34:03 UTC
(In reply to Katrin Fischer from comment #23)
> Please add a unit test!

Still waiting for a test - please?