Skip to content

Commit

Permalink
Fix _score access in double scripted terms agg (#82331)
Browse files Browse the repository at this point in the history 
When we're aggregation on a script that contains `score` with the value
type hint `double` we were always using a score of `0.0`. This passes
the score into the script properly.

This also updates `terms` agg tests in accordance with #82273. I found
this bug while updating tests for #82273 but wanted to split out the bug
fix. But I wanted to include all of the updates to these tests for
issue #82273 because they add extra type checks for similar issues.
  • Loading branch information
@nik9000
nik9000 committed Jan 31, 2022
1 parent a03cd92 commit 6383d22
Show file tree
Hide file tree
Showing 8 changed files with 375 additions and 343 deletions.
27 changes: 17 additions & 10 deletions ...internalClusterTest/java/org/elasticsearch/search/aggregations/bucket/BooleanTermsIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,11 @@
import org.elasticsearch.action.index.IndexRequestBuilder;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.search.aggregations.Aggregator.SubAggCollectionMode;
import org.elasticsearch.search.aggregations.bucket.terms.Terms;
import org.elasticsearch.search.aggregations.bucket.terms.LongTerms;
import org.elasticsearch.search.aggregations.bucket.terms.TermsAggregationBuilder;
import org.elasticsearch.search.aggregations.bucket.terms.UnmappedTerms;
import org.elasticsearch.test.ESIntegTestCase;

import static org.elasticsearch.search.aggregations.AggregationBuilders.terms;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse;
import static org.elasticsearch.xcontent.XContentFactory.jsonBuilder;
import static org.hamcrest.Matchers.equalTo;
Expand Down Expand Up @@ -72,18 +73,20 @@ public void setupSuiteScopeCluster() throws Exception {

public void testSingleValueField() throws Exception {
SearchResponse response = client().prepareSearch("idx")
.addAggregation(terms("terms").field(SINGLE_VALUED_FIELD_NAME).collectMode(randomFrom(SubAggCollectionMode.values())))
.addAggregation(
new TermsAggregationBuilder("terms").field(SINGLE_VALUED_FIELD_NAME).collectMode(randomFrom(SubAggCollectionMode.values()))
)
.get();

assertSearchResponse(response);

Terms terms = response.getAggregations().get("terms");
LongTerms terms = response.getAggregations().get("terms");
assertThat(terms, notNullValue());
assertThat(terms.getName(), equalTo("terms"));
final int bucketCount = numSingleFalses > 0 && numSingleTrues > 0 ? 2 : numSingleFalses + numSingleTrues > 0 ? 1 : 0;
assertThat(terms.getBuckets().size(), equalTo(bucketCount));

Terms.Bucket bucket = terms.getBucketByKey("false");
LongTerms.Bucket bucket = terms.getBucketByKey("false");
if (numSingleFalses == 0) {
assertNull(bucket);
} else {
Expand All @@ -104,18 +107,20 @@ public void testSingleValueField() throws Exception {

public void testMultiValueField() throws Exception {
SearchResponse response = client().prepareSearch("idx")
.addAggregation(terms("terms").field(MULTI_VALUED_FIELD_NAME).collectMode(randomFrom(SubAggCollectionMode.values())))
.addAggregation(
new TermsAggregationBuilder("terms").field(MULTI_VALUED_FIELD_NAME).collectMode(randomFrom(SubAggCollectionMode.values()))
)
.get();

assertSearchResponse(response);

Terms terms = response.getAggregations().get("terms");
LongTerms terms = response.getAggregations().get("terms");
assertThat(terms, notNullValue());
assertThat(terms.getName(), equalTo("terms"));
final int bucketCount = numMultiFalses > 0 && numMultiTrues > 0 ? 2 : numMultiFalses + numMultiTrues > 0 ? 1 : 0;
assertThat(terms.getBuckets().size(), equalTo(bucketCount));

Terms.Bucket bucket = terms.getBucketByKey("false");
LongTerms.Bucket bucket = terms.getBucketByKey("false");
if (numMultiFalses == 0) {
assertNull(bucket);
} else {
Expand All @@ -137,13 +142,15 @@ public void testMultiValueField() throws Exception {
public void testUnmapped() throws Exception {
SearchResponse response = client().prepareSearch("idx_unmapped")
.addAggregation(
terms("terms").field(SINGLE_VALUED_FIELD_NAME).size(between(1, 5)).collectMode(randomFrom(SubAggCollectionMode.values()))
new TermsAggregationBuilder("terms").field(SINGLE_VALUED_FIELD_NAME)
.size(between(1, 5))
.collectMode(randomFrom(SubAggCollectionMode.values()))
)
.get();

assertSearchResponse(response);

Terms terms = response.getAggregations().get("terms");
UnmappedTerms terms = response.getAggregations().get("terms");
assertThat(terms, notNullValue());
assertThat(terms.getName(), equalTo("terms"));
assertThat(terms.getBuckets().size(), equalTo(0));
Expand Down
198 changes: 106 additions & 92 deletions .../internalClusterTest/java/org/elasticsearch/search/aggregations/bucket/DoubleTermsIT.java
View file

Large diffs are not rendered by default.

28 changes: 14 additions & 14 deletions .../src/internalClusterTest/java/org/elasticsearch/search/aggregations/bucket/IpTermsIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.script.Script;
import org.elasticsearch.script.ScriptType;
import org.elasticsearch.search.aggregations.AggregationBuilders;
import org.elasticsearch.search.aggregations.AggregationTestScriptsPlugin;
import org.elasticsearch.search.aggregations.bucket.terms.Terms;
import org.elasticsearch.search.aggregations.bucket.terms.StringTerms;
import org.elasticsearch.search.aggregations.bucket.terms.TermsAggregationBuilder;

import java.util.Collection;
import java.util.Collections;
Expand Down Expand Up @@ -62,18 +62,18 @@ public void testScriptValue() throws Exception {

Script script = new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "doc['ip'].value", Collections.emptyMap());
SearchResponse response = client().prepareSearch("index")
.addAggregation(AggregationBuilders.terms("my_terms").script(script).executionHint(randomExecutionHint()))
.addAggregation(new TermsAggregationBuilder("my_terms").script(script).executionHint(randomExecutionHint()))
.get();
assertSearchResponse(response);
Terms terms = response.getAggregations().get("my_terms");
StringTerms terms = response.getAggregations().get("my_terms");
assertEquals(2, terms.getBuckets().size());

Terms.Bucket bucket1 = terms.getBuckets().get(0);
StringTerms.Bucket bucket1 = terms.getBuckets().get(0);
assertEquals(2, bucket1.getDocCount());
assertEquals("192.168.1.7", bucket1.getKey());
assertEquals("192.168.1.7", bucket1.getKeyAsString());

Terms.Bucket bucket2 = terms.getBuckets().get(1);
StringTerms.Bucket bucket2 = terms.getBuckets().get(1);
assertEquals(1, bucket2.getDocCount());
assertEquals("2001:db8::2:1", bucket2.getKey());
assertEquals("2001:db8::2:1", bucket2.getKeyAsString());
Expand All @@ -90,18 +90,18 @@ public void testScriptValues() throws Exception {

Script script = new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "doc['ip']", Collections.emptyMap());
SearchResponse response = client().prepareSearch("index")
.addAggregation(AggregationBuilders.terms("my_terms").script(script).executionHint(randomExecutionHint()))
.addAggregation(new TermsAggregationBuilder("my_terms").script(script).executionHint(randomExecutionHint()))
.get();
assertSearchResponse(response);
Terms terms = response.getAggregations().get("my_terms");
StringTerms terms = response.getAggregations().get("my_terms");
assertEquals(2, terms.getBuckets().size());

Terms.Bucket bucket1 = terms.getBuckets().get(0);
StringTerms.Bucket bucket1 = terms.getBuckets().get(0);
assertEquals(2, bucket1.getDocCount());
assertEquals("192.168.1.7", bucket1.getKey());
assertEquals("192.168.1.7", bucket1.getKeyAsString());

Terms.Bucket bucket2 = terms.getBuckets().get(1);
StringTerms.Bucket bucket2 = terms.getBuckets().get(1);
assertEquals(1, bucket2.getDocCount());
assertEquals("2001:db8::2:1", bucket2.getKey());
assertEquals("2001:db8::2:1", bucket2.getKeyAsString());
Expand All @@ -117,19 +117,19 @@ public void testMissingValue() throws Exception {
client().prepareIndex("index").setId("4").setSource("not_ip", "something")
);
SearchResponse response = client().prepareSearch("index")
.addAggregation(AggregationBuilders.terms("my_terms").field("ip").missing("127.0.0.1").executionHint(randomExecutionHint()))
.addAggregation(new TermsAggregationBuilder("my_terms").field("ip").missing("127.0.0.1").executionHint(randomExecutionHint()))
.get();

assertSearchResponse(response);
Terms terms = response.getAggregations().get("my_terms");
StringTerms terms = response.getAggregations().get("my_terms");
assertEquals(2, terms.getBuckets().size());

Terms.Bucket bucket1 = terms.getBuckets().get(0);
StringTerms.Bucket bucket1 = terms.getBuckets().get(0);
assertEquals(2, bucket1.getDocCount());
assertEquals("127.0.0.1", bucket1.getKey());
assertEquals("127.0.0.1", bucket1.getKeyAsString());

Terms.Bucket bucket2 = terms.getBuckets().get(1);
StringTerms.Bucket bucket2 = terms.getBuckets().get(1);
assertEquals(2, bucket2.getDocCount());
assertEquals("192.168.1.7", bucket2.getKey());
assertEquals("192.168.1.7", bucket2.getKeyAsString());
Expand Down

0 comments on commit 6383d22

Please sign in to comment.