From e5ab09f7087f0aabc399bf288adb9e610f016473 Mon Sep 17 00:00:00 2001 From: markharwood Date: Tue, 14 Aug 2018 11:14:47 +0100 Subject: [PATCH] Aggregations/HL Rest client fix: missing scores (#32774) Significance score doubles were being parsed as long. Existing tests did not catch this because SignificantLongTermsTests and SignificantStringTermsTests did not set the score. Fixed these and also added integration test. Thanks for the report/fix, Blakko Closes #32770 --- .../org/elasticsearch/client/SearchIT.java | 30 +++++++++++++++++++ .../significant/ParsedSignificantTerms.java | 2 +- .../SignificantLongTermsTests.java | 5 +++- .../SignificantStringTermsTests.java | 5 +++- 4 files changed, 39 insertions(+), 3 deletions(-) diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/SearchIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/SearchIT.java index 9c9c5425f00..3151e9badb6 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/SearchIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/SearchIT.java @@ -59,6 +59,9 @@ import org.elasticsearch.search.SearchHit; import org.elasticsearch.search.aggregations.BucketOrder; import org.elasticsearch.search.aggregations.bucket.range.Range; import org.elasticsearch.search.aggregations.bucket.range.RangeAggregationBuilder; +import org.elasticsearch.search.aggregations.bucket.significant.SignificantTerms; +import org.elasticsearch.search.aggregations.bucket.significant.SignificantTermsAggregationBuilder; +import org.elasticsearch.search.aggregations.bucket.significant.heuristics.PercentageScore; import org.elasticsearch.search.aggregations.bucket.terms.Terms; import org.elasticsearch.search.aggregations.bucket.terms.TermsAggregationBuilder; import org.elasticsearch.search.aggregations.matrix.stats.MatrixStats; @@ -267,6 +270,33 @@ public class SearchIT extends ESRestHighLevelClientTestCase { assertEquals(2, type2.getDocCount()); assertEquals(0, type2.getAggregations().asList().size()); } + + public void testSearchWithSignificantTermsAgg() throws IOException { + SearchRequest searchRequest = new SearchRequest(); + SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); + searchSourceBuilder.query(new MatchQueryBuilder("num","50")); + searchSourceBuilder.aggregation(new SignificantTermsAggregationBuilder("agg1", ValueType.STRING) + .field("type.keyword") + .minDocCount(1) + .significanceHeuristic(new PercentageScore())); + searchSourceBuilder.size(0); + searchRequest.source(searchSourceBuilder); + SearchResponse searchResponse = execute(searchRequest, highLevelClient()::search, highLevelClient()::searchAsync); + assertSearchHeader(searchResponse); + assertNull(searchResponse.getSuggest()); + assertEquals(Collections.emptyMap(), searchResponse.getProfileResults()); + assertEquals(0, searchResponse.getHits().getHits().length); + assertEquals(0f, searchResponse.getHits().getMaxScore(), 0f); + SignificantTerms significantTermsAgg = searchResponse.getAggregations().get("agg1"); + assertEquals("agg1", significantTermsAgg.getName()); + assertEquals(1, significantTermsAgg.getBuckets().size()); + SignificantTerms.Bucket type1 = significantTermsAgg.getBucketByKey("type1"); + assertEquals(1, type1.getDocCount()); + assertEquals(1, type1.getSubsetDf()); + assertEquals(1, type1.getSubsetSize()); + assertEquals(3, type1.getSupersetDf()); + assertEquals(1d/3d, type1.getSignificanceScore(), 0d); + } public void testSearchWithRangeAgg() throws IOException { { diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/ParsedSignificantTerms.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/ParsedSignificantTerms.java index 1b4739c184d..26c4ec420d0 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/ParsedSignificantTerms.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/ParsedSignificantTerms.java @@ -175,7 +175,7 @@ public abstract class ParsedSignificantTerms extends ParsedMultiBucketAggregatio bucket.subsetDf = value; bucket.setDocCount(value); } else if (InternalSignificantTerms.SCORE.equals(currentFieldName)) { - bucket.score = parser.longValue(); + bucket.score = parser.doubleValue(); } else if (InternalSignificantTerms.BG_COUNT.equals(currentFieldName)) { bucket.supersetDf = parser.longValue(); } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantLongTermsTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantLongTermsTests.java index 985d82d4e1b..755cb6e8529 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantLongTermsTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantLongTermsTests.java @@ -57,7 +57,10 @@ public class SignificantLongTermsTests extends InternalSignificantTermsTestCase Set terms = new HashSet<>(); for (int i = 0; i < numBuckets; ++i) { long term = randomValueOtherThanMany(l -> terms.add(l) == false, random()::nextLong); - buckets.add(new SignificantLongTerms.Bucket(subsetDfs[i], subsetSize, supersetDfs[i], supersetSize, term, aggs, format)); + SignificantLongTerms.Bucket bucket = new SignificantLongTerms.Bucket(subsetDfs[i], subsetSize, + supersetDfs[i], supersetSize, term, aggs, format); + bucket.updateScore(significanceHeuristic); + buckets.add(bucket); } return new SignificantLongTerms(name, requiredSize, 1L, pipelineAggregators, metaData, format, subsetSize, supersetSize, significanceHeuristic, buckets); diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantStringTermsTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantStringTermsTests.java index 5dafc1e8461..2255373fd34 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantStringTermsTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantStringTermsTests.java @@ -50,7 +50,10 @@ public class SignificantStringTermsTests extends InternalSignificantTermsTestCas Set terms = new HashSet<>(); for (int i = 0; i < numBuckets; ++i) { BytesRef term = randomValueOtherThanMany(b -> terms.add(b) == false, () -> new BytesRef(randomAlphaOfLength(10))); - buckets.add(new SignificantStringTerms.Bucket(term, subsetDfs[i], subsetSize, supersetDfs[i], supersetSize, aggs, format)); + SignificantStringTerms.Bucket bucket = new SignificantStringTerms.Bucket(term, subsetDfs[i], subsetSize, + supersetDfs[i], supersetSize, aggs, format); + bucket.updateScore(significanceHeuristic); + buckets.add(bucket); } return new SignificantStringTerms(name, requiredSize, 1L, pipelineAggregators, metaData, format, subsetSize, supersetSize, significanceHeuristic, buckets);