I'm getting increasingly familiar with the QueryParser and its configuration, so I'll have to take a look at this one in the future...

It would be extraordinarily fiddly to test with actual queries, but a careful checking through of ccl.properties, bib1.att, and queryparser.yaml should be enough to give a sign off, I think.

to test I suppose you could do that;
run zebrasrv -f $KOHA_CONF
Make some searches with and without QP activated and check if the query are the same in the log printed in terminal by zebrasrv

M. Saby

Created attachment 35373 [details] [review]
Bug 11193 : edit Queryparser configuration to take changes of bug 8252 into account

This patch make some changes to queryparser.yaml, to take into account the changes made in ccl.properties
* new indexes : bio, char-encoding, country-publication, cumulative-index-availability, ff7-00, ff7-01, ff7-02, ff7-01-02, ff8-18, ff8-18-21, ff8-19, ff8-21, ff8-28,ff8-29,ff8-30,ff8-31, ta, lf, music-number, title-page-availability, script-title, tpubdate, rtype
* deleted indexes : audience and ff8-22 (alias of ta), biography and ff-834 (alias of bio), format (alias of ff8-23), l-format (alias of ff7-01-02), illustration-code (alias of ff8-18-21), regularity-code, type-of-serial (alias of ff8-21), literature-code (alias of lf), music (alias of music-number), record-type (alias of rtype)
* changed index : graphics-type, graphics-support, video-mt

I am not sure if I understood well the behavior of this file, and of the best way for testing the changes made...

Rebased a trivial merge conflict in the yaml file.

Note that running koha qa tools on this patch produces quite some output.
And finally:
 FAIL   etc/searchengine/queryparser.yaml
   FAIL   yaml_valid
                YAML Error: Inconsistent indentation level

Moving to FQA to express need for clarification.

I'll take a look next week end
Thanks for testing.

Mathieu

Generally speaking, I find that QP behavior and configuration is not well documented, especially for a librarian like me...
My personal opinion is that QP having been introduced in 2012, it should be well documented and bug-proof, and the older code should have been deprecated.

Mathieu

(In reply to mathieu saby from comment #7)
> My personal opinion is that QP having been introduced in 2012, it should be
> well documented and bug-proof, and the older code should have been
> deprecated.
> 
> Mathieu

That would imply that someone was still working on QP ;). As far as I know, no one is actually actively maintaining or enhancing that code.

I have some ideas about how to do that, but I don't have the time/money/sponsorship/whatever to do it.

The older code is still used in many many cases even when QP is activated, so deprecating the older code is really not an option at this stage.

As for documentation, http://evergreen-ils.org/dokuwiki/doku.php?id=documentation:technical:search_grammar might give you some guidance, although obviously it won't be the same since it's implemented differently in Koha. As far as I know, Evergreen doesn't have a search engine per se. Rather, it uses special tables and full-text indexing within PostgreSQL, so a lot of things (like facets) are totally irrelevant for Koha.

But yeah... in the end there's no one who is currently maintaining QP, as far as I know, so I wouldn't expect too much from it at the moment.

(In reply to M. de Rooy from comment #5)
> Rebased a trivial merge conflict in the yaml file.
> 
> Note that running koha qa tools on this patch produces quite some output.
> And finally:
>  FAIL   etc/searchengine/queryparser.yaml
>    FAIL   yaml_valid
>                 YAML Error: Inconsistent indentation level
> 
> Moving to FQA to express need for clarification.

Why are there two patches at the moment? Is one with a correct indentation level and the other is not?

(In reply to David Cook from comment #9)
> Why are there two patches at the moment? Is one with a correct indentation
> level and the other is not?

No, old one is obsolete.

> But yeah... in the end there's no one who is currently maintaining QP, as
> far as I know, so I wouldn't expect too much from it at the moment.

Well, searching is a pretty important feature in a ILS ;-) So I hope somebody will take care of that. Even if there is no formal maintainer, at least writing up-to-date documentation about how it works and how configuring it would be great...

I'm experiencing difficulties with my VM, but I will try to fix this patch today.

Mathieu

(In reply to mathieu saby from comment #11)
> Well, searching is a pretty important feature in a ILS ;-) So I hope
> somebody will take care of that. Even if there is no formal maintainer, at
> least writing up-to-date documentation about how it works and how
> configuring it would be great...
> 
> I'm experiencing difficulties with my VM, but I will try to fix this patch
> today.
> 
> Mathieu

Yes, searching is an important feature in an ILS.

If you're having issues with the QueryParser, I would recommend turning it off or paying someone to fix it/improve it.

Yes, up-to-date documentation would be great, but that's not going to appear out of the void. Someone has to put their time and energy into doing it. Personally, I think the great thing about open source is that anyone can contribute. If something isn't working, a person can examine the code, work it out, patch it, and hopefully share their work with others.

I think where things fall apart a bit is when people expect others to do everything for them for nothing. It's great when someone with knowledge, skill, and expertise shares their work or invests their time and energy into a project, but it's really their choice.

I suppose what I'm trying to say is that there's no such thing as a free lunch. That is to say, even when we're receiving something for free, it's the product of human labour. In my case, I'm paid to work on Koha 8 hours a day 5 days a week. In the course of that work, I'll upstream bug fixes, enhancements, and features. That is, I'll contribute to Koha for free, but it's coming at someone's expense. Now, there are a lot of great people who volunteer on Koha on their own time, and I think that should be applauded. Those are people are the best. But even then... they're going to work on what they want to work on most likely. 

It seems clear to me that the QueryParser is not a critical issue at the moment. If it were, someone would be paying a company to work on it, or they would be volunteering their free time to work on it. Neither is happening at the moment.

Now, I'm actually pretty interested in the QueryParser. The current way Koha build queries is very suboptimal. However, it's not so obviously suboptimal that anyone is willing to pay anyone to fix it. So query parsing/building is in a state of limbo. I think about steering clients in the direction of fixing query parsing and building, but I have other projects on at the moment. Likewise, I am not willing to sacrifice my non-work time to work on Koha anymore.

In fact, of the two or three people I can think of who have worked on the QueryParser, I think none of them volunteer their free time on Koha at the moment.

So I return to the options of turning it off, paying someone to fix it/improve it, or learning how to do it yourself. I was curious about it, so I taught myself about it. Again, that's the glory of open source.

Even in the case of documentation, it takes resources to build and maintain documentation. 

If you're interested in bringing the documentation about the QueryParser up-to-date, I would suggest creating a wiki page, and start building up the documentation. It can be improved over time. Next time I'm working with/on it, maybe I'll take a look.

(In reply to David Cook from comment #12)
> (In reply to mathieu saby from comment #11)
> I suppose what I'm trying to say is that there's no such thing as a free
> lunch. That is to say, even when we're receiving something for free, it's
> the product of human labour. In my case, I'm paid to work on Koha 8 hours a
> day 5 days a week. In the course of that work, I'll upstream bug fixes,
> enhancements, and features. That is, I'll contribute to Koha for free, but
> it's coming at someone's expense. Now, there are a lot of great people who
> volunteer on Koha on their own time, and I think that should be applauded.
> Those are people are the best. But even then... they're going to work on
> what they want to work on most likely. 

With all respect, but if Matthieu is such a person (volunteering on Koha in its own time now?), I could imagine that your reply would not really be interpreted as being applauded to. But I could be wrong :)

(In reply to M. de Rooy from comment #13)
> (In reply to David Cook from comment #12)
> > (In reply to mathieu saby from comment #11)
> > I suppose what I'm trying to say is that there's no such thing as a free
> > lunch. That is to say, even when we're receiving something for free, it's
> > the product of human labour. In my case, I'm paid to work on Koha 8 hours a
> > day 5 days a week. In the course of that work, I'll upstream bug fixes,
> > enhancements, and features. That is, I'll contribute to Koha for free, but
> > it's coming at someone's expense. Now, there are a lot of great people who
> > volunteer on Koha on their own time, and I think that should be applauded.
> > Those are people are the best. But even then... they're going to work on
> > what they want to work on most likely. 
> 
> With all respect, but if Matthieu is such a person (volunteering on Koha in
> its own time now?), I could imagine that your reply would not really be
> interpreted as being applauded to. But I could be wrong :)

Well, I've got no time for polemic, and, as Marcel put it, I am still doing some work on Koha, but only on my free time, and about some old patches I wrote when I was in charge. And I'm moving to another town, so that's not the best circumstancies for me...
What I wrote for QP was not aimed to hurt anybody, it was a general remark for the "community".

Mathieu

(In reply to mathieu saby from comment #14)
> Well, I've got no time for polemic, and, as Marcel put it, I am still doing
> some work on Koha, but only on my free time, and about some old patches I
> wrote when I was in charge. And I'm moving to another town, so that's not
> the best circumstancies for me...
> What I wrote for QP was not aimed to hurt anybody, it was a general remark
> for the "community".
> 
> Mathieu

My apologies if my response appeared overly charged. I suppose I did take exception to the lines "it should be well documented and bug-proof, and the older code should have been deprecated", and further exception to the lines "So I hope somebody will take care of that. Even if there is no formal maintainer, at least writing up-to-date documentation about how it works and how configuring it would be great...".

In hindsight, I'm sure that your remarks were motivated by a desire to be helpful rather than demanding or critical. In the moment, I read them as the latter. I read a great article years ago about how "should" is often an unhelpful word as it implies obligation and criticism. It often doesn't feel good to be told that you "should" do something, especially when you're volunteering your time for free to help where you can. 

I agree that it would be helpful to have up-to-date documentation and for the QP code to be bug-free. As I mentioned, I even have lots of ideas about how to do it, but there's no sponsorship for it. I think that's a key part of why it wasn't documented or finished in the first place. No one cared enough about it to support it. As a librarian and a developer, I care quite a bit about search. I've read through the QP code and the Zebra code, so that I might understand it as thoroughly as possible. So now I understand what works, what doesn't work, and a fair bit about what needs to be done, but there's no support. Mostly, there are demands and critiques, and neither of those are helpful.

So, my apologies again for the length and tone of my past comments. I am probably overly sensitive when it comes to search in Koha and expectations with open source. I'm not an adherent to the Linus Torvald style of communication, so I probably should have been nicer ;).

Created attachment 35609 [details] [review]
[PATCH 2/2] Bug 11193 : Followup - fix tests

Bug 11193 : Followup - fix tests

This patch fixes one broken line in YAML configuration file
It also changes the number of fields for keyword class from 111 to 121 in QueryParser.t

Here is a followup, but I cannot guide anyone further about the way of testing the patch...
I suggested this way in an earlier comment: to test I suppose you could do that;
run zebrasrv -f $KOHA_CONF
Make some searches with and without QP activated and check if the query are the same in the log printed in terminal by zebrasrv


Mathieu

And apologies accepted. Let's forget that!
Mathieu

I've applied the patches against master 3.19.00.011 head 7741.

(In reply to mathieu saby from comment #17)
> [...] to test I suppose you could do the run:
> zebrasrv -f $KOHA_CONF

Thanks, Mathieu! OK.

> Make some searches with and without QP activated [....]
 
1) I've thought that QP activating was about setting the syspref "UseQueryParser" to "Do not try" or to "Try":
Is it right?

2) But, on ADMIN, I've read this syspref UseQueryParser's note:
UseQueryParser : to use the QueryParser module for parsing queries. Please note: enabling this will have no impact if you do not have QueryParser installed, and everything will continue to work as usual. 

Which QueryParser had I to install?
Perhaps one of the 2 following ones?
>apt-cache search queryparser
liblucene-queryparser-perl - Turn a Lucene query into a Perl data structure
libsearch-queryparser-perl - Perl module to parse URI query strings into a data structure

3) My installation is a MARC21 one. 
I tested the "bio" and "tpubdate" indexes.
bio was OK: it found biblio records.
tpubdate found none (ERROR).

Is this error relating to this bug? 
Or is this bug OK?

Thanks in advance. I keep the current "Needs Signoff" status for now.

I've applied the patches against master 3.19.00.011 head 7741.

My installation is a MARC21 one.

The QP-file was the one in the $KOHA_KONF's XML element:
<queryparser_config>/.....-dev/etc/searchengine/queryparser.yaml</queryparser_config>

which I had linked to the truly patched QP-file [not in the dev library]:
/..../etc/searchengine/queryparser.yaml
via a symb link.

I've set the syspref "UseQueryParser" to "Do not try" and to "Try", as required, to test.

The following 5 search test cases had been submitted to the intranet catalog's simple search.

To the log activated by:
>zebrasrv -f $KOHA_KONF
these were these differences between "Try" and "Do not try":
1) bio:a
try: RPN @attrset Bib-1 @attr 4=6 @attr 5=1 @attr 1=8834 @attr 2=102 a
dnt: RPN @attrset Bib-1 @attr 1=8834 a
2) aceee [i.e. a single word in simple search...]
try: RPN @attrset Bib-1 @or @or @attr 4=6 @attr 5=1 @attr 1=1016 @attr 2=102 
aceee @attr 4=6 @attr 5=1 @attr 9=20 @attr 2=102 aceee @attr 4=6 @attr 5=1 
@attr 9=34 @attr 2=102 aceee
dnt: RPN @attrset Bib-1 @attr 1=1016 @attr 4=6 @attr 5=1 ACEEE
3) tpubdate:c
try: RPN @attrset Bib-1 @attr 4=6 @attr 5=1 @attr 1=8806 @attr 2=102 c
dnt: RPN @attrset Bib-1 @attr 1=1016 @attr 4=6 @attr 5=1 tpubdate:c
4) pn:rossi
try: RPN @attrset Bib-1 @attr 4=6 @attr 5=1 @attr 1=Personal-name @attr 2=102 
rossi
dnt: RPN @attrset Bib-1 @attr 1=Personal-name rossi

For documentation, no differences about these other searching cases:
5) pn,wrdl:rossi [on simple search]
RPN @attrset Bib-1 @attr 1=Personal-name @attr 4=6 rossi
6) Personal Name rossi [on advanced search]:
RPN @attrset Bib-1 @attr 1=Personal-name @attr 4=6 @attr 5=1 rossi

IMO in 1), 3) and 4) desc rilevance has been added. In 3) on dnt the index 
"tpubdate" has not been recognized (IMO RIGHT) and so it has become the 
keyword index 1016.
Are those differences real errors? I'm not sure.

For now I pass the patch to "Failed QA" status.

Thank you for testing, and for all those details.
I'll try to take an other look in a few days.
For the install of queryparser, I suppose the package is installed automatically with other dependancies when you install Koha, but not sure of that...

Mathieu

We're looking to remoe QueryParser now.. as such I'm going to mark this one as RESOLVED WONTFIX