Bug 11619

Summary: Duplicate key in QueryParser config
Product: Koha Reporter: Marcel de Rooy <m.de.rooy>
Component: SearchingAssignee: Marcel de Rooy <m.de.rooy>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: trivial    
Priority: P5 - low CC: fridolin.somers, gmcharlt, jonathan.druart, katrin.fischer
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Bug 11619: Duplicate key in QueryParser config
Bug 11619: Duplicate key in QueryParser config
Bug 11619: Duplicate key in QueryParser config
Bug 11619: Duplicate key in QueryParser config
[PASSED QA] Bug 11619: Duplicate key in QueryParser config

Description Marcel de Rooy 2014-01-27 12:50:21 UTC
Found by Jonathan with some hints from koha-qa:

duplicate key is language:

 647     language:
 648       bib1_mapping:
 649         biblioserver:
 650           1: 54
 651       enabled: 1
 652       index: language
 653       aliases:
 654         - language
 655         - ln
 656       label: Language
 657     language:
 658       bib1_mapping:
 659         biblioserver:
 660           1: 1095
 661       enabled: 1
 662       aliases:
 663         - language-original
 664       index: language-original
 665       label: Language of original
Comment 1 Marcel de Rooy 2014-01-27 12:53:45 UTC Comment hidden (obsolete)
Comment 2 Marcel de Rooy 2014-01-27 13:08:19 UTC Comment hidden (obsolete)
Comment 3 Marcel de Rooy 2014-01-27 13:30:10 UTC
Comment on attachment 24747 [details] [review]
Bug 11619: Duplicate key in QueryParser config

Still yaml warnings..
Comment 4 Marcel de Rooy 2014-01-27 13:31:56 UTC
I saw some 'suspect' lines around author too.
But I do not seem to get rid of the YAML warnings from koha-qa.pl
I am leaving this to the experts for now :)
Comment 5 Jonathan Druart 2014-01-27 13:57:58 UTC
(In reply to M. de Rooy from comment #4)
> I saw some 'suspect' lines around author too.
> But I do not seem to get rid of the YAML warnings from koha-qa.pl
> I am leaving this to the experts for now :)

I think your first patch is right.
The warning from koha-qa is normal. The tests are done before the patch AND after the patch. The warnings only appears 1 time, so before applying the patch. After the patch, it does not appear anymore.
Comment 6 Marcel de Rooy 2014-01-27 14:05:14 UTC
Comment on attachment 24746 [details] [review]
Bug 11619: Duplicate key in QueryParser config

OK I will obsolete this one.
Comment 7 Marcel de Rooy 2014-01-27 14:07:20 UTC
This test plan is nonsense ('stupid') by the way :)
Comment 8 Jonathan Druart 2014-01-27 14:21:57 UTC Comment hidden (obsolete)
Comment 9 Katrin Fischer 2014-02-02 14:52:21 UTC
I have problems in getting the search to work:

Tested with MARC21:
- Applied patch and copied file
- Cataloged record Language-original = 041$h
- Activated QueryParser
- Waited for Zebra to reindex my newly added record
- Searched for the unique content of 041$h as keyword search
  > Record was found
- Searched for language-original:<041$h>
  > Record was not found

I have no success in using the index explicitly.
Comment 10 Marcel de Rooy 2014-02-03 07:56:15 UTC
(In reply to Katrin Fischer from comment #9)
> I have problems in getting the search to work:
> 
> Tested with MARC21:
> - Applied patch and copied file
> - Cataloged record Language-original = 041$h
> - Activated QueryParser
> - Waited for Zebra to reindex my newly added record
> - Searched for the unique content of 041$h as keyword search
>   > Record was found
> - Searched for language-original:<041$h>
>   > Record was not found
> 
> I have no success in using the index explicitly.

Thanks for testing, Katrin.
I have to admit that my test plan should be reworded (see comment 7). Will do that in a minute. Sorry for any confusion about that..
Note that this report is about fixing the QueryParser config and it is not about searching on language-original. As long as this specific index is not in the field set of C4::Search::getIndexes, you will have no results (without QueryParser). But that is true for many more indexes in record.abs. Just mentioning a few indexes with the same behavior from the first few control fields: Microform-generation, Identifier-standard, BNB-card-number, BGF-number, Number-db, Number-natl-biblio, etc.
So I feel that since this fixes QP, it should not be a reason to fail this patch.
Comment 11 Marcel de Rooy 2014-02-03 08:00:44 UTC Comment hidden (obsolete)
Comment 12 Katrin Fischer 2014-02-03 08:16:37 UTC
Hi Jonathan, I understand that we have to list the index in Search.pm when not using the QueryParser. But now I am more confused as to why it didn't work in my test?
Comment 13 Marcel de Rooy 2014-02-03 08:49:09 UTC
(In reply to Katrin Fischer from comment #12)
> Hi Jonathan, I understand that we have to list the index in Search.pm when
> not using the QueryParser. But now I am more confused as to why it didn't
> work in my test?

Katrin,
I retested. If I enable QP and search on the index, I find the record with the search term in 041$h.
If I disable QP, I do not find it.
So please retest.
Comment 14 Katrin Fischer 2014-02-03 08:53:14 UTC
Can you add the search term/query you used?
Comment 15 Marcel de Rooy 2014-02-03 08:55:17 UTC
(In reply to Katrin Fischer from comment #14)
> Can you add the search term/query you used?

I just simply added dut in 041$h and search for language-original: dut
Comment 16 Katrin Fischer 2014-02-03 08:57:43 UTC
Hm weird, I will retest. Jonathan, did you make any changes to the new patch?
Comment 17 Jonathan Druart 2014-02-03 09:05:37 UTC
(In reply to Katrin Fischer from comment #16)
> Hm weird, I will retest. Jonathan, did you make any changes to the new patch?

No, I always mention when I amend a patch.
Comment 18 Marcel de Rooy 2014-02-03 16:51:15 UTC
(In reply to Katrin Fischer from comment #16)
> Hm weird, I will retest. Jonathan, did you make any changes to the new patch?

Please do not forget to copy the queryparser.yaml to your zebra install.
You could restart Zebra but I do not think that it was necessary.
Edit a record after that and search..
Comment 19 Katrin Fischer 2014-02-03 20:31:08 UTC
It still doesn't work in my test installation:

- Checked that the file was copied correctly

- Checked my koha-conf.xml for the QueryParser line.
 <queryparser_config>/home/katrin/koha-dev/etc/searchengine/queryparser.yaml</queryparser_config>

- Checked QueryParser is activated.
- Keyword search finds my 041$h result.
- language-original:dedede doesn't.
- Changed the record to have 'dut' instead.
- Reindexed with -z
- Keyword search finds dut.
- language-original:dut gives no result.

- No errors in my koha-opac-error_log.

Marcel, does the space make a difference for you? language-original: dut vs. language-original:dut?
Comment 20 Katrin Fischer 2014-02-03 20:42:57 UTC
*sigh* outdated index configuration... works alright now :)
Comment 21 Katrin Fischer 2014-02-03 20:44:06 UTC
Created attachment 25016 [details] [review]
[PASSED QA] Bug 11619: Duplicate key in QueryParser config

Corrects a double entry for language in yaml file.
Language should have been language-original.

Test plan:
Check that you have language-original in your zebra install.
Specifically, this index should cover MARC21 041$h.
Enable QueryParser and search for a record with this index.
Note that this patch does not enable searching on this
index without QueryParser. This is true for many more indexes
in record.abs that are not included in the getIndexes routine.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Works as described - make sure you are testing with a current
indexing configuration.
Comment 22 Marcel de Rooy 2014-02-04 13:19:25 UTC
(In reply to Katrin Fischer from comment #20)
> *sigh* outdated index configuration... works alright now :)

Thanks for your perseverance :)
Comment 23 Galen Charlton 2014-02-04 18:46:27 UTC
Pushed to master, along with a follow-up fixing a test case that the main patch broke.

Thanks, Marcel!
Comment 24 Marcel de Rooy 2014-02-05 11:29:12 UTC
(In reply to Galen Charlton from comment #23)
> Pushed to master, along with a follow-up fixing a test case that the main
> patch broke.

Thanks, Galen.
I remove the non-mandatory dependency for 10544. It can also be applied on top of this..
Comment 25 Fridolin Somers 2014-04-30 15:08:34 UTC
Pushed to 3.14.x, will be in 3.14.06