[Bug 11137] New: QueryParser can ignore parts of a Boolean search after the first
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11137 Bug ID: 11137 Summary: QueryParser can ignore parts of a Boolean search after the first Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: major Priority: P5 - low Component: Searching Assignee: gmcharlt@gmail.com Reporter: gmcharlt@gmail.com QA Contact: testopia@bugs.koha-community.org The way that the QueryParser configuration is initialized results in duplicate search field aliases being defined, which in turn can cause problems with Boolean searches. For example, consider the following search: kw:history && earth This will retrieve all records that have both 'history' and 'earth' as keywords. Depending on your search settings, this search is received by Zebra as @attrset Bib-1 @and @attr 4=6 @attr 5=1 @attr 1=1016 history @attr 4=6 @attr 5=1 @attr 1=1016 earth Now consider this search, which should be identical: kw:history && kw:earth This search retrieves all records containing 'history', but ignores 'earth'. The search received by Zebra is: @attrset Bib-1 @attr 4=6 @attr 5=1 @attr 1=1016 histor -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11137 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11137 --- Comment #1 from Galen Charlton <gmcharlt@gmail.com> --- Created attachment 22380 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=22380&action=edit Bug 11137: regression tests for QP search field alias bug This patch adds regression tests to verify that Boolean searches using QueryParser function correctly. This patch also ensures that QP is correctly initalized when Search.t is run. To test: [1] Apply this patch and the following patch. [2] Verify that prove -v t/QueryParser.t works [3] Verify that prove -v t/db_dependent/Search.t works [4] (optional) instead of applying both patches at the same time, apply only the regression test patch and run the tests listed in steps 2 and 3. The following tests should fail: t/db_dependent/Search.t (Wstat: 512 Tests: 198 Failed: 2) Failed tests: 42, 71 Non-zero exit status: 2 t/QueryParser.t (Wstat: 256 Tests: 28 Failed: 1) Failed test: 12 Non-zero exit status: 1 Signed-off-by: Galen Charlton <gmc@esilibrary.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11137 --- Comment #2 from Galen Charlton <gmcharlt@gmail.com> --- Created attachment 22381 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=22381&action=edit Bug 11137: fix certain types of searches run using QueryParser QueryParser was loading redundant search aliases, which in turn resulted in certain queries involving two or more clauses joined by a Boolean operator getting parsed in correctly. To test: [1] Enable QueryParser. [2] Arrange your database so that some some bib records contain the keyword 'history' while a smaller subset contain both 'history' and 'earth'. (The exact words used don't matter.) [3] Run the following search from the OPAC global search bar: kw:history && kw:earth [4] Observe that the records that are returned include *all* bibs in the database with the keyword 'history'. In other words, the restriction that the records must also contain 'earth' is not observed. [5] Apply the patch and rerun the search. [6] This time, the results that are returned should include only those that contain both 'history' and 'earth'. [7] Verify that prove -v t/QueryParser.t passes. Signed-off-by: Galen Charlton <gmc@esilibrary.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11137 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #22380|0 |1 is obsolete| | Attachment #22381|0 |1 is obsolete| | --- Comment #3 from Galen Charlton <gmcharlt@gmail.com> --- Created attachment 22383 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=22383&action=edit Bug 11137: regression tests for QP search field alias bug This patch adds regression tests to verify that Boolean searches using QueryParser function correctly. This patch also ensures that QP is correctly initalized when Search.t is run. To test: [1] Apply this patch and the following patch. [2] Verify that prove -v t/QueryParser.t works [3] Verify that prove -v t/db_dependent/Search.t works [4] (optional) instead of applying both patches at the same time, apply only the regression test patch and run the tests listed in steps 2 and 3. The following tests should fail: t/db_dependent/Search.t (Wstat: 512 Tests: 198 Failed: 2) Failed tests: 42, 71 Non-zero exit status: 2 t/QueryParser.t (Wstat: 256 Tests: 28 Failed: 1) Failed test: 12 Non-zero exit status: 1 Signed-off-by: Galen Charlton <gmc@esilibrary.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11137 --- Comment #4 from Galen Charlton <gmcharlt@gmail.com> --- Created attachment 22384 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=22384&action=edit Bug 11137: fix certain types of searches run using QueryParser QueryParser was loading redundant search aliases, which in turn resulted in certain queries involving two or more clauses joined by a Boolean operator getting parsed in correctly. To test: [1] Enable QueryParser. [2] Arrange your database so that some some bib records contain the keyword 'history' while a smaller subset contain both 'history' and 'earth'. (The exact words used don't matter.) [3] Run the following search from the OPAC global search bar: kw:history && kw:earth [4] Observe that the records that are returned include *all* bibs in the database with the keyword 'history'. In other words, the restriction that the records must also contain 'earth' is not observed. [5] Apply the patch and rerun the search. [6] This time, the results that are returned should include only those that contain both 'history' and 'earth'. [7] Verify that prove -v t/QueryParser.t passes. Signed-off-by: Galen Charlton <gmc@esilibrary.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11137 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11137 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #22383|0 |1 is obsolete| | Attachment #22384|0 |1 is obsolete| | --- Comment #5 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 22438 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=22438&action=edit [SIGNED-OFF] Bug 11137: regression tests for QP search field alias bug This patch adds regression tests to verify that Boolean searches using QueryParser function correctly. This patch also ensures that QP is correctly initalized when Search.t is run. To test: [1] Apply this patch and the following patch. [2] Verify that prove -v t/QueryParser.t works [3] Verify that prove -v t/db_dependent/Search.t works [4] (optional) instead of applying both patches at the same time, apply only the regression test patch and run the tests listed in steps 2 and 3. The following tests should fail: t/db_dependent/Search.t (Wstat: 512 Tests: 198 Failed: 2) Failed tests: 42, 71 Non-zero exit status: 2 t/QueryParser.t (Wstat: 256 Tests: 28 Failed: 1) Failed test: 12 Non-zero exit status: 1 Signed-off-by: Galen Charlton <gmc@esilibrary.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11137 --- Comment #6 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 22439 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=22439&action=edit [SIGNED-OFF] Bug 11137: fix certain types of searches run using QueryParser QueryParser was loading redundant search aliases, which in turn resulted in certain queries involving two or more clauses joined by a Boolean operator getting parsed in correctly. To test: [1] Enable QueryParser. [2] Arrange your database so that some some bib records contain the keyword 'history' while a smaller subset contain both 'history' and 'earth'. (The exact words used don't matter.) [3] Run the following search from the OPAC global search bar: kw:history && kw:earth [4] Observe that the records that are returned include *all* bibs in the database with the keyword 'history'. In other words, the restriction that the records must also contain 'earth' is not observed. [5] Apply the patch and rerun the search. [6] This time, the results that are returned should include only those that contain both 'history' and 'earth'. [7] Verify that prove -v t/QueryParser.t passes. Signed-off-by: Galen Charlton <gmc@esilibrary.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Passes test plans, all tests and QA script. Also checked the db_dependent/Search.t tests. Tried some other searches, all seems to be working correctly. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11137 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@biblibre.co | |m --- Comment #7 from Jonathan Druart <jonathan.druart@biblibre.com> --- I cannot test this patch. My MARC21 DB is broken and with my UNIMARC DB, I got no result. I tried to debug: In QueryParser::Driver::PQF::target_syntax, debug is enable. - with marcflavour=MARC21 QP query for biblioserver: history #relevance_dsc PQF query: @attr 2=102 @or @or @attr 1=1016 @attr 5=1 @attr 4=6 "history" @attr 9=20 @attr 2=102 @attr 5=1 @attr 4=6 "history" @attr 9=34 @attr 2=102 @attr 5=1 @attr 4=6 "history" - with marcflavour=UNIMARC QP query for biblioserver: history #relevance_asc PQF query: @and @or @or @attr 1=1016 @attr 4=6 "history" @attr 9=20 @attr 2=102 @attr 4=6 "history" @attr 9=34 @attr 2=102 @attr 4=6 "history" @or @or @attr 1=1016 @attr 4=6 "#relevance_asc" @attr 9=20 @attr 2=102 @attr 4=6 "#relevance_asc" @attr 9=34 @attr 2=102 @attr 4=6 "#relevance_asc" The call to $self->parse_tree->target_syntax does not remove the modifiers for UNIMARC. I don't know how continue to investigate. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11137 --- Comment #8 from Jonathan Druart <jonathan.druart@biblibre.com> --- So stupid... My Unimarc DB got a default sort field relevance dsc and there is no modifier relevance dsc in the qp.yml file. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11137 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11137 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #22438|0 |1 is obsolete| | Attachment #22439|0 |1 is obsolete| | --- Comment #9 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 22932 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=22932&action=edit Bug 11137: regression tests for QP search field alias bug This patch adds regression tests to verify that Boolean searches using QueryParser function correctly. This patch also ensures that QP is correctly initalized when Search.t is run. To test: [1] Apply this patch and the following patch. [2] Verify that prove -v t/QueryParser.t works [3] Verify that prove -v t/db_dependent/Search.t works [4] (optional) instead of applying both patches at the same time, apply only the regression test patch and run the tests listed in steps 2 and 3. The following tests should fail: t/db_dependent/Search.t (Wstat: 512 Tests: 198 Failed: 2) Failed tests: 42, 71 Non-zero exit status: 2 t/QueryParser.t (Wstat: 256 Tests: 28 Failed: 1) Failed test: 12 Non-zero exit status: 1 Signed-off-by: Galen Charlton <gmc@esilibrary.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11137 --- Comment #10 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 22933 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=22933&action=edit Bug 11137: fix certain types of searches run using QueryParser QueryParser was loading redundant search aliases, which in turn resulted in certain queries involving two or more clauses joined by a Boolean operator getting parsed in correctly. To test: [1] Enable QueryParser. [2] Arrange your database so that some some bib records contain the keyword 'history' while a smaller subset contain both 'history' and 'earth'. (The exact words used don't matter.) [3] Run the following search from the OPAC global search bar: kw:history && kw:earth [4] Observe that the records that are returned include *all* bibs in the database with the keyword 'history'. In other words, the restriction that the records must also contain 'earth' is not observed. [5] Apply the patch and rerun the search. [6] This time, the results that are returned should include only those that contain both 'history' and 'earth'. [7] Verify that prove -v t/QueryParser.t passes. Signed-off-by: Galen Charlton <gmc@esilibrary.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Passes test plans, all tests and QA script. Also checked the db_dependent/Search.t tests. Tried some other searches, all seems to be working correctly. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> I reproduce the bug and I confirm this patch fixes it. Without the patch, kw:history && kw:earth, I got: Search biblios OK 4 1 1+0 RPN @attrset Bib-1 @attr 4=6 @attr 1=1016 history With the path, I get: Search biblios OK 2 1 1+0 RPN @attrset Bib-1 @and @attr 4=6 @attr 1=1016 history @attr 4=6 @attr 1=1016 earth -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11137 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |jonathan.druart@biblibre.co |y.org |m -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11137 --- Comment #11 from Jonathan Druart <jonathan.druart@biblibre.com> --- Galen: I know there is no sense to sort by relevance asc but don't you know it is worth to add an entry in the qp config file? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11137 --- Comment #12 from Galen Charlton <gmcharlt@gmail.com> --- (In reply to Jonathan Druart from comment #11)
Galen: I know there is no sense to sort by relevance asc but don't you know it is worth to add an entry in the qp config file?
I think so, as there's nothing preventing somebody from setting relevance ascending as the default sort order. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11137 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #13 from Galen Charlton <gmcharlt@gmail.com> --- Pushed to master and 3.14.x. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11137 --- Comment #14 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Galen Charlton from comment #12)
(In reply to Jonathan Druart from comment #11)
Galen: I know there is no sense to sort by relevance asc but don't you know it is worth to add an entry in the qp config file?
I think so, as there's nothing preventing somebody from setting relevance ascending as the default sort order.
Patch submitted in bug 11255. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11137 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |Pushed to Stable CC| |tomascohen@gmail.com --- Comment #15 from Tomás Cohen Arazi <tomascohen@gmail.com> --- This patch has been pushed to 3.12.x, will be in 3.12.8. Thanks Galen! -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org