Bug 9588 - weighted search query with index
Summary: weighted search query with index
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Fridolin Somers
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-02-11 13:46 UTC by Fridolin Somers
Modified: 2014-05-26 21:04 UTC (History)
4 users (show)

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


Attachments
Proposed patch (2.63 KB, patch)
2013-02-11 14:04 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 9588: weighted search query with index (2.69 KB, patch)
2013-02-12 11:43 UTC, Paul Poulain
Details | Diff | Splinter Review
Bug 9588: weighted search query with index (3.07 KB, patch)
2013-04-05 09:24 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Fridolin Somers 2013-02-11 13:46:37 UTC
When QueryWeightFields is enabled, the searching query is created with several options.
In C4::Search::_build_weighted_query, when no index is defined, the query is build with fuzzy and stemming options. When an index is defined, theses options are missing, only unconditional right truncation is used.
The consequence is that when QueryStemming is disabled, a search with index can give more results (due to right truncation) that a search without.
Comment 1 Fridolin Somers 2013-02-11 14:04:37 UTC Comment hidden (obsolete)
Comment 2 Paul Poulain 2013-02-12 11:43:15 UTC
Patch tested with a sandbox, by koha.aixmarseille <koha.aixmarseille@gmail.com>
Comment 3 Paul Poulain 2013-02-12 11:43:34 UTC Comment hidden (obsolete)
Comment 4 Marcel de Rooy 2013-03-29 11:36:38 UTC
Fridolyn,
This is just a short patch, but somewhat complex :) Looks good though!
Before searching it all myself, a few questions:

Why exactly do you remove the rt (right truncation)?
Should the truncate pref be tested here too?

What is the exact reason for the lower ranks for wordlist, fuzzy and stemming?
Comment 5 Marcel de Rooy 2013-04-04 08:10:55 UTC
Changing the status of this patch to reflect need for clarification. See comment4 please. Thanks.
Comment 6 Fridolin Somers 2013-04-04 13:08:30 UTC
(In reply to comment #4)
> Why exactly do you remove the rt (right truncation)?
This is the main purpose of the patch : changing weighted query with an index specified so that it behaves like the weighted query without index specified.

> Should the truncate pref be tested here too?
No, if truncate pref is on, truncation will be performed on the operand before performing weighted query on this operand.

> What is the exact reason for the lower ranks for wordlist, fuzzy and stemming?
This is in order to have a good order with relevance sort order, from the more exact match to the less : 
exact > phrase > word list > fuzzy > stemmed
Comment 7 Marcel de Rooy 2013-04-05 07:26:30 UTC
QA: Looking at this one again.
Comment 8 Marcel de Rooy 2013-04-05 09:22:15 UTC
QA Comment:
I am marking this one as Medium patch. It is certainly not trivial because of the side effects in Search.pm. 

I have tested quite a lot of searches and have still some doubts. But IMO the balance for pushing this patch goes to the positive side.

This patch makes that the results for a search like Title=art are different than for Title=art* (QueryWeightFields enabled). That is very welcome!
With stemming on, there will be no difference between Title=art and Title=art*, but the other three possibilities with Fuzzy/Stemming have all different results now.

My only problem is that the number of hits for Title=art can now exceed the hits for Title=art*. We cannot be proud of that ;) In a small testdb with Fuzzy and Stemming, I now have 237 for art and always 222 for art*. This is a very funny side-effect of this patch, although the cause of this difference is that QueryWeightFields is ignored when detecting truncation somewhere else in Search.pm. 

At the end I do not feel that this patch should be blamed for this bug. It repairs Fuzzy and Stemming for index searches, but just makes visible that the code for truncation kills the other options in a very specific context, the index search where the user adds truncation manually. We can wonder how many people will search that way in comparison to the number of people using an index field without truncation. That makes the balance go to push for me..

I will mark this as Passed QA, but will send a message to QA team and RM to allow them to give another opinion as well.
Comment 9 Marcel de Rooy 2013-04-05 09:24:05 UTC
Created attachment 17205 [details] [review]
Bug 9588: weighted search query with index

When QueryWeightFields is enabled, the searching query is created with several options.
In C4::Search::_build_weighted_query, when no index is defined, the query is build with fuzzy and stemming options. When an index is defined, theses options are missing, only unconditional right truncation is used.
The consequence is that when QueryStemming is disabled, a search with index can give more results (due to right truncation) that a search without.

This patch adds stemming and fuzzy on search with index, conditioned with QueryFuzzy and QuerryStemming sysprefs.
Also changes world list search (wrld) weight to r6 in order to set fuzzy search to r8 and stemming search to r9 (like search without index).

Test plan :
- Go to searching preferences (admin/preferences.pl?tab=searching)
- Set QueryAutoTruncate to "only if * is added"
- Set QueryFuzzy and QuerryStemming to "Don't try"
- Set QueryWeightFields to "Enable"
- Go to advanced search page
- Select an indexe (ie Title) and perform a search on a short word
=> Look at zebrarv log and see that query does not contain right truncation : @attr 5=1
- Set QueryFuzzy to "Try"
- Perform same search
=> Look at zebrarv log and see that query contains fuzzy : @attr 5=103
- Set QueryFuzzy to "Don't try" and QuerryStemming to "Try"
- Perform same search
=> Look at zebrarv log and see that query contains rigth truncation on stemmed word : @attr 5=1

Signed-off-by: koha.aixmarseille <koha.aixmarseille@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
This patch makes Fuzzy and Stemming influence search results on weighted
queries when using an index. Side-effect is however that the results for a
search like index=term* (add truncation manually too) could be LOWER than the
the number of hits for index=term. Further comments on Bugzilla.
Comment 10 Katrin Fischer 2013-04-05 09:32:27 UTC
Resding through this it seems to make sense, there are only 2 smaller things I was wondering about:
- is the truncation not depending on AutoQueryTruncate?
- should weighting for fuzzy and stemming be switched so that stemming is first? Fuzzy seems a wild guess, while stemming (at least in theory) should give you the better results.
Comment 11 Marcel de Rooy 2013-04-05 09:34:14 UTC
(In reply to comment #10)
> - is the truncation not depending on AutoQueryTruncate?
This only applies if you have the pref set to add with *.

> - should weighting for fuzzy and stemming be switched so that stemming is
> first? Fuzzy seems a wild guess, while stemming (at least in theory) should
> give you the better results.
Fridolyn followed here other parts of the code in Search.pm.
Comment 12 Katrin Fischer 2013-04-05 09:40:06 UTC
Thx Marcel :)
Comment 13 Fridolin Somers 2013-04-08 08:29:36 UTC
(In reply to comment #8)
Thanks a lot for your work.

I think new behaviors must be included in the work on query parser, rather than in the old C4::Search::_build_weighted_query.
Comment 14 Jared Camins-Esakov 2013-04-22 00:55:03 UTC
This patch has been pushed to master and 3.12.x.

I agree with Marcel, that on the balance this patch is better included than not.