In Elasticsearch 7 hits.total is now an object (https://www.elastic.co/guide/en/elasticsearch/reference/7.0/breaking-changes-7.0.html#hits-total-now-object-search-response). The total is now in $result->{hits}->{total}->{value} instead of $result->{hits}->{total}. There is also $result->{hits}->{total}->{relation} which specifies if the value is exact or if it's a lower bound. So it's possible to say "Your search returned > 10000 results." You can always get an exact total by adding a track_total_hits parameter set to true when using the Elasticsearch search method (https://www.elastic.co/guide/en/elasticsearch/reference/7.0/breaking-changes-7.0.html#track-total-hits-10000-default).
Created attachment 130291 [details] [review] Bug 28610: Elasticsearch 7 - hits.total is now an object In Elasticsearch 7 hits.total is now an object which is not always an exact value. You can always get an exact total by adding a track_total_hits parameter set to true when using the Elasticsearch search method To test: 1) Run prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t 2) If you observe an error about types, apply patch for bug 25669 3) Run prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t 4) Observe that tests with count fail 5) Apply patch 6) Observe that tests with count pass 7) Sign off Sponsored-by: Lund University Library
Created attachment 130343 [details] [review] Bug 28610: Elasticsearch 7 - hits.total is now an object In Elasticsearch 7 hits.total is now an object which is not always an exact value. You can always get an exact total by adding a track_total_hits parameter set to true when using the Elasticsearch search method To test: 1) Run prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t 2) If you observe an error about types, apply patch for bug 25669 3) Run prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t 4) Observe that tests with count fail 5) Apply patch 6) Observe that tests with count pass 7) Sign off Sponsored-by: Lund University Library
Created attachment 130844 [details] [review] Bug 28610: Elasticsearch 7 - hits.total is now an object In Elasticsearch 7 hits.total is now an object which is not always an exact value. You can always get an exact total by adding a track_total_hits parameter set to true when using the Elasticsearch search method To test: 1) Run prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t 2) If you observe an error about types, apply patch for bug 25669 3) Run prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t 4) Observe that tests with count fail 5) Apply patch 6) Observe that tests with count pass 7) Sign off Sponsored-by: Lund University Library Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
It work on ES 6 and ES 7
+ if (version->parse($self->{es_version}) >= version->parse('7.0.0')) { It's going to be mess very quickly if we are starting to add conditions on the version. Cannot we avoid that?
I've removed the condition, and the tests are passing under ES6.
(In reply to Jonathan Druart from comment #5) > + if (version->parse($self->{es_version}) >= version->parse('7.0.0')) { > > It's going to be mess very quickly if we are starting to add conditions on > the version. Cannot we avoid that? +1 This may impact a lot search performance. Or we should add a cache on parsed ES version.
Created attachment 131008 [details] [review] Bug 28610: Elasticsearch 7 - hits.total is now an object In Elasticsearch 7 hits.total is now an object which is not always an exact value. You can always get an exact total by adding a track_total_hits parameter set to true when using the Elasticsearch search method To test: 1) Run prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t 2) If you observe an error about types, apply patch for bug 25669 3) Run prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t 4) Observe that tests with count fail 5) Apply patch 6) Observe that tests with count pass 7) Sign off Sponsored-by: Lund University Library
(In reply to Jonathan Druart from comment #6) > I've removed the condition, and the tests are passing under ES6. Great, ES6 supports track_total_hits. That makes it much easier.
(In reply to Kevin Carnes from comment #9) > (In reply to Jonathan Druart from comment #6) > > I've removed the condition, and the tests are passing under ES6. > > Great, ES6 supports track_total_hits. That makes it much easier. Not sure someone told you yet, but great to see these updates for Elasticsearch 7 going on - thx Kevin!
Shouldn't 'track_total_hits' option be set in the 'count' subroutine too ?
Created attachment 131436 [details] [review] Bug 28610: Elasticsearch 7 - hits.total is now an object In Elasticsearch 7 hits.total is now an object which is not always an exact value. You can always get an exact total by adding a track_total_hits parameter set to true when using the Elasticsearch search method To test: 1) Run prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t 2) If you observe an error about types, apply patch for bug 25669 3) Run prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t 4) Observe that tests with count fail 5) Apply patch 6) Observe that tests with count pass 7) Sign off Sponsored-by: Lund University Library
(In reply to Julian Maurice from comment #11) > Shouldn't 'track_total_hits' option be set in the 'count' subroutine too ? Yes, thank you for catching that.
Tested with bug 25669 applied. It works great with ES 6 and ES 7. It looks like the version of Search::Elasticsearch (6.x or 7.x) doesn't matter too much. I tried both and everything went well.
Created attachment 131542 [details] [review] Bug 28610: Elasticsearch 7 - hits.total is now an object In Elasticsearch 7 hits.total is now an object which is not always an exact value. You can always get an exact total by adding a track_total_hits parameter set to true when using the Elasticsearch search method To test: 1) Run prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t 2) If you observe an error about types, apply patch for bug 25669 3) Run prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t 4) Observe that tests with count fail 5) Apply patch 6) Observe that tests with count pass 7) Sign off Sponsored-by: Lund University Library Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
-requires 'Search::Elasticsearch', '5.01'; +requires 'Search::Elasticsearch', '6.00'; Why this change in this patch? We certainly should upgrade our minimum version, but it should be done on its own bug report.
(In reply to Jonathan Druart from comment #16) > -requires 'Search::Elasticsearch', '5.01'; > +requires 'Search::Elasticsearch', '6.00'; > > Why this change in this patch? We certainly should upgrade our minimum > version, but it should be done on its own bug report. 6.00 is the minimum version that has track_total_hits in its API for search (look for track_total_hits in https://metacpan.org/release/DRTECH/Search-Elasticsearch-6.00/source/lib/Search/Elasticsearch/Client/6_0/Role/API.pm and https://metacpan.org/release/DRTECH/Search-Elasticsearch-5.02/source/lib/Search/Elasticsearch/Client/5_0/Role/API.pm)
As Release Manager, I'm OK to change min version here. As long as we tell it in release notes. It is already ES 6 in https://wiki.koha-community.org/wiki/System_requirements_and_recommendations
It has already been dropped officially with Bug 27252 But actually with syspref ElasticsearchCrossFields off, ES 5 still worked [1] so this time will be the real end of support of ES 5. Hopefully not much instances use it as it's EOL since 2019-03-11 https://www.elastic.co/support/eol I gave a go at release notes. Tell me if info is missing or if it's not the right writing style. [1] https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27770#c9
Release notes looks great. Thanks a lot.
Just tested this with Elasticsearch 8.1.1 (with bug 25669) and it seems to work well :)
Created attachment 132389 [details] [review] Bug 28610: Elasticsearch 7 - hits.total is now an object In Elasticsearch 7 hits.total is now an object which is not always an exact value. You can always get an exact total by adding a track_total_hits parameter set to true when using the Elasticsearch search method To test: 1) Run prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t 2) If you observe an error about types, apply patch for bug 25669 3) Run prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t 4) Observe that tests with count fail 5) Apply patch 6) Observe that tests with count pass 7) Sign off Sponsored-by: Lund University Library Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Pushed to master for 22.05, thanks to everybody involved [U+1F984]
Pushed to 21.11.x for 21.11.05
Backported: Pushed to 21.05.x branch for 21.05.17 Nothing to document (not yet full support for ES 7), marking resolved.
(In reply to Victor Grousset/tuxayo from comment #25) > Backported: Pushed to 21.05.x branch for 21.05.17 > Nothing to document (not yet full support for ES 7), marking resolved. hi Victor can you push this for ES7 support, (i think it was missed)
> (i think it was missed) Indeed! :o Thanks for catching that. Pushed now.