Skip to content

Commit

Permalink
Aggregations/HL Rest client fix: missing scores (#32774)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
markharwood authored Aug 14, 2018
1 parent 00b006f commit e5ab09f
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@
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;
Expand Down Expand Up @@ -267,6 +270,33 @@ public void testSearchWithTermsAgg() throws IOException {
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 {
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ static <B extends ParsedBucket> B parseSignificantTermsBucketXContent(final XCon
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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,10 @@ protected InternalSignificantTerms createTestInstance(String name,
Set<Long> 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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,10 @@ protected InternalSignificantTerms createTestInstance(String name,
Set<BytesRef> 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);
Expand Down

0 comments on commit e5ab09f

Please sign in to comment.