Bug 12323 - Acquisitions search results fixed in order of biblionumber
Summary: Acquisitions search results fixed in order of biblionumber
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Kyle M Hall
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-05-28 18:34 UTC by Kyle M Hall
Modified: 2015-12-03 22:02 UTC (History)
7 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 12323 - Acquisitions search results fixed in order of biblionumber (1.84 KB, patch)
2014-05-28 18:38 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 12323 [SIGNED OFF] - Acquisitions search results fixed in order of biblionumber (2.51 KB, patch)
2014-06-19 04:30 UTC, David Cook
Details | Diff | Splinter Review
Bug 12323 [SIGNED OFF] - Acquisitions search results fixed in order of biblionumber (2.70 KB, patch)
2014-07-21 08:44 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2014-05-28 18:34:42 UTC
The acquisitions search is exhibiting the same behavior as bug 11410. Results are always fixed in order of biblionumber, among other possible issues ( including the ampersand issue ).
Comment 1 Kyle M Hall 2014-05-28 18:38:42 UTC Comment hidden (obsolete)
Comment 2 Kyle M Hall 2014-05-28 18:39:53 UTC
Note: UseQueryParser needs to be disabled.
Comment 3 Christopher Brannon 2014-05-28 19:55:41 UTC
The results look much better, but when I do a search, the number of results vary.  For example, in our catalog:

"benny and joon" produce 1404 results.

"benny & joon" produce 1525 results.

As far as the results go, I am very pleased, but just need to make sure the number of results isn't a sign of another problem.

Christopher
Comment 4 David Cook 2014-06-19 04:10:03 UTC
This patch definitely works, but (In reply to Christopher Brannon from comment #3)
> The results look much better, but when I do a search, the number of results
> vary.  For example, in our catalog:
> 
> "benny and joon" produce 1404 results.
> 
> "benny & joon" produce 1525 results.
> 
> As far as the results go, I am very pleased, but just need to make sure the
> number of results isn't a sign of another problem.
> 
> Christopher

Christopher, this is definitely the sign of a different issue.

If you're doing a search for "benny and joon" or "benny & joon", it's most likely doing a keyword wordlist search. If I recall correctly, that means that it's searching for "benny", "joon", "and" or "benny", "joon", "&" as separate keywords. It's not conducting a boolean search. 

To do a boolean search, I think you'll have to use the Advanced Search. 

(It might also be possible by typing something like kw:"benny" and kw:"joon" into the regular search box, but I'm not 100% sure on that one off the top of my head.)
Comment 5 David Cook 2014-06-19 04:19:32 UTC
(In reply to Christopher Brannon from comment #3)
> The results look much better, but when I do a search, the number of results
> vary.  For example, in our catalog:
> 
> "benny and joon" produce 1404 results.
> 
> "benny & joon" produce 1525 results.
> 
> As far as the results go, I am very pleased, but just need to make sure the
> number of results isn't a sign of another problem.
> 
> Christopher

I just tried using & and && in the query parser as boolean operators and they don't work either (although doing an advanced boolean search while using QP will show && as a boolean operator, but that points to problems with the QueryParser).

I would recommend (kw:benny) and (kw:joon), or kw:"benny" and kw:"joon". Either of those should yield the same results.
Comment 6 David Cook 2014-06-19 04:30:57 UTC Comment hidden (obsolete)
Comment 7 Katrin Fischer 2014-07-06 18:53:32 UTC
(In reply to Kyle M Hall from comment #2)
> Note: UseQueryParser needs to be disabled.

I have tested this and with QueryParser set to "Try" my search results are NOT sorted by biblionumber.

With QueryParser set to "Do not try" the results are sorted. 

Is this the expected behaviour?
Comment 8 Katrin Fischer 2014-07-06 18:54:23 UTC
Setting to "Failed" - waiting for an answer from Kyle.
Comment 9 David Cook 2014-07-07 00:04:37 UTC
(In reply to Katrin Fischer from comment #7)
> (In reply to Kyle M Hall from comment #2)
> > Note: UseQueryParser needs to be disabled.
> 
> I have tested this and with QueryParser set to "Try" my search results are
> NOT sorted by biblionumber.
> 
> With QueryParser set to "Do not try" the results are sorted. 
> 
> Is this the expected behaviour?

This should be the expected behaviour, as the QueryParser doesn't rely on buildQuery for performing relevance searching (relevance searching is built into the QP config). So the QP will have the same results before and after this patch, while non-QP will be very different (i.e. relevance only added when using this patch).

Ideally, we would just use QueryParser here (as buildQuery isn't very good at parsing free-form searches), but until we transition more completely to QueryParser, I suppose this makes sense.
Comment 10 Katrin Fischer 2014-07-12 18:51:05 UTC
I've retested this:

I searched for an existing record to create an order line from in acquisitions. My search term was DE-576, which is the MarcOrgCode that appears in most of my records.

Without patch, plain master:
- QueryParser: Try
- Results ARE sorted by biblionumber

- QueryParser: Don't try
- Results ARE sorted by biblionumber

With patch:
- Query parser: Try
- Results ARE sorted by bilbionumber

- Query parser: Don't try
- Results ARE sorted by bilbionumber

... I am confused now. Maybe some other setting plays a role here?
Comment 11 David Cook 2014-07-14 00:09:06 UTC
(In reply to Katrin Fischer from comment #10)
> I've retested this:
> 
> I searched for an existing record to create an order line from in
> acquisitions. My search term was DE-576, which is the MarcOrgCode that
> appears in most of my records.
> 
> Without patch, plain master:
> - QueryParser: Try
> - Results ARE sorted by biblionumber
> 
> - QueryParser: Don't try
> - Results ARE sorted by biblionumber
> 
> With patch:
> - Query parser: Try
> - Results ARE sorted by bilbionumber
> 
> - Query parser: Don't try
> - Results ARE sorted by bilbionumber
> 
> ... I am confused now. Maybe some other setting plays a role here?

When QueryParser is set to "Don't try", "QueryAutoTruncation" will turn off relevance searching if it's set to "automatically". 

However, relevance searching shouldn't be affected by "QueryAutoTruncation" when QueryParser is set to "Try".

I suspect...that your result might be coincidence, Katrin. With the QueryParser, it applies relevance to your search query as well as adding relevance subqueries for title keyword and title exact.

Since you're using "DE-576" as your search term, you'll probably get no hits for title keyword or title exact, so the relevance "bumps" become irrelevant.

Then, for the results you do get, I suspect that "DE-576" would only show up once in a record so each record retrieved would have the same relevance score. Since the search term is quite specific, Zebra will probably only return records with that exact search term so you won't get anything "less relevant".

tl;dr

Katrin, could you try again but using a more vague search term? Something that is likely to appear in a lot of different records with different frequency? Probably a noun. I often like to use "health" or "test" as we have a lot of medical records.

I might be wrong, but...I think that's the most logical explanation for your results.
Comment 12 Katrin Fischer 2014-07-20 21:12:47 UTC
This one is really confusing to me - can someone else give this a try please?
Comment 13 Jonathan Druart 2014-07-21 08:44:21 UTC
Created attachment 29913 [details] [review]
Bug 12323 [SIGNED OFF] - Acquisitions search results fixed in order of biblionumber

The acquisitions search is exhibiting the same behavior as bug 11410.
Results are always fixed in order of biblionumber, among other possible
issues ( including the ampersand issue ).

Test Plan:
1) From an open basket, choose "Add to basket"
2) Run a search for "From an existing record"
3) Note the "View MARC" URLs are fixed in order of biblionumber
4) Apply this patch
5) Refresh the page
6) Note the results are no longer fixed in order of biblionumber

Signed-off-by: David Cook <dcook@prosentient.com.au>

Works as described.

I think the code could be a bit tidier, but I think it makes sense to
use buildQuery here. It'll detect CCL, CQL, and PQF queries, as well
as parsing a regular keyword search as one would expect when searching
the catalogue.

It also has the added bonus of performing relevance searching,
so long as QueryAutoTruncation is off, and so long as library staff
avoid using the "*" truncation wildcard (see bug 12430).

While there are simpler ways to fix this acq bug, I think this was
probably the best move, as it adds a bit to the consistency of what
librarians can expect from their search results.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Same result with and without the patch but I agree with the changes.
BuildQuery should be called before SimpleSearch if QP is not used.
Comment 14 Tomás Cohen Arazi 2015-02-10 17:09:19 UTC
Patch pushed to master.

Thanks Kyle!
Comment 15 Chris Cormack 2015-02-14 07:23:30 UTC
Pushed to 3.18.x will be in 3.18.4
Comment 16 Mason James 2015-02-26 04:42:07 UTC
Pushed to 3.16.x, will be in 3.16.8