Bug 9588

Patch tested with a sandbox, by koha.aixmarseille <koha.aixmarseille@gmail.com>

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?

Changing the status of this patch to reflect need for clarification. See comment4 please. Thanks.

(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

QA: Looking at this one again.

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.

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.

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.

(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.

Thx Marcel :)

(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.

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.