Bug 9239 - Koha should share Evergreen's QueryParser module for parsing queries
Summary: Koha should share Evergreen's QueryParser module for parsing queries
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jared Camins-Esakov
QA Contact: Katrin Fischer
URL:
Keywords:
Depends on: 9183
Blocks: 9234 24735
  Show dependency treegraph
 
Reported: 2012-12-06 23:53 UTC by Jared Camins-Esakov
Modified: 2020-02-26 12:35 UTC (History)
5 users (show)

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


Attachments
Bug 9239: Add QueryParser module to Koha (61.77 KB, patch)
2012-12-12 02:54 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 9239: Introduce QueryParser driver for PQF (79.51 KB, patch)
2012-12-12 02:55 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 9239: Add syspref to enable QueryParser (5.76 KB, patch)
2012-12-12 02:55 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 9239: Allow the use of QueryParser for all queries (23.95 KB, patch)
2012-12-12 02:55 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 9239: Add QueryParser module to Koha (61.77 KB, patch)
2012-12-17 20:44 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 9239: Introduce QueryParser driver for PQF (87.30 KB, patch)
2012-12-17 20:44 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 9239: Add syspref to enable QueryParser (5.81 KB, patch)
2012-12-17 20:44 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 9239: Allow the use of QueryParser for all queries (24.72 KB, patch)
2012-12-17 20:44 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 9239: Add QueryParser module to Koha (61.83 KB, patch)
2012-12-18 08:32 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 9239: Introduce QueryParser driver for PQF (87.36 KB, patch)
2012-12-18 08:34 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 9239: Add syspref to enable QueryParser (5.71 KB, patch)
2012-12-18 08:34 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 9239: Allow the use of QueryParser for all queries (24.77 KB, patch)
2012-12-18 08:35 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 9239 follow-up: POD and perlcritic fixes (7.87 KB, patch)
2013-01-25 12:50 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
[PAssed QA] Bug 9239: Add QueryParser module to Koha (61.90 KB, patch)
2013-01-30 15:11 UTC, Elliott Davis
Details | Diff | Splinter Review
[Passed QA] Bug 9239: Introduce QueryParser driver for PQF (87.49 KB, patch)
2013-01-30 15:11 UTC, Elliott Davis
Details | Diff | Splinter Review
[Passed QA] Bug 9239: Add syspref to enable QueryParser (5.84 KB, patch)
2013-01-30 15:12 UTC, Elliott Davis
Details | Diff | Splinter Review
[Passed QA] Bug 9239: Allow the use of QueryParser for all queries (24.94 KB, patch)
2013-01-30 15:12 UTC, Elliott Davis
Details | Diff | Splinter Review
[Signed off] Bug 9239 follow-up: POD and perlcritic fixes (7.33 KB, patch)
2013-01-30 15:12 UTC, Elliott Davis
Details | Diff | Splinter Review
Bug 9239 follow-up: update koha-conf.xml (2.27 KB, patch)
2013-02-07 00:55 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 9239 follow-up: update koha-conf.xml (2.33 KB, patch)
2013-02-17 08:07 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 9239 QA follow-up: fix highlighting and merge error (2.60 KB, patch)
2013-03-04 22:03 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 9239 QA follow-up: fix highlighting and merge error (2.68 KB, patch)
2013-03-04 22:29 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 9239 QA-follow-up: quiet warnings and fix problem with limits (2.37 KB, patch)
2013-03-07 12:23 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 9239 QA follow-up: escape CGI input (6.41 KB, patch)
2013-03-09 15:37 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 9239 QA follow-up: the last QA follow-up was missing a require (2.29 KB, patch)
2013-03-10 13:18 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 9239 QA follow-up: don't fall back to kw if field is specified (1.11 KB, patch)
2013-03-11 22:36 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 9239 QA follow-up: better error handling (1.96 KB, patch)
2013-03-14 12:45 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 9239 QA follow-up: remove stray debug code (3.33 KB, patch)
2013-03-14 14:13 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 9239 QA follow-up: escape quotes in searches (1.48 KB, patch)
2013-03-14 14:51 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 9239 QA follow-up: amend error handling (891 bytes, patch)
2013-03-14 16:14 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 9239 QA follow-up: amend error handling (890 bytes, patch)
2013-03-14 16:16 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 9239 QA follow-up: fix overeager quote escaping (1.75 KB, patch)
2013-03-14 18:04 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
[PASSED QA] Bug 9239: Add QueryParser module to Koha (61.95 KB, patch)
2013-03-14 21:36 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 9239: Introduce QueryParser driver for PQF (87.54 KB, patch)
2013-03-14 21:36 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 9239: Add syspref to enable QueryParser (5.85 KB, patch)
2013-03-14 21:37 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 9239: Allow the use of QueryParser for all queries (25.00 KB, patch)
2013-03-14 21:37 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 9239 follow-up: POD and perlcritic fixes (7.40 KB, patch)
2013-03-14 21:37 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 9239 follow-up: update koha-conf.xml (2.47 KB, patch)
2013-03-14 21:37 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 9239 QA follow-up: fix highlighting and merge error (2.78 KB, patch)
2013-03-14 21:37 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 9239 QA-follow-up: quiet warnings and fix problem with limits (2.44 KB, patch)
2013-03-14 21:37 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 9239 QA follow-up: escape CGI input (6.49 KB, patch)
2013-03-14 21:38 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 9239 QA follow-up: the last QA follow-up was missing a require (2.36 KB, patch)
2013-03-14 21:38 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 9239 QA follow-up: don't fall back to kw if field is specified (1.18 KB, patch)
2013-03-14 21:38 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 9239 QA follow-up: remove stray debug code (3.40 KB, patch)
2013-03-14 21:38 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 9239 QA follow-up: better error handling (2.03 KB, patch)
2013-03-14 21:38 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 9239 QA follow-up: escape quotes in searches (1.55 KB, patch)
2013-03-14 21:39 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 9239 QA follow-up: amend error handling (959 bytes, patch)
2013-03-14 21:39 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 9239 QA follow-up: fix overeager quote escaping (2.96 KB, patch)
2013-03-14 21:39 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 9239: Add QueryParser module to Koha (62.00 KB, patch)
2013-03-17 01:26 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 9239: Introduce QueryParser driver for PQF (87.59 KB, patch)
2013-03-17 01:26 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 9239: Add syspref to enable QueryParser (6.33 KB, patch)
2013-03-17 01:27 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 9239: Allow the use of QueryParser for all queries (29.21 KB, patch)
2013-03-17 01:27 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 9239 follow-up: POD and perlcritic fixes (7.45 KB, patch)
2013-03-17 01:27 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 9239 follow-up: update koha-conf.xml (2.52 KB, patch)
2013-03-17 01:27 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 9239 QA follow-up: fix highlighting and merge error (2.83 KB, patch)
2013-03-17 01:27 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 9239 QA-follow-up: quiet warnings and fix problem with limits (2.49 KB, patch)
2013-03-17 01:27 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 9239 QA follow-up: escape CGI input (6.54 KB, patch)
2013-03-17 01:28 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 9239 QA follow-up: the last QA follow-up was missing a require (2.41 KB, patch)
2013-03-17 01:28 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 9239 QA follow-up: don't fall back to kw if field is specified (1.23 KB, patch)
2013-03-17 01:28 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 9239 QA follow-up: remove stray debug code (3.45 KB, patch)
2013-03-17 01:28 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 9239 QA follow-up: better error handling (5.81 KB, patch)
2013-03-17 01:28 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 9239 QA follow-up: escape quotes in searches (1.60 KB, patch)
2013-03-17 01:28 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 9239 QA follow-up: amend error handling (1011 bytes, patch)
2013-03-17 01:29 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 9239 QA follow-up: fix overeager quote escaping (3.01 KB, patch)
2013-03-17 01:29 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jared Camins-Esakov 2012-12-06 23:53:29 UTC
Koha's current query parsing code is very fragile, hard to understand, and generally quite buggy. By sharing Evergreen's general-purpose QueryParser module, we can eliminate a lot of that code and offer users a better experience.
Comment 1 Jared Camins-Esakov 2012-12-12 02:54:54 UTC Comment hidden (obsolete)
Comment 2 Jared Camins-Esakov 2012-12-12 02:55:05 UTC Comment hidden (obsolete)
Comment 3 Jared Camins-Esakov 2012-12-12 02:55:15 UTC Comment hidden (obsolete)
Comment 4 Jared Camins-Esakov 2012-12-12 02:55:23 UTC Comment hidden (obsolete)
Comment 5 Jared Camins-Esakov 2012-12-12 02:59:31 UTC
The four patches I have attached to this bug do not represent entirely finished work. In particular, I did not yet write test plans, and a reshuffling of the code hierarchies means that the unit tests I had do not work at present. However, rather than wait until I have completely finished everything, I wanted to post this work-in-progress so that people who were interested could apply the patches, try a few searches, and see what they think.

One important point on testing: you will need to add <queryparser_config> element at the end of koha-conf.xml which points to etc/searchengine/queryparser.yaml. For example,
 <queryparser_config>/home/jcamins/kohaclone/etc/searchengine/queryparser.yaml</queryparser_config>
Comment 6 Jared Camins-Esakov 2012-12-17 20:44:12 UTC Comment hidden (obsolete)
Comment 7 Jared Camins-Esakov 2012-12-17 20:44:23 UTC Comment hidden (obsolete)
Comment 8 Jared Camins-Esakov 2012-12-17 20:44:31 UTC Comment hidden (obsolete)
Comment 9 Jared Camins-Esakov 2012-12-17 20:44:43 UTC Comment hidden (obsolete)
Comment 10 Chris Cormack 2012-12-18 08:22:03 UTC
Tests all pass

t/QueryParser.t .. ok    
All tests successful.
Files=1, Tests=27,  0 wallclock secs ( 0.03 usr  0.01 sys +  0.14 cusr  0.01 csys =  0.19 CPU)
Result: PASS

Working through the rest of the test plan now
Comment 11 Chris Cormack 2012-12-18 08:32:11 UTC Comment hidden (obsolete)
Comment 12 Chris Cormack 2012-12-18 08:34:45 UTC Comment hidden (obsolete)
Comment 13 Chris Cormack 2012-12-18 08:34:59 UTC Comment hidden (obsolete)
Comment 14 Chris Cormack 2012-12-18 08:35:12 UTC Comment hidden (obsolete)
Comment 15 Jared Camins-Esakov 2013-01-25 12:50:29 UTC Comment hidden (obsolete)
Comment 16 Katrin Fischer 2013-01-25 17:00:59 UTC
First tests look promising :)
Comment 17 Katrin Fischer 2013-01-30 06:57:07 UTC
Hi Jared,

there is a little problem on updating the koha-conf.xml file for existing installations. I have tested this with 2 different installations now and updating the koha-conf.xml file to add the query parser line does not work for me.

To reproduce:

1) Apply all patches
2) (Run database update)
3) Update koha-conf.xml:

perl Makefile.PL --prev-install-log ../koha-dev/misc/koha-install-log
make
make test
sudo make upgrade

It prompted me for dom and icu (which I thought I had already activated). It completed successfully, but checking the generated file, the tag for the query parser is missing.

Can you please take a look what's going wrong here? I will try to continue testing with a new installation.
Comment 18 Katrin Fischer 2013-01-30 07:32:05 UTC
I set up a new dev installation with Normarc and the conf file still doesn't show anything related to the query parser:

Last lines of my koha-conf.xml file:
 <zebra_bib_index_mode>dom</zebra_bib_index_mode>
 <zebra_auth_index_mode>dom</zebra_auth_index_mode>
</config>
</yazgfs>

When I added the lines you gave me on IRC I was able to successfully test some searches with the new query syntax, so I think this might be the only piece missing?
Comment 19 Elliott Davis 2013-01-30 15:11:19 UTC Comment hidden (obsolete)
Comment 20 Elliott Davis 2013-01-30 15:11:40 UTC Comment hidden (obsolete)
Comment 21 Elliott Davis 2013-01-30 15:12:00 UTC Comment hidden (obsolete)
Comment 22 Elliott Davis 2013-01-30 15:12:22 UTC Comment hidden (obsolete)
Comment 23 Elliott Davis 2013-01-30 15:12:40 UTC Comment hidden (obsolete)
Comment 24 Elliott Davis 2013-01-30 15:15:04 UTC
Applied to fix a couple of merge issues.  Appears to work as advertised.  Provided test plan passes.  I didn't experience the issues that katrin did but I already had those fields in my koha-conf xml file.
Comment 25 Elliott Davis 2013-01-30 15:46:01 UTC
I went back and removed a line from my conf file that I had from testing previously (which is why it worked for me initially) and re-ran make and got the same error as KF.  

Apologies for the mis-informed test.  I think there should be a follow up to add this line to make but shouldn't be a reason to fail the test.

(In reply to comment #24)
> Applied to fix a couple of merge issues.  Appears to work as advertised. 
> Provided test plan passes.  I didn't experience the issues that katrin did
> but I already had those fields in my koha-conf xml file.
Comment 26 Elliott Davis 2013-01-30 15:46:01 UTC
I went back and removed a line from my conf file that I had from testing previously (which is why it worked for me initially) and re-ran make and got the same error as KF.  

Apologies for the mis-informed test.  I think there should be a follow up to add this line to make but shouldn't be a reason to fail the test.

(In reply to comment #24)
> Applied to fix a couple of merge issues.  Appears to work as advertised. 
> Provided test plan passes.  I didn't experience the issues that katrin did
> but I already had those fields in my koha-conf xml file.
Comment 27 Jared Camins-Esakov 2013-02-07 00:55:46 UTC Comment hidden (obsolete)
Comment 28 Chris Cormack 2013-02-17 08:07:58 UTC Comment hidden (obsolete)
Comment 29 Katrin Fischer 2013-02-27 07:40:05 UTC
Starting testing on this again...
Comment 30 Jared Camins-Esakov 2013-03-04 22:03:19 UTC Comment hidden (obsolete)
Comment 31 Chris Cormack 2013-03-04 22:29:02 UTC Comment hidden (obsolete)
Comment 32 Katrin Fischer 2013-03-06 07:41:25 UTC
Working on this again... we are getting close!
Comment 33 Jared Camins-Esakov 2013-03-07 12:23:32 UTC Comment hidden (obsolete)
Comment 34 Jared Camins-Esakov 2013-03-09 15:37:32 UTC Comment hidden (obsolete)
Comment 35 Jared Camins-Esakov 2013-03-10 13:18:35 UTC Comment hidden (obsolete)
Comment 36 Jared Camins-Esakov 2013-03-11 22:36:39 UTC Comment hidden (obsolete)
Comment 37 Jared Camins-Esakov 2013-03-14 12:45:08 UTC Comment hidden (obsolete)
Comment 38 Jared Camins-Esakov 2013-03-14 14:13:37 UTC Comment hidden (obsolete)
Comment 39 Jared Camins-Esakov 2013-03-14 14:51:25 UTC Comment hidden (obsolete)
Comment 40 Jared Camins-Esakov 2013-03-14 16:14:34 UTC Comment hidden (obsolete)
Comment 41 Jared Camins-Esakov 2013-03-14 16:16:09 UTC Comment hidden (obsolete)
Comment 42 Jared Camins-Esakov 2013-03-14 18:04:31 UTC Comment hidden (obsolete)
Comment 43 Katrin Fischer 2013-03-14 21:36:38 UTC Comment hidden (obsolete)
Comment 44 Katrin Fischer 2013-03-14 21:36:58 UTC Comment hidden (obsolete)
Comment 45 Katrin Fischer 2013-03-14 21:37:06 UTC Comment hidden (obsolete)
Comment 46 Katrin Fischer 2013-03-14 21:37:17 UTC Comment hidden (obsolete)
Comment 47 Katrin Fischer 2013-03-14 21:37:32 UTC Comment hidden (obsolete)
Comment 48 Katrin Fischer 2013-03-14 21:37:40 UTC Comment hidden (obsolete)
Comment 49 Katrin Fischer 2013-03-14 21:37:48 UTC Comment hidden (obsolete)
Comment 50 Katrin Fischer 2013-03-14 21:37:55 UTC Comment hidden (obsolete)
Comment 51 Katrin Fischer 2013-03-14 21:38:05 UTC Comment hidden (obsolete)
Comment 52 Katrin Fischer 2013-03-14 21:38:30 UTC Comment hidden (obsolete)
Comment 53 Katrin Fischer 2013-03-14 21:38:38 UTC Comment hidden (obsolete)
Comment 54 Katrin Fischer 2013-03-14 21:38:45 UTC Comment hidden (obsolete)
Comment 55 Katrin Fischer 2013-03-14 21:38:53 UTC Comment hidden (obsolete)
Comment 56 Katrin Fischer 2013-03-14 21:39:08 UTC Comment hidden (obsolete)
Comment 57 Katrin Fischer 2013-03-14 21:39:16 UTC Comment hidden (obsolete)
Comment 58 Katrin Fischer 2013-03-14 21:39:24 UTC Comment hidden (obsolete)
Comment 59 Jared Camins-Esakov 2013-03-17 01:26:45 UTC
Created attachment 16180 [details] [review]
Bug 9239: Add QueryParser module to Koha

Although the OpenILS::QueryParser class is going to be moved into a
separate CPAN distribution, this hasn't been done yet, and for ease of
testing it makes sense to include the class in Koha for the moment.

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Elliott Davis <elliott@bywatersolions.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Comment 60 Jared Camins-Esakov 2013-03-17 01:26:58 UTC
Created attachment 16181 [details] [review]
Bug 9239: Introduce QueryParser driver for PQF

Since the most expressive query language supported by Zebra is PQF, this
patch adds a PQF driver for QueryParser which will translate QueryParser
queries into standard PQF (guided by mappings which have been written to
match Koha's existing Zebra configuration) which can then be sent to
Zebra. This driver, Koha::QueryParser::Driver::PQF(::*) extends the
OpenILS::QueryParser(::*) class(es), so as to preserve maximum
interoperability between the various users of the QueryParser driver.

Initially, search syntax is as follows:
* AND operator: &&
* OR operator: ||
* GROUPING operators: ( )

Fields can mostly be searched using the ccl prefixes they have now. The
exception is the various date limits which are searched with a syntax
like this: pubdate(2008)

For sorting, you can simply add #title-sort-az (etc.) to your query.

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Elliott Davis <elliott@bywatersolions.com>
Test Passed successfully after installing missing dep for Test::Deep

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Comment 61 Jared Camins-Esakov 2013-03-17 01:27:07 UTC
Created attachment 16182 [details] [review]
Bug 9239: Add syspref to enable QueryParser

In light of the experimental nature of the QueryParser code at this
juncture, it behooves Koha to have the option of disabling QP, at least
for the purpose of comparing results between QP-enabled and QP-disabled
searches. This patch adds a UseQueryParser syspref to do just that. In
cases where the user enables QP without having QP installed, however,
there will be no dramatic failure, and Koha will simply fall back to
using the non-QP search code.

In order to reduce the overhead added by the introduction of QueryParser,
this patch adds a C4::Context->queryparser accessor to a static
QueryParser object which is initialized the first time it is required
and then reset thereafter.

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Elliott Davis <elliott@bywatersolions.com>
Fixed merge conflict in sysprefs.sql and updatedatabase.sql

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Fixed merge conflict in sysprefs.sql und updatedatabase.pl
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Comment 62 Jared Camins-Esakov 2013-03-17 01:27:15 UTC
Created attachment 16183 [details] [review]
Bug 9239: Allow the use of QueryParser for all queries

With the inclusion of this patch, all searches will (try) to use
QueryParser for handling queries for both the bibliographic and authority
databases if UseQueryParser is enabled. If QueryParser is unavailable,
UseQueryParser is disabled, or the search uses CCL indexes, the old
search code will be used.

To test:
1) Apply patch.
2) Run the unit test with `prove t/QueryParser.t`
3) Enable the UseQueryParser syspref.
4) Try searches that should return results in the following places:
   * OPAC (simple search)
   * OPAC (advanced search)
   * OPAC (authorities)
   * Staff client (header search)
   * Staff client (advanced search)
   * Staff client (cataloging search)
   * Staff client (authorities)
   * Staff client (importing a batch using a match point)
   * Staff client (searching for an item for adding to a label)
   * Staff client (acquisitions)
   * Staff client (searching for a record to create a serial)
   * ANYWHERE ELSE I HAVE FORGOTTEN
5) Disable the UseQueryParser syspref. Repeat at least some of the
   searches you did above.
6) If all searches worked, sign off.

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Elliott Davis <elliott@bywatersolions.com>
Searching still works as expected for variuos places.
QueryParser syspref seemed to be enabled by default

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Comment 63 Jared Camins-Esakov 2013-03-17 01:27:24 UTC
Created attachment 16184 [details] [review]
Bug 9239 follow-up: POD and perlcritic fixes

There were several perlcritic violations for returning explicit undef,
and splitting up the various driver classes resulted in POD compliance
errors, where the POD was starting with head2 rather than head1. This
patch fixes those errors (but has no functional impact).

Signed-off-by: Elliott Davis <elliott@bywatersolions.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Comment 64 Jared Camins-Esakov 2013-03-17 01:27:32 UTC
Created attachment 16185 [details] [review]
Bug 9239 follow-up: update koha-conf.xml

The previous patches were missing the koha-conf.xml updates. This patch
updates koha-conf.xml and makes the changes neccessary to include the
QueryParser configuration file in the packages.

To test:
1) Run Makefile and check generated koha-conf.xml to confirm that the
   line <queryparser_config>...</queryparser_config> is there with an
   absolute path.
2) That was it.

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Tested update successfully - new line now shows up and
query parser is used.
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Comment 65 Jared Camins-Esakov 2013-03-17 01:27:47 UTC
Created attachment 16186 [details] [review]
Bug 9239 QA follow-up: fix highlighting and merge error

QP searches with && broke search highlighting on the OPAC details page.
This patch corrects encoding of the query_desc parameter that is passed
to the details page.

My last attempt at rebasing also transposed the variable for index
names with the variable for operators, meaning that the dropdown in
the basic search did not work.

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Fixes some problems raised during QA successfully.
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Comment 66 Jared Camins-Esakov 2013-03-17 01:27:57 UTC
Created attachment 16187 [details] [review]
Bug 9239 QA-follow-up: quiet warnings and fix problem with limits

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Comment 67 Jared Camins-Esakov 2013-03-17 01:28:05 UTC
Created attachment 16188 [details] [review]
Bug 9239 QA follow-up: escape CGI input

Koha was not previously escaping CGI input, which caused problems for
highlighting and is a security issue.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Thx for fixing this.
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Comment 68 Jared Camins-Esakov 2013-03-17 01:28:18 UTC
Created attachment 16189 [details] [review]
Bug 9239 QA follow-up: the last QA follow-up was missing a require

This patch also corrects the definition of the an= index, which was
missing exactness.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Comment 69 Jared Camins-Esakov 2013-03-17 01:28:30 UTC
Created attachment 16190 [details] [review]
Bug 9239 QA follow-up: don't fall back to kw if field is specified

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Comment 70 Jared Camins-Esakov 2013-03-17 01:28:37 UTC
Created attachment 16191 [details] [review]
Bug 9239 QA follow-up: remove stray debug code

Remove a line of debug code from EG, provide better error handling
when presented with weird data in the authority linker, and correct
queryparser configuration to reflect the correct configuration for
Zebra.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Comment 71 Jared Camins-Esakov 2013-03-17 01:28:46 UTC
Created attachment 16192 [details] [review]
Bug 9239 QA follow-up: better error handling

* Fix a long-standing bug in the linker that could crash the linker when
run against odd data.
* Sanitize input to SimpleSearch.
* Correctly handle CCL indexes with QP.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Comment 72 Jared Camins-Esakov 2013-03-17 01:28:56 UTC
Created attachment 16193 [details] [review]
Bug 9239 QA follow-up: escape quotes in searches

This patch escapes quotes in searches for those rare instances where
there is a literal quote inside a search term.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Comment 73 Jared Camins-Esakov 2013-03-17 01:29:08 UTC
Created attachment 16194 [details] [review]
Bug 9239 QA follow-up: amend error handling

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Comment 74 Jared Camins-Esakov 2013-03-17 01:29:15 UTC
Created attachment 16195 [details] [review]
Bug 9239 QA follow-up: fix overeager quote escaping

The quote escaping added two follow-ups ago was a little too eager,
and escaped perfectly valid quotes in some instances. This patch moves
the escaping deeper into the loop so that no needed quotes will be
escaped.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Test plan included here, as this is the last patch in the series:

All tests pass.

OPAC
Simple search
- Keyword searches using boolean operators || and && - OK
- Pubdate searches using pubdate() - OK
- Using different search options: keyword, title, etc. - OK

Facets
- limit to available from a query parser simple search - OK
- Other facets - OK

Advanced search
- Date range search - OK
- Itemtype limits - OK
- Language limit - OK
- Combining various serach options with and without 'more' options
  and using boolean operators - OK

Authorities
- Searching for partial and complete names like "M", "Benedictus" works. - OK
- Biblio count isc correct - OK
- Links to linked biblios work. - OK.

INTRANET
Staged MARC import
- Import records using various matching points - OK

Serials
- Search for a serial in subscription form - OK

Acquisitions
- Search for existing record from basket - OK

Authorities
- Running the linker script with QP on works. - OK
- Searching for authorities works. - OK
- Biblio count isc correct - OK
- Links to linked biblios work. - OK.

NOTE: As agreed with Jared in chat, query parser should be
off by default for the next release.
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Comment 75 Jared Camins-Esakov 2013-03-17 01:39:08 UTC
This patch has been pushed to master.
Comment 76 Jared Camins-Esakov 2013-03-17 19:39:36 UTC
If you are using a git install and do not run the installer, you will need to update koha-conf.xml and add the following line (modified to point to your git clone) before "</config>" near the bottom of the koha-conf.xml file:
 <queryparser_config>/home/jcamins/kohaclone/etc/searchengine/queryparser.yaml</queryparser_config>