In Koha/SearchEngine/Elasticsearch/QueryBuilder.pm : There is a map of convertion from CCL : our %index_field_convert = ( 'kw' => '', 'ab' => 'abstract', 'au' => 'author', ... You see that kw is replaced by empty. See that in _convert_index_strings() : my ($conv) = $self->_convert_index_fields($field); unless ( defined($conv) ) { push @res, $s; next; } push @res, ($conv->{field} ? $conv->{field} . ':' : '') . $self->_modify_string_by_type( %$conv, operand => $term ); If Field is empty the character ":" is not added. This behavior is missing from _convert_index_strings_freeform() : $search =~ s/($field_name_pattern)($multi_field_pattern):/(exists $index_field_convert{$1} ? $index_field_convert{$1} : $1)."$2:"/oge; Query "kw:term" will be converted to ":term" :(
Created attachment 116241 [details] [review] Bug 27597: Remove leading colon in ES query If we are searching on kw there is a leading colon at the beginning of the generated query: kw:foo becomes :foo Note that it only happens when there is no other terms before. Test plan: 0. Don't apply the patch 1. Search for kw:foo 2. Notice the error Error: Unable to perform your search. Please try again. and the logs say Failed to parse query [(:foo*)] 3. Apply the patch 4. Repeat the search and notice that you know get: "12 result(s) found for 'kw:foo'."
Created attachment 116242 [details] [review] Bug 27597: Remove extra spaces in the generated query Look at the tests, sometime we have extra spaces. With this patch they still pass and the expected queries are better. Feel free to drop this patch if you have any concerns about it.
Super, But we also need to fix in _convert_index_strings_freeform() rigth ?
And BTW is it in theory correct to replace "kw:" with "*:" ?
(In reply to Fridolin Somers from comment #3) > Super, > > But we also need to fix in _convert_index_strings_freeform() rigth ? I fixed it where we deal with colons, so it seems to be the right place. _convert_index_strings_freeform is only called from _clean_search_term anyway. (In reply to Fridolin Somers from comment #4) > And BTW is it in theory correct to replace "kw:" with "*:" ? If it's what we want we should map is in %index_field_convert then.
It also breaks itemBarcodeFallbackSearch. [2021/02/17 08:36:15] [WARN] [Request] ** [http://es:9200]-[400] [query_shard_exception] Failed to parse query [(:street*)], with: {"index_uuid":"YeYOreb1Rl2j-l52RVXJ4A","index":"koha_kohadev_biblios"}, called f rom sub Search::Elasticsearch::Role::Client::Direct::__ANON__ at /kohadevbox/koha/Koha/SearchEngine/Elasticsearch/Search.pm line 96. With vars: {'status_code' => 400,'body' => {'status' => 400,'error' => {'faile d_shards' => [{'index' => 'koha_kohadev_biblios','reason' => {'type' => 'query_shard_exception','index_uuid' => 'YeYOreb1Rl2j-l52RVXJ4A','caused_by' => {'reason' => 'Cannot parse \'(:street*)\': Encountered " ":" ": "" at line 1, column 1.
# Remove unquoted colons that have whitespace on either side of them - $term =~ s/(:+)(\s+)$lookahead/$2/g; + $term =~ s/(:+)(\s+)$lookahead//g; $term =~ s/(\s+)(:+)$lookahead/$1/g; Could you explain this change please ?
(In reply to Fridolin Somers from comment #7) > # Remove unquoted colons that have whitespace on either side of them > - $term =~ s/(:+)(\s+)$lookahead/$2/g; > + $term =~ s/(:+)(\s+)$lookahead//g; > $term =~ s/(\s+)(:+)$lookahead/$1/g; > > Could you explain this change please ? Look at the unit tests, it removes extra spaces.
Created attachment 117856 [details] [review] Bug 27597: Remove leading colon in ES query If we are searching on kw there is a leading colon at the beginning of the generated query: kw:foo becomes :foo Note that it only happens when there is no other terms before. Test plan: 0. Don't apply the patch 1. Search for kw:foo 2. Notice the error Error: Unable to perform your search. Please try again. and the logs say Failed to parse query [(:foo*)] 3. Apply the patch 4. Repeat the search and notice that you know get: "12 result(s) found for 'kw:foo'." Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Created attachment 117857 [details] [review] Bug 27597: Remove extra spaces in the generated query Look at the tests, sometime we have extra spaces. With this patch they still pass and the expected queries are better. Feel free to drop this patch if you have any concerns about it. Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
All is good ;)
I am stuck on this one as I can't get Elasticsearch to work :( Error: Unable to perform your search. Please try again. Applied the patch, reset mappings, reindexed... status of Elasticsearch is green.
Created attachment 118190 [details] [review] Bug 27597: Remove leading colon in ES query If we are searching on kw there is a leading colon at the beginning of the generated query: kw:foo becomes :foo Note that it only happens when there is no other terms before. Test plan: 0. Don't apply the patch 1. Search for kw:foo 2. Notice the error Error: Unable to perform your search. Please try again. and the logs say Failed to parse query [(:foo*)] 3. Apply the patch 4. Repeat the search and notice that you know get: "12 result(s) found for 'kw:foo'." Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
(In reply to Fridolin Somers from comment #10) > Created attachment 117857 [details] [review] [review] > Bug 27597: Remove extra spaces in the generated query > > Look at the tests, sometime we have extra spaces. > With this patch they still pass and the expected queries are better. > Feel free to drop this patch if you have any concerns about it. > > Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com> I obsoleted this, it causes a recreation of issues solved in bug 24567, tested with the comment from bug 24372: Using an example record in the testing DB, try searching for: Pictura murală din nordul Moldovei: modificari estetice si restarare = Mural painting in the north of Moldavia: aesthetic modification and restoration In ES the search fails. If you look at the query we end up with terms like: Moldoveimodificari AND Moldaviaaesthetic
Pushed to master for 21.05, thanks to everybody involved!
Pushed to 20.11.x for 20.11.04
Pushed to 20.05.x for 20.05.10
Can't backport to 19.11.x: can't solve a conflict. If there is an interest in having this backported, please submit a patch for 19.11. <<<<<<< HEAD $term =~ s/(\:[:\s]+|[:\s]+:)$lookahead//g; ||||||| parent of 622a68f695 (Bug 27597: Remove leading colon in ES query) $term =~ s/(:+)(\s+)$lookahead/$2/g; $term =~ s/(\s+)(:+)$lookahead/$1/g; ======= $term =~ s/(:+)(\s+)$lookahead/$2/g; $term =~ s/(\s+)(:+)$lookahead/$1/g; $term =~ s/^://; >>>>>>> 622a68f695 (Bug 27597: Remove leading colon in ES query)