Description
David Cook
2016-01-11 06:15:07 UTC
Here's my latest findings: Input: "http://libris.kb.se/resource/bib/219553" C4::Matcher::_normalize() = "HTTPLIBRISKBSERESOURCEBIB219553" Zebra CHR = "http libris kb se resource bib 219553" Zebra ICU = "http libriskbse resource bib 219553" It seems to me that the smartest thing to do is NOT to normalize with C4::Matcher::_normalize(), because we're probably going to get it wrong as we have above. Zebra indexes "http://libris.kb.se/resource/bib/219553" as "http libris kb se resource bib 219553" (CHR Phrase) or as "http libriskbse resource bib 219553" (ICU Phrase) or as "http://libris.kb.se/resource/bib/219553" (URL, which is a Charmap when using either CHR or ICU). If we query Zebra with "http://libris.kb.se/resource/bib/219553", it will normalize the query the same way that it normalized "http://libris.kb.se/resource/bib/219553" when it was originally indexing it, and we'll get a match. Of course, we can't necessarily stop using C4::Matcher::_normalize() as it's the default behaviour. Many people may count on that _normalize() without even knowing it... even if it's potentially working badly. I think what I want to do is create a new normalizer which does nothing, and call it "none" or "raw". That way, I'm passing to Zebra the same thing that it's seen before, and it will normalize it exactly the same way and the likelihood of an accurate match increases considerably. Of course, what I've said isn't 100% true. There's one more obstacle between C4::Matcher::get_matches and Zebra's normalized indexes. And that's C4::Search::SimpleSearch(), which uses the s/:/=/g regex before sending the query. While that's trivial for Zebra's CHR and ICU indexing for words and phrases, since they just strip out punctuation anyway, it's a problem for Zebra's URL Charmap, which doesn't normalize URLs. So you'll get a failed match in that case. So I think a pure match would have to be done without any normalization before the query gets to Zebra. That should be easy enough to implement in C4::Matcher when it uses C4::Search::SimpleSearch(). Because it's a simple search, it shouldn't be normalizing the query anyway. I'll just add a flag so that you can do an unnormalized search. This is turning out to be harder than expected... In C4::Matcher::get_matches, it creates a query like "qualifier,qualifier:value" for QP, or "qualifier,qualifier=value" for non-QP. That's OK... but in C4::Search::SimpleSearch, QP gets turned off by "qualifier,qualifier" so it becomes a non-QP query... but it's created with a ":" instead of a "=". This matters because we need a "=" instead of a ":" for the non-QP query... and we can't wholesale change ":" to "=" because of the URL syntax required by the URL register... I've solved this one... but now I need to write up an easy test plan... Created attachment 46848 [details] [review] Bug 15541 - Prevent normalization during matching/import process This patch allows you to use the "qualifier,qualifier" syntax in the Record Matching Rules "Search Index" when using the QueryParser. While QueryParser doesn't support this syntax, it will now fallback correctly to non-QueryParser functionality. Without the patch, your search will just fail silently. This patch also adds a "skip_normalize" flag to C4::Search::SimpleSearch(), and sets the flag during C4::Matcher::get_matches. This prevents the s/:/=/g and s/=/:/g normalization which happens in a heavy-handed way to provide correct query syntax. However, get_matches() already uses the correct syntax, so this normalization is unneeded. The normalization also mangles URLs which causes failures when using the "url" (ie "u") register in Zebra (see bug 15555). This patch also creates "raw" and "none" normalizers for the Record Matching Rules, which prevents the stripping of spaces and punctuation by Koha prior to sending queries to Zebra. This is important for a number of reasons. First and foremost, Zebra does normalization better than Koha. ICU and CHR normalize strings differently, so it's better not to try to outsmart Zebra with pre-normalization of punctuation and spaces, as it will lead to search problems. Second, when using the "u" register in Zebra, you don't want to normalize the value, since it's stored "as is" in the Zebra database. Normalization causes search failures. _TEST PLAN_ 1) Apply patches from http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15555 2) Create a record in Koha with a 024 $a http://koha-community.org/test $2 uri 3) Do a full re-index of Zebra 4) Create a Record Matching Rule in Koha with the following details: Description: Test Match threshold: 100 Record type: Bibliographic Match point 1: Search index: id-other,st-urx Score: 100 Tag: 024 Subfields: a Normalization rule: raw 5) Download your record from Koha as a .mrc file (ie isomarc, binary marc, etc) 6) Go to "Stage MARC records for import" 7) Upload your .marc file. 8) Change your "Record matching rule" to "Test" 9) Click Stage for import 10) It should say "1 records with at least one match in catalogu per matching rule "Test". Marked this with the "Academy" keyword, although it might be a bit advanced... More than happy to walk people through the Zebra stuff relating to this bug though. Created attachment 47022 [details] [review] Bug 15541 - Prevent normalization during matching/import process This patch allows you to use the "qualifier,qualifier" syntax in the Record Matching Rules "Search Index" when using the QueryParser. While QueryParser doesn't support this syntax, it will now fallback correctly to non-QueryParser functionality. Without the patch, your search will just fail silently. This patch also adds a "skip_normalize" flag to C4::Search::SimpleSearch(), and sets the flag during C4::Matcher::get_matches. This prevents the s/:/=/g and s/=/:/g normalization which happens in a heavy-handed way to provide correct query syntax. However, get_matches() already uses the correct syntax, so this normalization is unneeded. The normalization also mangles URLs which causes failures when using the "url" (ie "u") register in Zebra (see bug 15555). This patch also creates "raw" and "none" normalizers for the Record Matching Rules, which prevents the stripping of spaces and punctuation by Koha prior to sending queries to Zebra. This is important for a number of reasons. First and foremost, Zebra does normalization better than Koha. ICU and CHR normalize strings differently, so it's better not to try to outsmart Zebra with pre-normalization of punctuation and spaces, as it will lead to search problems. Second, when using the "u" register in Zebra, you don't want to normalize the value, since it's stored "as is" in the Zebra database. Normalization causes search failures. _TEST PLAN_ 1) Apply patches from http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15555 2) Create a record in Koha with a 024 $a http://koha-community.org/test $2 uri 3) Do a full re-index of Zebra 4) Create a Record Matching Rule in Koha with the following details: Description: Test Match threshold: 100 Record type: Bibliographic Match point 1: Search index: id-other,st-urx Score: 100 Tag: 024 Subfields: a Normalization rule: raw 5) Download your record from Koha as a .mrc file (ie isomarc, binary marc, etc) 6) Go to "Stage MARC records for import" 7) Upload your .marc file. 8) Change your "Record matching rule" to "Test" 9) Click Stage for import 10) It should say "1 records with at least one match in catalogu per matching rule "Test". Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com> Thanks Bouzid! I test with: File empty File contain 1 marc record File contain several marc record Created attachment 50264 [details] [review] Bug 15541: [QA Follow-up] Add new SimpleSearch argument to POD Also moved one line conform the order of arguments. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> QA Comment: Thanks for your patch, David. BTW This is imo not a trivial patch or something to signoff for an academy user, new to Koha. [1] Although your changes in SimpleSearch are very small, it would probably help if you would add a test case in t/db_dependent/Search.t. (It seems to fail some tests currently, separate from your patch.) [2] The source_normalizer or norms just lead me to two FIXMEs: # FIXME normalize, substr # FIXME - default normalizer You introduce none and raw in the regex while this field actually does nothing (and should not have been there in this stage). Your patch starts using this field, while the only thing you want is not running _normalize for (mainly?) URLs. If we do not really solve the problem of this unused field, I think we should not touch it. BTW What would be the difference between raw and none? The regex suggests more than what we offer. [3] I have some doubts about this new condition: if ($QParser && $matchpoint->{'index'} !~ m/\w,\w/) { Somehow you managed to bump in another unfinished area here too :) I would rather leave the condition as it was. In the QParser branch you could choose to ignore/remove the second specifier word with a similar regex. Changing status for now. Thanks for your comments, Marcel! I appreciate you taking the time to look at the patch. (In reply to Marcel de Rooy from comment #11) > QA Comment: > Thanks for your patch, David. > BTW This is imo not a trivial patch or something to signoff for an academy > user, new to Koha. Fair enough. I think perhaps the patch grew with time and I forgot to remove the Academy keyword. > [1] Although your changes in SimpleSearch are very small, it would probably > help if you would add a test case in t/db_dependent/Search.t. (It seems to > fail some tests currently, separate from your patch.) Hmm, also fair enough. I'll look at doing that. > [2] The source_normalizer or norms just lead me to two FIXMEs: > # FIXME normalize, substr > # FIXME - default normalizer > You introduce none and raw in the regex while this field actually does > nothing (and should not have been there in this stage). Your patch starts > using this field, while the only thing you want is not running _normalize > for (mainly?) URLs. If we do not really solve the problem of this unused > field, I think we should not touch it. BTW What would be the difference > between raw and none? The regex suggests more than what we offer. I'm not sure what you mean here... so I'll look at the code again. I think Kyle was having problems with normalization, so it's possible that he removed _normalize() in a different patch? > [3] I have some doubts about this new condition: > if ($QParser && $matchpoint->{'index'} !~ m/\w,\w/) { > Somehow you managed to bump in another unfinished area here too :) > I would rather leave the condition as it was. In the QParser branch you > could choose to ignore/remove the second specifier word with a similar regex. > Hmm, I don't think that's a good idea. You don't want to ignore/remove the second qualifier; it's important that the qualifier is in the query. You want to use it, but I don't think the QParser can do it. I think I lifted this condition from a different part of the Koha code to make the behaviour a bit more consistent. Perhaps it would be smarter to add a more universal "can_query_parse()" or something to test if the query is parseable by QParser... > Changing status for now. This fails to apply now anyway because of some of the ElasticSearch stuff... Ah, easy fix fortunately... Created attachment 50971 [details] [review] Bug 15541 - Prevent normalization during matching/import process This patch allows you to use the "qualifier,qualifier" syntax in the Record Matching Rules "Search Index" when using the QueryParser. While QueryParser doesn't support this syntax, it will now fallback correctly to non-QueryParser functionality. Without the patch, your search will just fail silently. This patch also adds a "skip_normalize" flag to C4::Search::SimpleSearch(), and sets the flag during C4::Matcher::get_matches. This prevents the s/:/=/g and s/=/:/g normalization which happens in a heavy-handed way to provide correct query syntax. However, get_matches() already uses the correct syntax, so this normalization is unneeded. The normalization also mangles URLs which causes failures when using the "url" (ie "u") register in Zebra (see bug 15555). This patch also creates "raw" and "none" normalizers for the Record Matching Rules, which prevents the stripping of spaces and punctuation by Koha prior to sending queries to Zebra. This is important for a number of reasons. First and foremost, Zebra does normalization better than Koha. ICU and CHR normalize strings differently, so it's better not to try to outsmart Zebra with pre-normalization of punctuation and spaces, as it will lead to search problems. Second, when using the "u" register in Zebra, you don't want to normalize the value, since it's stored "as is" in the Zebra database. Normalization causes search failures. _TEST PLAN_ 1) Apply patches from http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15555 2) Create a record in Koha with a 024 $a http://koha-community.org/test $2 uri 3) Do a full re-index of Zebra 4) Create a Record Matching Rule in Koha with the following details: Description: Test Match threshold: 100 Record type: Bibliographic Match point 1: Search index: id-other,st-urx Score: 100 Tag: 024 Subfields: a Normalization rule: raw 5) Download your record from Koha as a .mrc file (ie isomarc, binary marc, etc) 6) Go to "Stage MARC records for import" 7) Upload your .marc file. 8) Change your "Record matching rule" to "Test" 9) Click Stage for import 10) It should say "1 records with at least one match in catalogu per matching rule "Test". (In reply to Marcel de Rooy from comment #11) > [1] Although your changes in SimpleSearch are very small, it would probably > help if you would add a test case in t/db_dependent/Search.t. (It seems to > fail some tests currently, separate from your patch.) When I run "prove t/db_dependent/Search.t", it says that all tests are successful, although there are lots of warnings generated. I'll review the test code to see if I can add a test... > [2] The source_normalizer or norms just lead me to two FIXMEs: > # FIXME normalize, substr > # FIXME - default normalizer These FIXME messages pre-date my patch. > You introduce none and raw in the regex while this field actually does > nothing (and should not have been there in this stage). I don't understand this sentence. > Your patch starts > using this field, while the only thing you want is not running _normalize > for (mainly?) URLs. URLs are certainly the main thing I want to not normalize, but honestly nothing should be normalized during the matching, as Zebra already handles normalization for search queries. > If we do not really solve the problem of this unused > field, I think we should not touch it. BTW What would be the difference > between raw and none? The regex suggests more than what we offer. There is no difference between raw and none. They're just synonyms. I'm open to removing _normalize() from the matching completely. Honestly, I figured that the community would be against it, so I created a way of opting out of the default _normalize(). That way default system behaviour doesn't change, while some people will know how to deactivate it. > [3] I have some doubts about this new condition: > if ($QParser && $matchpoint->{'index'} !~ m/\w,\w/) { > Somehow you managed to bump in another unfinished area here too :) > I would rather leave the condition as it was. In the QParser branch you > could choose to ignore/remove the second specifier word with a similar regex. > It's not really a new condition. If you look at C4::Search::SimpleSearch, you'll see that it's already there: $QParser = C4::Context->queryparser if (C4::Context->preference('UseQueryParser') && ! ($query =~ m/\w,\w|\w=\w/)); Also, you can't remove the second specifier word, because it's essential to the query. You also can't ignore it, because the QParser can't handle it. That's why I added the same condition that's found in C4::Search::SimpleSearch. You need to catch this condition before going into the QParser code block. -- Can the QA team indicate how to proceed? I'll look at adding a unit test. However, the QParser condition or one similar to it needs to stay. As for _normalize(), I'll remove it completely, if that's a change which will be accepted by the QA team and the RM. (In reply to Marcel de Rooy from comment #11) > [1] Although your changes in SimpleSearch are very small, it would probably > help if you would add a test case in t/db_dependent/Search.t. (It seems to > fail some tests currently, separate from your patch.) I've reviewed Matcher.pm and Search.pm, an I don't think it would be possible to add a test case. The functions aren't atomic enough. The altered behaviour is internal to the functions and thus I don't see any way to automatically test it. Changing status back to Signed Off. Happy to discuss further or implement changes where necessary. (In reply to David Cook from comment #16) > When I run "prove t/db_dependent/Search.t", it says that all tests are > successful, although there are lots of warnings generated. Between April 15 and May 23 some things may have changed :) > > [2] The source_normalizer or norms just lead me to two FIXMEs: > > # FIXME normalize, substr > > # FIXME - default normalizer > These FIXME messages pre-date my patch. True. But that is obviously not my point. Although Matcher.pm has some code around norms and normalizer, actually it is just uncompleted functionality. The code just calls a simple _normalized routine. The "references to normalization subroutines (under C4::Normalize) to be applied to the source string" are not used and do not exist. > I'm open to removing _normalize() from the matching completely. Honestly, I > figured that the community would be against it, so I created a way of opting > out of the default _normalize(). That way default system behaviour doesn't > change, while some people will know how to deactivate it. I am not suggesting to remove it. As noted above, I would be hesitant to make certain changes in uncompleted functionality. > > [3] I have some doubts about this new condition: > > if ($QParser && $matchpoint->{'index'} !~ m/\w,\w/) { > > Somehow you managed to bump in another unfinished area here too :) > > I would rather leave the condition as it was. In the QParser branch you > > could choose to ignore/remove the second specifier word with a similar regex. > It's not really a new condition. If you look at C4::Search::SimpleSearch, > you'll see that it's already there: Good point. Leave it there. > Can the QA team indicate how to proceed? Could you (slightly) adjust the way to skip the normalize call for the URLs, that does not suggest that the norms functionality is present while we do not even have C4::Normalizer? Maybe Jonathan wants to jump in too and add comments? (In reply to Marcel de Rooy from comment #18) > Could you (slightly) adjust the way to skip the normalize call for the URLs, > that does not suggest that the norms functionality is present while we do > not even have C4::Normalizer? I'm not sure that I understand your question... Regardless of whether or not you provide a value for "Normalization rule" on /cgi-bin/koha/admin/matching-rules.pl, the default _normalize() function will run. While the matchpoint_component_norms.norm_routine isn't currently used in the code, I add some code that does use it for the sole purpose of detecting a "none" or "raw" normalization (ie no normalization). Are you saying that you don't want me to add that code because it suggests to people that adding a value there (besides "none" or "raw") will have an effect? I could a checkbox on the UI that says "Skip normalization rule" or something like that, but it would fulfil the same purpose of adding a "none" norm_routine in the database. Sorry, but I'm still not sure what you're asking, Marcel. Honestly, there should probably be no normalization at this step. I would remove the "Normalization rule" from the code, but it affects too many existing systems. The patch I have here is the lightest change while also providing the desired functionality. I'm not sure what else a person could do to skip the normalization in a way that is backwards compatible. (In reply to David Cook from comment #19) > While the matchpoint_component_norms.norm_routine isn't currently used in > the code, I add some code that does use it for the sole purpose of detecting > a "none" or "raw" normalization (ie no normalization). > > Sorry, but I'm still not sure what you're asking, Marcel. Honestly, there > should probably be no normalization at this step. I would remove the > "Normalization rule" from the code, but it affects too many existing > systems. The patch I have here is the lightest change while also providing > the desired functionality. I'm not sure what else a person could do to skip > the normalization in a way that is backwards compatible. I understand your point hopefully :) If we start using this field: Since it was meant to add a Normalize class, I think you should create at least a rudimentary one now. Moving the _normalize code and add your none/raw normalizations. (In reply to Marcel de Rooy from comment #20) > I understand your point hopefully :) > If we start using this field: Since it was meant to add a Normalize class, I > think you should create at least a rudimentary one now. > Moving the _normalize code and add your none/raw normalizations. I'm not sure that I understand your point, unfortunately :(. I think using normalization during matching is a bad idea, as Zebra already normalizes the query before processing it. Here's the problem: Custom normalization: Strip hyphens Zebra normalization: Strip colons Data: "Mont-Royal: it's a mountain" Zebra would index it as "Mont-Royal it's a mountain". You'd retrieve that exact phrase with the query "Mont-Royal: it's a mountain", which would be transformed into "Mont-Royal it's a mountain" by Zebra. However, with the custom normalization, your query would be "MontRoyal: it's a mountain" before going to Zebra, and then it would be "MontRoyal it's a mountain" when processed by Zebra. If you're doing an exact search, your search will fail, even though that same data in the regular Koha search would work. We need to normalize indexing/retrieval consistently, and the way to do that is with Zebra alone. If there needs to be extra normalization, I think it should be done before the record even makes it to Koha... This bug is marked as signed off, but the main patch has no sign-off. Is the status incorrect or did someone miss adding a sign-off? It looks like bouzid.fergani@inlibro.com's sign-off got lost along the way. I am not sure how much the patch set was changed after the initial sign off - setting to Failed QA to get David's or Bouzid's attention. Patches still apply. To be honest, I don't know what I'm supposed to do about this patch anymore. I don't think Marcel liked my patch, but his suggestions weren't feasible, so... I don't know. I'm not inclined to work on it, since I'm not sure what I need to do to get it to an acceptable point. It's essentially a duplicate of bug 14238 anyway, so perhaps Kyle can write his own patch. I would be happy to add my sign off to that. Considering you have written a patch, and I have not, I'd like to see yours get it in ;) David, is this patch still in good condition? If so I'd be willing to test and sign-off. (In reply to David Cook from comment #24) > To be honest, I don't know what I'm supposed to do about this patch anymore. > > I don't think Marcel liked my patch, but his suggestions weren't feasible, > so... I don't know. I'm not inclined to work on it, since I'm not sure what > I need to do to get it to an acceptable point. > > It's essentially a duplicate of bug 14238 anyway, so perhaps Kyle can write > his own patch. I would be happy to add my sign off to that. (In reply to Kyle M Hall from comment #25) > Considering you have written a patch, and I have not, I'd like to see yours > get it in ;) > > David, is this patch still in good condition? If so I'd be willing to test > and sign-off. > Sorry for the delay. Didn't see your comment until now. I'll have to review the patch, but I'll get back to you this week. Kyle, I just reviewed the code, and it looks good to me. Thanks for taking the time to test it :). David, what do u think of bug 17318? Might obsolete this one too? Comment on attachment 50971 [details] [review] Bug 15541 - Prevent normalization during matching/import process Review of attachment 50971 [details] [review]: ----------------------------------------------------------------- ::: C4/Matcher.pm @@ +660,4 @@ > > my $searcher = Koha::SearchEngine::Search->new({index => $Koha::SearchEngine::BIBLIOS_INDEX}); > ( $error, $searchresults, $total_hits ) = > + $searcher->simple_search_compat( $query, 0, $max_matches, undef, "skip_normalize" ); 3 parameters turned to 5 with one set to undef? Wouldn't making this parameter a hashref mean cleaner code and easier adding/removing parameters in the future? As pointed out by tcohen on IRC, bug 17318 covers this and more. Marking this as a duplicate. *** This bug has been marked as a duplicate of bug 17318 *** Cheers, Mark. For some reason, I didn't get any of these emails... and just thought to check on the progress now. Definitely a duplicate. Tomas's patches should solved this. As for your suggestion, regarding changing it to a hashref, yeah it would mean cleaner code, but it would also add more changes. I suppose you could argue that it falls into the "refactor" as you go, but I was trying to do the lightest touch. Irrelevant in any case, since Tomas's patches obsolete these, but I'll keep that in mind for next time... While part of this patch is a duplicate of bug 17318, there are some parts which are not duplicates. I'm re-opening this bug and will upload an updated patch for the unique changes. Created attachment 57852 [details] [review] Bug 15541 - Prevent normalization during matching/import process This patch allows you to use the "qualifier,qualifier" syntax in the Record Matching Rules "Search Index" when using the QueryParser. While QueryParser doesn't support this syntax, it will now fallback correctly to non-QueryParser functionality. Without the patch, your search will just fail silently. This patch also adds a "skip_normalize" option to C4::Search::SimpleSearch(), and uses the option during C4::Matcher::get_matches. This prevents the s/:/=/g and s/=/:/g normalization. This normalization is heavy-handed, and while it is necessary sometimes to generate a valid CCL query or QueryParser query, C4::Matcher::get_matches() already produces a valid CCL query, so we don't need to do this normalization. Additionally, this normalization causes problems when you use a Zebra register which isn't normalized: namely the "u" register. Strings are stored "as is", so http://localhost/koha/resource/1 is stored as is during indexing. When you search, you need to pass the exact same thing through the query to get a match. Using http=//localhost/koha/resource/1 in your query will yield zero results. _TEST PLAN_ 0) Apply patch 1) Create a Record Matching Rule in Koha with the following details: Matching rule code: TEST Description: Test Match threshold: 100 Record type: Bibliographic Match point 1: Search index: id-other,st-urx Score: 100 Tag: 024 Subfields: a Normalization rule: None 2) Create a record in Koha with an indexable URI 2a) Default framework 2b) 024 $a http://koha-community.org/test $2 uri 2c) 040 $c test 2d) 245 $a This is a test record 2e) 942 $c Books 2f) Save (save again if cautioned about missing fields as these should autofill) 3) Do a full re-index of Zebra 4) Download your record from Koha as a .mrc file (ie isomarc, binary marc, etc) 5) Go to "Stage MARC records for import" 5a) Upload your .marc file. 5b) Change your "Record matching rule" to "Test" 5c) Click Stage for import 9) It should say "1 records with at least one match in catalog per matching rule "Test". NOTE: For completeness, you can go through this process on a clean master branch, and note that it will say '0 records with at least one match in catalog per matching rule "TEST"' Created attachment 59295 [details] [review] Bug 15541 - Prevent normalization during matching/import process This patch allows you to use the "qualifier,qualifier" syntax in the Record Matching Rules "Search Index" when using the QueryParser. While QueryParser doesn't support this syntax, it will now fallback correctly to non-QueryParser functionality. Without the patch, your search will just fail silently. This patch also adds a "skip_normalize" option to C4::Search::SimpleSearch(), and uses the option during C4::Matcher::get_matches. This prevents the s/:/=/g and s/=/:/g normalization. This normalization is heavy-handed, and while it is necessary sometimes to generate a valid CCL query or QueryParser query, C4::Matcher::get_matches() already produces a valid CCL query, so we don't need to do this normalization. Additionally, this normalization causes problems when you use a Zebra register which isn't normalized: namely the "u" register. Strings are stored "as is", so http://localhost/koha/resource/1 is stored as is during indexing. When you search, you need to pass the exact same thing through the query to get a match. Using http=//localhost/koha/resource/1 in your query will yield zero results. _TEST PLAN_ 0) Apply patch 1) Create a Record Matching Rule in Koha with the following details: Matching rule code: TEST Description: Test Match threshold: 100 Record type: Bibliographic Match point 1: Search index: id-other,st-urx Score: 100 Tag: 024 Subfields: a Normalization rule: None 2) Create a record in Koha with an indexable URI 2a) Default framework 2b) 024 $a http://koha-community.org/test $2 uri 2c) 040 $c test 2d) 245 $a This is a test record 2e) 942 $c Books 2f) Save (save again if cautioned about missing fields as these should autofill) 3) Do a full re-index of Zebra 4) Download your record from Koha as a .mrc file (ie isomarc, binary marc, etc) 5) Go to "Stage MARC records for import" 5a) Upload your .marc file. 5b) Change your "Record matching rule" to "Test" 5c) Click Stage for import 9) It should say "1 records with at least one match in catalog per matching rule "Test". NOTE: For completeness, you can go through this process on a clean master branch, and note that it will say '0 records with at least one match in catalog per matching rule "TEST"' Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz> Cheers Alex :) Created attachment 61216 [details] [review] Bug 15541 - Prevent normalization during matching/import process This patch allows you to use the "qualifier,qualifier" syntax in the Record Matching Rules "Search Index" when using the QueryParser. While QueryParser doesn't support this syntax, it will now fallback correctly to non-QueryParser functionality. Without the patch, your search will just fail silently. This patch also adds a "skip_normalize" option to C4::Search::SimpleSearch(), and uses the option during C4::Matcher::get_matches. This prevents the s/:/=/g and s/=/:/g normalization. This normalization is heavy-handed, and while it is necessary sometimes to generate a valid CCL query or QueryParser query, C4::Matcher::get_matches() already produces a valid CCL query, so we don't need to do this normalization. Additionally, this normalization causes problems when you use a Zebra register which isn't normalized: namely the "u" register. Strings are stored "as is", so http://localhost/koha/resource/1 is stored as is during indexing. When you search, you need to pass the exact same thing through the query to get a match. Using http=//localhost/koha/resource/1 in your query will yield zero results. _TEST PLAN_ 0) Apply patch 1) Create a Record Matching Rule in Koha with the following details: Matching rule code: TEST Description: Test Match threshold: 100 Record type: Bibliographic Match point 1: Search index: id-other,st-urx Score: 100 Tag: 024 Subfields: a Normalization rule: None 2) Create a record in Koha with an indexable URI 2a) Default framework 2b) 024 $a http://koha-community.org/test $2 uri 2c) 040 $c test 2d) 245 $a This is a test record 2e) 942 $c Books 2f) Save (save again if cautioned about missing fields as these should autofill) 3) Do a full re-index of Zebra 4) Download your record from Koha as a .mrc file (ie isomarc, binary marc, etc) 5) Go to "Stage MARC records for import" 5a) Upload your .marc file. 5b) Change your "Record matching rule" to "Test" 5c) Click Stage for import 9) It should say "1 records with at least one match in catalog per matching rule "Test". NOTE: For completeness, you can go through this process on a clean master branch, and note that it will say '0 records with at least one match in catalog per matching rule "TEST"' Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> QA Comment: Looks good to me. Finally :) %options: Putting a hash in the parameter list may be tricky of course. We always like to have unit tests. But unfortunately we have no tests for get_matches.. Passed QA Pushed to master for 17.05, thanks David! This won't get ported back to 16.11.x as it is an enhancement. |