Bug 10271 - An index containing ns or nb does not work
Summary: An index containing ns or nb does not work
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor (vote)
Assignee: Fridolin Somers
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-05-17 12:02 UTC by Fridolin Somers
Modified: 2015-06-04 23:23 UTC (History)
4 users (show)

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


Attachments
Proposed patch (2.81 KB, patch)
2013-05-17 12:43 UTC, Fridolin Somers
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 10271 - An index containing ns or nb does not work (3.88 KB, patch)
2013-09-21 17:48 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 10271 - followup - Correct tabs (800 bytes, patch)
2013-09-21 17:48 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 10271 - An index containing ns or nb does not work (3.92 KB, patch)
2013-10-21 23:25 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 10271 - followup - Correct tabs (858 bytes, patch)
2013-10-21 23:26 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Fridolin Somers 2013-05-17 12:02:58 UTC
When an index does not contain a structure part, the structure "wrdl" is automatically added and a structure is mandatory to build search query (to convert ':' into '=').
But the code that tests that the structure is not already defined looks in entire index string : 
  $index =~ /(st-|phr|ext|wrdl|nb|ns)/
It should look for a comma followed by a structure and in the case of "nb" and "ns" look for an exact match.

The consequence is that an index containing ns or nb or phr or etc does not work.
Comment 1 Fridolin Somers 2013-05-17 12:43:36 UTC Comment hidden (obsolete)
Comment 2 Bernardo Gonzalez Kriegel 2013-09-20 06:28:40 UTC
Trying to test, but

Test plan :
- Desactivate all searching sysprefs.
Which ones?

- Create a new index called "ansa" number 8999 into bib1.att, ccl.properties and records.abs
How? (I mean, what's the syntax on each file?, you have an example?)
Comment 3 Fridolin Somers 2013-09-20 09:57:58 UTC
(In reply to Bernardo Gonzalez Kriegel from comment #2)
> Trying to test, but
> 
> Test plan :
> - Desactivate all searching sysprefs.
> Which ones?
 QueryAutoTruncate = only if * is added
 QueryFuzzy = Don't try
 QueryStemming = Don't try
 QueryWeightFields = Disable
 UseQueryParser = Do not try
> 
> - Create a new index called "ansa" number 8999 into bib1.att, ccl.properties
> and records.abs
> How? (I mean, what's the syntax on each file?, you have an example?)
bib1.att : att 8999 ansa
ccl.properties : ansa 1=8999
records.abs : melm 999 ansa:w,ansa:p

Thanks for testing.
Comment 4 Bernardo Gonzalez Kriegel 2013-09-21 17:48:22 UTC Comment hidden (obsolete)
Comment 5 Bernardo Gonzalez Kriegel 2013-09-21 17:48:59 UTC Comment hidden (obsolete)
Comment 6 Kyle M Hall 2013-10-21 23:25:22 UTC
Created attachment 22232 [details] [review]
Bug 10271 - An index containing ns or nb does not work

When an index does not contain a structure part, the structure "wrdl" is automatically added and a structure is mandatory to build search query (to convert ':' into '=').
But the code that tests that the structure is not already defined looks in entire index string :
  $index =~ /(st-|phr|ext|wrdl|nb|ns)/
It should look for a comma followed by a structure and in the case of "nb" and "ns" look for an exact match.
The consequence is that an index containing ns or nb or phr or etc does not work.

This patch modifies the regexp for this part and other parts looking at structures into index.

Test plan :
- Desactivate all searching sysprefs.
- Create a new index called "ansa" number 8999 into bib1.att, ccl.properties and records.abs
- Index a biblio with a value on this index, ie "VALUE"
- Perform a search on this index by editing url : http://<server>/cgi-bin/koha/catalogue/search.pl?idx=ansa&q=VALUE
=> Without patch, the search does not work. The PQF query is "@and ansa: VALUE"
=> With patch, the search works. The PQF query is "@attr 1=8999 VALUE";
- Perform same test with an index containing a structure ie "aphra"
- Set QueryAutoTruncate syspref to automatically
=> Check * is added to operand : PQF query is "@attr 1=8999 @attr 4=6 @attr 5=1 VALUE"
- You may check stopwords removal but this feature is obsolete

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Comment: as far as I can test, this works. Small tab error reported
by koha-qa, fixed in a followup.

This kind of patch is difficult to test without explicit instructions,
not everyone knows how to check what kind of PQF search is used.
I don't know. But I can test search results.

Test:
1) Deactivate search sysprefs
QueryAutoTruncate = only if * is added
QueryFuzzy = Don't try
QueryStemming = Don't try
QueryWeightFields = Disable
UseQueryParser = Do not try

2) Create new index 'ansa'
bib1.att : att 8999 ansa
ccl.properties : ansa 1=8999
records.abs : melm 999 ansa:w,ansa:p

1) and 2) from comment 3 on Bug

3) In the undestanding that index refers to field 999,
edited default framework, made 999a visible on editor

4) Edit sample record, add 'VALUE' to 999a, save, reindex

5) Search with /cgi-bin/koha/catalogue/search.pl?idx=ansa&q=VALUE
No results

6) Apply patch, repeat search
Got results

That's all I can test. If not enough for QA, then this
must wait until further and explicit test instructions

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 7 Kyle M Hall 2013-10-21 23:26:14 UTC
Created attachment 22233 [details] [review]
Bug 10271 - followup - Correct tabs

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 8 Kyle M Hall 2013-10-21 23:28:28 UTC
Passes koha-qa.pl. Seems to work as advertised. No ill affects detected.
Comment 9 Galen Charlton 2013-10-28 16:16:45 UTC
Pushed to master, along with a patch that adds a regression test.  For MARC21, there is an existing index that this patch fixes: Code-institution.

Thanks, Fridolyn!
Comment 10 Tomás Cohen Arazi 2013-12-05 18:32:35 UTC
This patch has been pushed to 3.12.x, will be in 3.12.8.

Thanks Fridolin!