When searching on date of publication or date of acquisition index from advanced search, there are several bugs : - The search is performed on no index, using only search therm. - The next or previous pages lead to an empty result page. This comes from specific behavior in C4/Search.pm.
Created attachment 14472 [details] [review] Proposed patch
See test plan in commit message
Created attachment 15114 [details] [review] Bug 9368: specific behavior of yr and acqdate indexes When searching on the date of publication or date of acquisition indexes from advanced search, there are several bugs : - The search is performed on specific index, using only search term. - The next or previous pages lead to an empty result page. This comes from specific behavior in C4/Search.pm, $indexes_set flag is used but the index is not added to the operand. This patch corrects this. Test plan : - Go to advanced search in the staff interface - Select publication date or acquisition date - Enter a valid search term (you must have several pages of results) - Click on search => result page shows a correct result number - Click on page 2 => next results are shown Signed-off-by: Galen Charlton <gmc@esilibrary.com>
QA comment: Before this patch, if the "publication date" index is selected on the adv search form, zebra requests on @attrset Bib-1 2000 The index is not defined! After applying it: @attrset Bib-1 @attr 1=pubdate @attr 4=109 @attr 2=3 2000 Which is what the user want. Marked as Passed QA.
Created attachment 16268 [details] [review] Bug 9368: specific behavior of yr and acqdate indexes When searching on the date of publication or date of acquisition indexes from advanced search, there are several bugs : - The search is performed on specific index, using only search term. - The next or previous pages lead to an empty result page. This comes from specific behavior in C4/Search.pm, $indexes_set flag is used but the index is not added to the operand. This patch corrects this. Test plan : - Go to advanced search in the staff interface - Select publication date or acquisition date - Enter a valid search term (you must have several pages of results) - Click on search => result page shows a correct result number - Click on page 2 => next results are shown Signed-off-by: Galen Charlton <gmc@esilibrary.com>
This breaks the yr and acqdate indexes when QP is enabled. I will provide a counter-patch or follow-up once we are in feature freeze (assigning to myself so that I don't forget).
(In reply to Jared Camins-Esakov from comment #6) > This breaks the yr and acqdate indexes when QP is enabled. I will provide a > counter-patch or follow-up once we are in feature freeze (assigning to > myself so that I don't forget). If QP is enabled, those special behavior for yr and acqdate are usless non ? May we add a "unless useQueryParser" condition ?
Hi Jared, can you submit a follow-up for the QueryParser?
Hello I discovered the problem recently and signaled it to Biblibre, who pointed to this bug... Not a big issue, but it must be fixed, for QP and non-QP instances... Maybe an alternative could be to suppress the ability to select a date with the dropdown list (I'm not sure that's very intuitive), and just use the special input field made for filtering by dates. Mathieu
(In reply to mathieu saby from comment #10) > Maybe an alternative could be to suppress the ability to select a date with > the dropdown list (I'm not sure that's very intuitive), and just use the > special input field made for filtering by dates. +1
Created attachment 26652 [details] [review] Bug 9368 - do not change if QueryParser
@Jared Here is the follow-up to change code only if not QP on. "Bug 9368 - do not change if QueryParser" needs signoff
It looks to me that "Publication Date (yyyy)" searching is already broken in master (before applying these patches) when using QueryParser. I get a query like this: @attr 2=102 @or @or @attr 1=1016 @attr 5=1 @attr 4=6 "yr:2012" @attr 9=20 @attr 2=102 @attr 5=1 @attr 4=6 "yr:2012" @attr 9=34 @attr 2=102 @attr 5=1 @attr 4=6 "yr:2012" It's trying to do a keyword search with the keyword string as "yr:2012". It's worth noting that the "Bug 9368 - do not change if QueryParser" patch doesn't fix this. -- I have a counter-patch that I will be posting in a moment, which will fix the non-QueryParser search, and which will cause QueryParser to fallback to non-QueryParser mode instead of breaking. (It seems to me that QueryParser needs a lot more work before its able to replace the existing buildQuery, unfortunately.)
Created attachment 28916 [details] [review] Bug 9368 [ALTERNATE] - specific behavior of yr and acqdate indexes This patch fixes the "Publication date" and "Acquisition date" searches when using non-QueryParser and QueryParser searches. It adds structure attributes to the template, which is consistent with how phrase searching is currently handled. It removes unnecessary code from Search.pm, adds some necessary code which is consistent with existing code, and adds a lot of explanatory comments. _TEST PLAN_ Before applying: 0) Turn off QueryParser 1) Try a "Publication date" or "Acquisition date" search from the staff client advanced search. 2) Note that even though the description on the result page makes it seem like you're doing an index-specific search, you're actually doing a keyword search. You can verify this by checking the 008 from positions 7 to 10 for "Publication date" or "Date accessioned" on items for "Acquisition date". 3) Turn on QueryParser 4) Try doing the searches from Step 1. 5) A "Publication date" search should probably produce zero results After applying patch: 6) Keep QueryParser on 7) Try doing the searches from Step 1. 8) Notice that you're actually getting results consistent with your search (ie the 008/7-10 shows the date you searched for, and there is a "Date accessioned" in items which matches your search) 9) Turn off QueryParser 10) Note that your results are exactly the same as step 8 (N.B. this is because QueryParser is falling back to non-QP mode instead of producing a bad query.)
Btw, it also fixes the paging issue.
I'm going to assign this to myself, since I think my alternate patch solves the key problems of this issue. I have a few other bugs out there which mention QueryParser problems, and I'm hoping to unite those some point down the road
Created attachment 30318 [details] [review] Bug 9368 [ALTERNATE] - specific behavior of yr and acqdate indexes This patch fixes the "Publication date" and "Acquisition date" searches when using non-QueryParser and QueryParser searches. It adds structure attributes to the template, which is consistent with how phrase searching is currently handled. It removes unnecessary code from Search.pm, adds some necessary code which is consistent with existing code, and adds a lot of explanatory comments. _TEST PLAN_ Before applying: 0) Turn off QueryParser 1) Try a "Publication date" or "Acquisition date" search from the staff client advanced search. 2) Note that even though the description on the result page makes it seem like you're doing an index-specific search, you're actually doing a keyword search. You can verify this by checking the 008 from positions 7 to 10 for "Publication date" or "Date accessioned" on items for "Acquisition date". 3) Turn on QueryParser 4) Try doing the searches from Step 1. 5) A "Publication date" search should probably produce zero results After applying patch: 6) Keep QueryParser on 7) Try doing the searches from Step 1. 8) Notice that you're actually getting results consistent with your search (ie the 008/7-10 shows the date you searched for, and there is a "Date accessioned" in items which matches your search) 9) Turn off QueryParser 10) Note that your results are exactly the same as step 8 (N.B. this is because QueryParser is falling back to non-QP mode instead of producing a bad query.) Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Works great, thanks David. I've tested search on year range (yr:1999-2001) and using next/previous page in results. Everything is ok.
Created attachment 31552 [details] [review] [PASSED QA] Bug 9368 [ALTERNATE] - specific behavior of yr and acqdate indexes This patch fixes the "Publication date" and "Acquisition date" searches when using non-QueryParser and QueryParser searches. It adds structure attributes to the template, which is consistent with how phrase searching is currently handled. It removes unnecessary code from Search.pm, adds some necessary code which is consistent with existing code, and adds a lot of explanatory comments. _TEST PLAN_ Before applying: 0) Turn off QueryParser 1) Try a "Publication date" or "Acquisition date" search from the staff client advanced search. 2) Note that even though the description on the result page makes it seem like you're doing an index-specific search, you're actually doing a keyword search. You can verify this by checking the 008 from positions 7 to 10 for "Publication date" or "Date accessioned" on items for "Acquisition date". 3) Turn on QueryParser 4) Try doing the searches from Step 1. 5) A "Publication date" search should probably produce zero results After applying patch: 6) Keep QueryParser on 7) Try doing the searches from Step 1. 8) Notice that you're actually getting results consistent with your search (ie the 008/7-10 shows the date you searched for, and there is a "Date accessioned" in items which matches your search) 9) Turn off QueryParser 10) Note that your results are exactly the same as step 8 (N.B. this is because QueryParser is falling back to non-QP mode instead of producing a bad query.) Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Patch pushed to master. Thanks David!