From 431a7ef99e1220cfa30fb3eb38d8ba9e448ce3ae Mon Sep 17 00:00:00 2001 From: Andy Bristol Date: Fri, 13 Mar 2020 18:04:09 -0700 Subject: [PATCH 1/3] add tests to SumAggregatorTests This adds tests for supported ValuesSourceTypes, unmapped fields, scripting, and the missing param. The tests for unmapped fields and scripting are migrated from the SumIT integration test --- .../metrics/SumAggregatorTests.java | 262 ++++++++++++++++-- .../search/aggregations/metrics/SumIT.java | 168 ++--------- 2 files changed, 262 insertions(+), 168 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/SumAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/SumAggregatorTests.java index ff76aa4d0edef..dbaa20836f2e2 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/SumAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/SumAggregatorTests.java @@ -25,34 +25,62 @@ import org.apache.lucene.document.StringField; import org.apache.lucene.index.DirectoryReader; import org.apache.lucene.index.IndexReader; +import org.apache.lucene.index.IndexableField; +import org.apache.lucene.index.MultiReader; import org.apache.lucene.index.RandomIndexWriter; import org.apache.lucene.index.Term; import org.apache.lucene.search.DocValuesFieldExistsQuery; import org.apache.lucene.search.IndexSearcher; -import org.apache.lucene.search.MatchAllDocsQuery; import org.apache.lucene.search.Query; import org.apache.lucene.search.TermQuery; import org.apache.lucene.store.Directory; import org.apache.lucene.util.BytesRef; import org.apache.lucene.util.NumericUtils; import org.elasticsearch.common.CheckedConsumer; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.index.mapper.NumberFieldMapper; +import org.elasticsearch.index.mapper.NumberFieldMapper.NumberType; +import org.elasticsearch.script.MockScriptEngine; +import org.elasticsearch.script.Script; +import org.elasticsearch.script.ScriptEngine; +import org.elasticsearch.script.ScriptModule; +import org.elasticsearch.script.ScriptService; +import org.elasticsearch.script.ScriptType; +import org.elasticsearch.search.aggregations.AggregationBuilder; import org.elasticsearch.search.aggregations.AggregatorTestCase; import org.elasticsearch.search.aggregations.support.AggregationInspectionHelper; +import org.elasticsearch.search.aggregations.support.CoreValuesSourceType; +import org.elasticsearch.search.aggregations.support.ValuesSourceType; +import org.elasticsearch.search.lookup.LeafDocLookup; import java.io.IOException; +import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; import java.util.function.Consumer; +import java.util.function.Function; +import static java.util.Collections.emptyMap; import static java.util.Collections.singleton; +import static java.util.Collections.singletonList; +import static java.util.Collections.singletonMap; +import static java.util.stream.Collectors.toList; +import static org.elasticsearch.common.lucene.search.Queries.newMatchAllQuery; +import static org.elasticsearch.search.aggregations.AggregationBuilders.sum; public class SumAggregatorTests extends AggregatorTestCase { private static final String FIELD_NAME = "field"; + private static final String VALUE_SCRIPT_NAME = "value_script"; + private static final String FIELD_SCRIPT_NAME = "field_script"; public void testNoDocs() throws IOException { - testCase(new MatchAllDocsQuery(), iw -> { + testCase(newMatchAllQuery(), iw -> { // Intentionally not writing any docs }, count -> { assertEquals(0L, count.getValue(), 0d); @@ -61,7 +89,7 @@ public void testNoDocs() throws IOException { } public void testNoMatchingField() throws IOException { - testCase(new MatchAllDocsQuery(), iw -> { + testCase(newMatchAllQuery(), iw -> { iw.addDocument(singleton(new NumericDocValuesField("wrong_number", 7))); iw.addDocument(singleton(new NumericDocValuesField("wrong_number", 1))); }, count -> { @@ -71,7 +99,7 @@ public void testNoMatchingField() throws IOException { } public void testNumericDocValues() throws IOException { - testCase(new MatchAllDocsQuery(), iw -> { + testCase(newMatchAllQuery(), iw -> { iw.addDocument(singleton(new NumericDocValuesField(FIELD_NAME, 1))); iw.addDocument(singleton(new NumericDocValuesField(FIELD_NAME, 2))); iw.addDocument(singleton(new NumericDocValuesField(FIELD_NAME, 1))); @@ -122,7 +150,7 @@ public void testQueryFiltering() throws IOException { public void testStringField() throws IOException { IllegalStateException e = expectThrows(IllegalStateException.class, () -> { - testCase(new MatchAllDocsQuery(), iw -> { + testCase(newMatchAllQuery(), iw -> { iw.addDocument(singleton(new SortedDocValuesField(FIELD_NAME, new BytesRef("1")))); }, count -> { assertEquals(0L, count.getValue(), 0d); @@ -165,27 +193,193 @@ public void testSummationAccuracy() throws IOException { } private void verifySummationOfDoubles(double[] values, double expected, double delta) throws IOException { - testCase(new MatchAllDocsQuery(), + testCase(newMatchAllQuery(), + sum("_name").field(FIELD_NAME), iw -> { for (double value : values) { iw.addDocument(singleton(new NumericDocValuesField(FIELD_NAME, NumericUtils.doubleToSortableLong(value)))); } }, result -> assertEquals(expected, result.getValue(), delta), - NumberFieldMapper.NumberType.DOUBLE + singleton(defaultFieldType(NumberType.DOUBLE)) + ); + } + + public void testUnmapped() throws IOException { + testCase(newMatchAllQuery(), + sum("_name").field("unknown_field"), + writer -> { + final int numDocs = randomIntBetween(10, 100); + for (int i = 0; i < numDocs; i++) { + writer.addDocument(singleton(new NumericDocValuesField(FIELD_NAME, randomLong()))); + } + }, + internalSum -> { + assertEquals(0d, internalSum.getValue(), 0d); + assertFalse(AggregationInspectionHelper.hasValue(internalSum)); + }, + singleton(defaultFieldType()) + ); + } + + public void testPartiallyUnmapped() throws IOException { + final MappedFieldType fieldType = new NumberFieldMapper.NumberFieldType(NumberType.LONG); + fieldType.setName(FIELD_NAME); + fieldType.setHasDocValues(true); + + final SumAggregationBuilder builder = sum("_name") + .field(fieldType.name()); + + final int numDocs = randomIntBetween(10, 100); + final List> docs = new ArrayList<>(numDocs); + int sum = 0; + for (int i = 0; i < numDocs; i++) { + final long value = randomLongBetween(0, 1000); + sum += value; + docs.add(singleton(new NumericDocValuesField(fieldType.name(), value))); + } + + try (Directory mappedDirectory = newDirectory(); Directory unmappedDirectory = newDirectory()) { + try (RandomIndexWriter mappedWriter = new RandomIndexWriter(random(), mappedDirectory)) { + mappedWriter.addDocuments(docs); + } + + new RandomIndexWriter(random(), unmappedDirectory).close(); + + try (IndexReader mappedReader = DirectoryReader.open(mappedDirectory); + IndexReader unmappedReader = DirectoryReader.open(unmappedDirectory); + MultiReader multiReader = new MultiReader(mappedReader, unmappedReader)) { + + final IndexSearcher searcher = newSearcher(multiReader, true, true); + + final InternalSum internalSum = search(searcher, newMatchAllQuery(), builder, fieldType); + assertEquals(sum, internalSum.getValue(), 0d); + assertTrue(AggregationInspectionHelper.hasValue(internalSum)); + } + } + } + + public void testValueScriptSingleValuedField() throws IOException { + scriptTestCase(1, + sum("_name") + .field(FIELD_NAME) + .script(new Script(ScriptType.INLINE, MockScriptEngine.NAME, VALUE_SCRIPT_NAME, emptyMap()))); + } + + public void testValueScriptMultiValuedField() throws IOException { + scriptTestCase(randomIntBetween(2, 5), + sum("_name") + .field(FIELD_NAME) + .script(new Script(ScriptType.INLINE, MockScriptEngine.NAME, VALUE_SCRIPT_NAME, emptyMap()))); + } + + public void testFieldScriptSingleValuedField() throws IOException { + scriptTestCase(1, + sum("_name") + .script(new Script(ScriptType.INLINE, MockScriptEngine.NAME, FIELD_SCRIPT_NAME, singletonMap("field", FIELD_NAME)))); + } + + public void testFieldScriptMultiValuedField() throws IOException { + scriptTestCase(randomIntBetween(2, 5), + sum("_name") + .script(new Script(ScriptType.INLINE, MockScriptEngine.NAME, FIELD_SCRIPT_NAME, singletonMap("field", FIELD_NAME)))); + } + + private void scriptTestCase(int numValuesPerField, SumAggregationBuilder builder) throws IOException { + final MappedFieldType fieldType = defaultFieldType(); + + final int numDocs = randomIntBetween(10, 100); + final List> docs = new ArrayList<>(numDocs); + long sum = 0; + for (int iDoc = 0; iDoc < numDocs; iDoc++) { + Set doc = new HashSet<>(); + for (int iValue = 0; iValue < numValuesPerField; iValue++) { + final long value = randomLongBetween(0, 1000); + sum += value; + doc.add(new SortedNumericDocValuesField(fieldType.name(), value)); + } + docs.add(doc); + } + final long expectedSum = sum + (numDocs * numValuesPerField); + + testCase(newMatchAllQuery(), + builder, + writer -> writer.addDocuments(docs), + internalSum -> { + assertEquals(expectedSum, internalSum.getValue(), 0d); + assertTrue(AggregationInspectionHelper.hasValue(internalSum)); + }, + singleton(fieldType) + ); + } + + public void testMissing() throws IOException { + final MappedFieldType aggField = defaultFieldType(); + final MappedFieldType irrelevantField = new NumberFieldMapper.NumberFieldType(NumberType.LONG); + irrelevantField.setName("irrelevant_field"); + + final int numDocs = randomIntBetween(10, 100); + final long missingValue = randomLongBetween(1, 1000); + long sum = 0; + final List> docs = new ArrayList<>(numDocs); + for (int i = 0; i < numDocs; i++) { + if (randomBoolean()) { + final long value = randomLongBetween(0, 1000); + sum += value; + docs.add(singleton(new NumericDocValuesField(aggField.name(), value))); + } else { + sum += missingValue; + docs.add(singleton(new NumericDocValuesField(irrelevantField.name(), randomLong()))); + } + } + final long finalSum = sum; + + testCase(newMatchAllQuery(), + sum("_name") + .field(aggField.name()) + .missing(missingValue), + writer -> writer.addDocuments(docs), + internalSum -> { + assertEquals(finalSum, internalSum.getValue(), 0d); + assertTrue(AggregationInspectionHelper.hasValue(internalSum)); + }, + List.of(aggField, irrelevantField) + ); + } + + public void testMissingUnmapped() throws IOException { + final int numDocs = randomIntBetween(10, 100); + final long missingValue = randomLongBetween(1, 1000); + final long expectedSum = numDocs * missingValue; + + testCase(newMatchAllQuery(), + sum("_name") + .field("unknown_field") + .missing(missingValue), + writer -> { + for (int i = 0; i < numDocs; i++) { + writer.addDocument(singleton(new NumericDocValuesField(FIELD_NAME, randomLong()))); + } + }, + internalSum -> { + assertEquals(expectedSum, internalSum.getValue(), 0d); + assertTrue(AggregationInspectionHelper.hasValue(internalSum)); + }, + singleton(defaultFieldType()) ); } private void testCase(Query query, CheckedConsumer indexer, Consumer verify) throws IOException { - testCase(query, indexer, verify, NumberFieldMapper.NumberType.LONG); + testCase(query, sum("_name").field(FIELD_NAME), indexer, verify, singleton(defaultFieldType())); } private void testCase(Query query, + SumAggregationBuilder aggregationBuilder, CheckedConsumer indexer, Consumer verify, - NumberFieldMapper.NumberType fieldNumberType) throws IOException { + Collection fieldTypes) throws IOException { try (Directory directory = newDirectory()) { try (RandomIndexWriter indexWriter = new RandomIndexWriter(random(), directory)) { indexer.accept(indexWriter); @@ -194,21 +388,49 @@ private void testCase(Query query, try (IndexReader indexReader = DirectoryReader.open(directory)) { IndexSearcher indexSearcher = newSearcher(indexReader, true, true); - MappedFieldType fieldType = new NumberFieldMapper.NumberFieldType(fieldNumberType); - fieldType.setName(FIELD_NAME); - fieldType.setHasDocValues(true); + final MappedFieldType[] fieldTypesArray = fieldTypes.toArray(new MappedFieldType[0]); + final InternalSum internalSum = search(indexSearcher, query, aggregationBuilder, fieldTypesArray); + verify.accept(internalSum); + } + } + } - SumAggregationBuilder aggregationBuilder = new SumAggregationBuilder("_name"); - aggregationBuilder.field(FIELD_NAME); + @Override + protected List getSupportedValuesSourceTypes() { + return singletonList(CoreValuesSourceType.NUMERIC); + } - SumAggregator aggregator = createAggregator(aggregationBuilder, indexSearcher, fieldType); - aggregator.preCollection(); - indexSearcher.search(query, aggregator); - aggregator.postCollection(); + @Override + protected AggregationBuilder createAggBuilderForTypeTest(MappedFieldType fieldType, String fieldName) { + return new SumAggregationBuilder("_name") + .field(fieldName); + } - verify.accept((InternalSum) aggregator.buildAggregation(0L)); + @Override + protected ScriptService getMockScriptService() { + final Map, Object>> scripts = Map.of( + VALUE_SCRIPT_NAME, vars -> ((Number) vars.get("_value")).doubleValue() + 1, + FIELD_SCRIPT_NAME, vars -> { + final String fieldName = (String) vars.get("field"); + final LeafDocLookup lookup = (LeafDocLookup) vars.get("doc"); + return lookup.get(fieldName).stream() + .map(value -> ((Number) value).longValue() + 1) + .collect(toList()); } - } + ); + final MockScriptEngine engine = new MockScriptEngine(MockScriptEngine.NAME, scripts, emptyMap()); + final Map engines = singletonMap(engine.getType(), engine); + return new ScriptService(Settings.EMPTY, engines, ScriptModule.CORE_CONTEXTS); } + private static MappedFieldType defaultFieldType() { + return defaultFieldType(NumberType.LONG); + } + + private static MappedFieldType defaultFieldType(NumberType numberType) { + final MappedFieldType fieldType = new NumberFieldMapper.NumberFieldType(numberType); + fieldType.setName(FIELD_NAME); + fieldType.setHasDocValues(true); + return fieldType; + } } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/SumIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/SumIT.java index b63bd845c7a2c..fb6b021eda0bf 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/SumIT.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/SumIT.java @@ -35,9 +35,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; -import java.util.HashMap; import java.util.List; -import java.util.Map; import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery; import static org.elasticsearch.index.query.QueryBuilders.termQuery; @@ -48,8 +46,6 @@ import static org.elasticsearch.search.aggregations.AggregationBuilders.terms; import static org.elasticsearch.search.aggregations.metrics.MetricAggScriptPlugin.METRIC_SCRIPT_ENGINE; import static org.elasticsearch.search.aggregations.metrics.MetricAggScriptPlugin.RANDOM_SCRIPT; -import static org.elasticsearch.search.aggregations.metrics.MetricAggScriptPlugin.SUM_VALUES_FIELD_SCRIPT; -import static org.elasticsearch.search.aggregations.metrics.MetricAggScriptPlugin.VALUE_FIELD_SCRIPT; import static org.elasticsearch.search.aggregations.metrics.MetricAggScriptPlugin.VALUE_SCRIPT; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; @@ -111,20 +107,9 @@ public void testEmptyAggregation() throws Exception { assertThat(sum.getValue(), equalTo(0.0)); } + /** This test has been moved to {@link SumAggregatorTests#testUnmapped()} */ @Override - public void testUnmapped() throws Exception { - SearchResponse searchResponse = client().prepareSearch("idx_unmapped") - .setQuery(matchAllQuery()) - .addAggregation(sum("sum").field("value")) - .get(); - - assertThat(searchResponse.getHits().getTotalHits().value, equalTo(0L)); - - Sum sum = searchResponse.getAggregations().get("sum"); - assertThat(sum, notNullValue()); - assertThat(sum.getName(), equalTo("sum")); - assertThat(sum.getValue(), equalTo(0.0)); - } + public void testUnmapped() throws Exception {} @Override public void testSingleValuedField() throws Exception { @@ -179,120 +164,33 @@ public void testSingleValuedFieldGetProperty() throws Exception { assertThat((double) ((InternalAggregation)sum).getProperty("value"), equalTo(expectedSumValue)); } + /** This test has been moved to {@link SumAggregatorTests#testPartiallyUnmapped()} */ @Override - public void testSingleValuedFieldPartiallyUnmapped() throws Exception { - SearchResponse searchResponse = client().prepareSearch("idx", "idx_unmapped") - .setQuery(matchAllQuery()) - .addAggregation(sum("sum").field("value")) - .get(); - - assertHitCount(searchResponse, 10); - - Sum sum = searchResponse.getAggregations().get("sum"); - assertThat(sum, notNullValue()); - assertThat(sum.getName(), equalTo("sum")); - assertThat(sum.getValue(), equalTo((double) 1+2+3+4+5+6+7+8+9+10)); - } + public void testSingleValuedFieldPartiallyUnmapped() throws Exception {} + /** this test has been moved to {@link SumAggregatorTests#testValueScriptSingleValuedField()} */ @Override - public void testSingleValuedFieldWithValueScript() throws Exception { - SearchResponse searchResponse = client().prepareSearch("idx") - .setQuery(matchAllQuery()) - .addAggregation(sum("sum").field("value").script( - new Script(ScriptType.INLINE, METRIC_SCRIPT_ENGINE, VALUE_SCRIPT, Collections.emptyMap()))) - .get(); - - assertHitCount(searchResponse, 10); - - Sum sum = searchResponse.getAggregations().get("sum"); - assertThat(sum, notNullValue()); - assertThat(sum.getName(), equalTo("sum")); - assertThat(sum.getValue(), equalTo((double) 1+2+3+4+5+6+7+8+9+10)); - } + public void testSingleValuedFieldWithValueScript() throws Exception {} + /** this test has been moved to {@link SumAggregatorTests#testValueScriptSingleValuedField()} */ @Override - public void testSingleValuedFieldWithValueScriptWithParams() throws Exception { - Map params = new HashMap<>(); - params.put("increment", 1); - SearchResponse searchResponse = client().prepareSearch("idx") - .setQuery(matchAllQuery()) - .addAggregation(sum("sum").field("value").script(new Script(ScriptType.INLINE, METRIC_SCRIPT_ENGINE, VALUE_SCRIPT, params))) - .get(); - - assertHitCount(searchResponse, 10); - - Sum sum = searchResponse.getAggregations().get("sum"); - assertThat(sum, notNullValue()); - assertThat(sum.getName(), equalTo("sum")); - assertThat(sum.getValue(), equalTo((double) 1+2+3+4+5+6+7+8+9+10)); - } + public void testSingleValuedFieldWithValueScriptWithParams() throws Exception {} + /** this test has been moved to {@link SumAggregatorTests#testFieldScriptSingleValuedField()} */ @Override - public void testScriptSingleValued() throws Exception { - SearchResponse searchResponse = client().prepareSearch("idx") - .setQuery(matchAllQuery()) - .addAggregation(sum("sum").script( - new Script(ScriptType.INLINE, METRIC_SCRIPT_ENGINE, VALUE_FIELD_SCRIPT, Collections.singletonMap("field", "value")))) - .get(); - - assertHitCount(searchResponse, 10); - - Sum sum = searchResponse.getAggregations().get("sum"); - assertThat(sum, notNullValue()); - assertThat(sum.getName(), equalTo("sum")); - assertThat(sum.getValue(), equalTo((double) 1+2+3+4+5+6+7+8+9+10)); - } + public void testScriptSingleValued() throws Exception {} + /** this test has been moved to {@link SumAggregatorTests#testFieldScriptSingleValuedField()} */ @Override - public void testScriptSingleValuedWithParams() throws Exception { - Map params = new HashMap<>(); - params.put("inc", 1); - SearchResponse searchResponse = client().prepareSearch("idx") - .setQuery(matchAllQuery()) - .addAggregation(sum("sum").script(new Script(ScriptType.INLINE, METRIC_SCRIPT_ENGINE, VALUE_FIELD_SCRIPT, params))) - .get(); - - assertHitCount(searchResponse, 10); - - Sum sum = searchResponse.getAggregations().get("sum"); - assertThat(sum, notNullValue()); - assertThat(sum.getName(), equalTo("sum")); - assertThat(sum.getValue(), equalTo((double) 2+3+4+5+6+7+8+9+10+11)); - } + public void testScriptSingleValuedWithParams() throws Exception {} + /** this test has been moved to {@link SumAggregatorTests#testFieldScriptMultiValuedField()} */ @Override - public void testScriptMultiValued() throws Exception { - SearchResponse searchResponse = client().prepareSearch("idx") - .setQuery(matchAllQuery()) - .addAggregation(sum("sum").script( - new Script(ScriptType.INLINE, METRIC_SCRIPT_ENGINE, SUM_VALUES_FIELD_SCRIPT, Collections.emptyMap()))) - .get(); - - assertHitCount(searchResponse, 10); - - Sum sum = searchResponse.getAggregations().get("sum"); - assertThat(sum, notNullValue()); - assertThat(sum.getName(), equalTo("sum")); - assertThat(sum.getValue(), equalTo((double) 2+3+3+4+4+5+5+6+6+7+7+8+8+9+9+10+10+11+11+12)); - } + public void testScriptMultiValued() throws Exception {} + /** this test has been moved to {@link SumAggregatorTests#testFieldScriptMultiValuedField()} */ @Override - public void testScriptMultiValuedWithParams() throws Exception { - Map params = new HashMap<>(); - params.put("inc", 1); - SearchResponse searchResponse = client().prepareSearch("idx") - .setQuery(matchAllQuery()) - .addAggregation( - sum("sum").script(new Script(ScriptType.INLINE, METRIC_SCRIPT_ENGINE, SUM_VALUES_FIELD_SCRIPT, params))) - .get(); - - assertHitCount(searchResponse, 10); - - Sum sum = searchResponse.getAggregations().get("sum"); - assertThat(sum, notNullValue()); - assertThat(sum.getName(), equalTo("sum")); - assertThat(sum.getValue(), equalTo((double) 3+4+4+5+5+6+6+7+7+8+8+9+9+10+10+11+11+12+12+13)); - } + public void testScriptMultiValuedWithParams() throws Exception {} @Override public void testMultiValuedField() throws Exception { @@ -310,39 +208,13 @@ public void testMultiValuedField() throws Exception { assertThat(sum.getValue(), equalTo((double) 2+3+3+4+4+5+5+6+6+7+7+8+8+9+9+10+10+11+11+12)); } + /** this test has been moved to {@link SumAggregatorTests#testValueScriptMultiValuedField()} */ @Override - public void testMultiValuedFieldWithValueScript() throws Exception { - - SearchResponse searchResponse = client().prepareSearch("idx") - .setQuery(matchAllQuery()) - .addAggregation(sum("sum").field("values").script( - new Script(ScriptType.INLINE, METRIC_SCRIPT_ENGINE, VALUE_SCRIPT, Collections.emptyMap()))) - .get(); - - assertHitCount(searchResponse, 10); - - Sum sum = searchResponse.getAggregations().get("sum"); - assertThat(sum, notNullValue()); - assertThat(sum.getName(), equalTo("sum")); - assertThat(sum.getValue(), equalTo((double) 2 + 3 + 3 + 4 + 4 + 5 + 5 + 6 + 6 + 7 + 7 + 8 + 8 + 9 + 9 + 10 + 10 + 11 + 11 + 12)); - } + public void testMultiValuedFieldWithValueScript() throws Exception {} + /** this test has been moved to {@link SumAggregatorTests#testValueScriptMultiValuedField()} */ @Override - public void testMultiValuedFieldWithValueScriptWithParams() throws Exception { - Map params = new HashMap<>(); - params.put("increment", 1); - SearchResponse searchResponse = client().prepareSearch("idx").setQuery(matchAllQuery()) - .addAggregation(sum("sum").field("values") - .script(new Script(ScriptType.INLINE, METRIC_SCRIPT_ENGINE, VALUE_SCRIPT, params))) - .get(); - - assertHitCount(searchResponse, 10); - - Sum sum = searchResponse.getAggregations().get("sum"); - assertThat(sum, notNullValue()); - assertThat(sum.getName(), equalTo("sum")); - assertThat(sum.getValue(), equalTo((double) 2 + 3 + 3 + 4 + 4 + 5 + 5 + 6 + 6 + 7 + 7 + 8 + 8 + 9 + 9 + 10 + 10 + 11 + 11 + 12)); - } + public void testMultiValuedFieldWithValueScriptWithParams() throws Exception {} @Override public void testOrderByEmptyAggregation() throws Exception { From eda3affbb5c9a592af310ff3e9a8b8ed89ad41d2 Mon Sep 17 00:00:00 2001 From: Andy Bristol Date: Mon, 16 Mar 2020 12:54:58 -0700 Subject: [PATCH 2/3] generalize script test case --- .../metrics/SumAggregatorTests.java | 127 +++++++++--------- 1 file changed, 67 insertions(+), 60 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/SumAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/SumAggregatorTests.java index dbaa20836f2e2..c25ce9216f371 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/SumAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/SumAggregatorTests.java @@ -37,6 +37,7 @@ import org.apache.lucene.util.BytesRef; import org.apache.lucene.util.NumericUtils; import org.elasticsearch.common.CheckedConsumer; +import org.elasticsearch.common.TriConsumer; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.index.mapper.NumberFieldMapper; @@ -206,19 +207,12 @@ private void verifySummationOfDoubles(double[] values, double expected, double d } public void testUnmapped() throws IOException { - testCase(newMatchAllQuery(), + sumRandomDocsTestCase(randomIntBetween(1, 5), sum("_name").field("unknown_field"), - writer -> { - final int numDocs = randomIntBetween(10, 100); - for (int i = 0; i < numDocs; i++) { - writer.addDocument(singleton(new NumericDocValuesField(FIELD_NAME, randomLong()))); - } - }, - internalSum -> { - assertEquals(0d, internalSum.getValue(), 0d); - assertFalse(AggregationInspectionHelper.hasValue(internalSum)); - }, - singleton(defaultFieldType()) + (sum, docs, result) -> { + assertEquals(0d, result.getValue(), 0d); + assertFalse(AggregationInspectionHelper.hasValue(result)); + } ); } @@ -260,56 +254,50 @@ public void testPartiallyUnmapped() throws IOException { } public void testValueScriptSingleValuedField() throws IOException { - scriptTestCase(1, + sumRandomDocsTestCase(1, sum("_name") .field(FIELD_NAME) - .script(new Script(ScriptType.INLINE, MockScriptEngine.NAME, VALUE_SCRIPT_NAME, emptyMap()))); + .script(new Script(ScriptType.INLINE, MockScriptEngine.NAME, VALUE_SCRIPT_NAME, emptyMap())), + (sum, docs, result) -> { + assertEquals(sum + docs.size(), result.getValue(), 0d); + assertTrue(AggregationInspectionHelper.hasValue(result)); + } + ); } public void testValueScriptMultiValuedField() throws IOException { - scriptTestCase(randomIntBetween(2, 5), + final int valuesPerField = randomIntBetween(2, 5); + sumRandomDocsTestCase(valuesPerField, sum("_name") .field(FIELD_NAME) - .script(new Script(ScriptType.INLINE, MockScriptEngine.NAME, VALUE_SCRIPT_NAME, emptyMap()))); + .script(new Script(ScriptType.INLINE, MockScriptEngine.NAME, VALUE_SCRIPT_NAME, emptyMap())), + (sum, docs, result) -> { + assertEquals(sum + (docs.size() * valuesPerField), result.getValue(), 0d); + assertTrue(AggregationInspectionHelper.hasValue(result)); + } + ); } public void testFieldScriptSingleValuedField() throws IOException { - scriptTestCase(1, + sumRandomDocsTestCase(1, sum("_name") - .script(new Script(ScriptType.INLINE, MockScriptEngine.NAME, FIELD_SCRIPT_NAME, singletonMap("field", FIELD_NAME)))); + .script(new Script(ScriptType.INLINE, MockScriptEngine.NAME, FIELD_SCRIPT_NAME, singletonMap("field", FIELD_NAME))), + (sum, docs, result) -> { + assertEquals(sum + docs.size(), result.getValue(), 0d); + assertTrue(AggregationInspectionHelper.hasValue(result)); + } + ); } public void testFieldScriptMultiValuedField() throws IOException { - scriptTestCase(randomIntBetween(2, 5), + final int valuesPerField = randomIntBetween(2, 5); + sumRandomDocsTestCase(valuesPerField, sum("_name") - .script(new Script(ScriptType.INLINE, MockScriptEngine.NAME, FIELD_SCRIPT_NAME, singletonMap("field", FIELD_NAME)))); - } - - private void scriptTestCase(int numValuesPerField, SumAggregationBuilder builder) throws IOException { - final MappedFieldType fieldType = defaultFieldType(); - - final int numDocs = randomIntBetween(10, 100); - final List> docs = new ArrayList<>(numDocs); - long sum = 0; - for (int iDoc = 0; iDoc < numDocs; iDoc++) { - Set doc = new HashSet<>(); - for (int iValue = 0; iValue < numValuesPerField; iValue++) { - final long value = randomLongBetween(0, 1000); - sum += value; - doc.add(new SortedNumericDocValuesField(fieldType.name(), value)); + .script(new Script(ScriptType.INLINE, MockScriptEngine.NAME, FIELD_SCRIPT_NAME, singletonMap("field", FIELD_NAME))), + (sum, docs, result) -> { + assertEquals(sum + (docs.size() * valuesPerField), result.getValue(), 0d); + assertTrue(AggregationInspectionHelper.hasValue(result)); } - docs.add(doc); - } - final long expectedSum = sum + (numDocs * numValuesPerField); - - testCase(newMatchAllQuery(), - builder, - writer -> writer.addDocuments(docs), - internalSum -> { - assertEquals(expectedSum, internalSum.getValue(), 0d); - assertTrue(AggregationInspectionHelper.hasValue(internalSum)); - }, - singleton(fieldType) ); } @@ -348,24 +336,43 @@ public void testMissing() throws IOException { } public void testMissingUnmapped() throws IOException { - final int numDocs = randomIntBetween(10, 100); final long missingValue = randomLongBetween(1, 1000); - final long expectedSum = numDocs * missingValue; - - testCase(newMatchAllQuery(), + sumRandomDocsTestCase(randomIntBetween(1, 5), sum("_name") .field("unknown_field") .missing(missingValue), - writer -> { - for (int i = 0; i < numDocs; i++) { - writer.addDocument(singleton(new NumericDocValuesField(FIELD_NAME, randomLong()))); - } - }, - internalSum -> { - assertEquals(expectedSum, internalSum.getValue(), 0d); - assertTrue(AggregationInspectionHelper.hasValue(internalSum)); - }, - singleton(defaultFieldType()) + (sum, docs, result) -> { + assertEquals(docs.size() * missingValue, result.getValue(), 0d); + assertTrue(AggregationInspectionHelper.hasValue(result)); + } + ); + } + + private void sumRandomDocsTestCase(int valuesPerField, + SumAggregationBuilder builder, + TriConsumer>, InternalSum> verify) throws IOException { + + final MappedFieldType fieldType = defaultFieldType(); + + final int numDocs = randomIntBetween(10, 100); + final List> docs = new ArrayList<>(numDocs); + long sum = 0; + for (int iDoc = 0; iDoc < numDocs; iDoc++) { + Set doc = new HashSet<>(); + for (int iValue = 0; iValue < valuesPerField; iValue++) { + final long value = randomLongBetween(0, 1000); + sum += value; + doc.add(new SortedNumericDocValuesField(fieldType.name(), value)); + } + docs.add(doc); + } + final long finalSum = sum; + + testCase(newMatchAllQuery(), + builder, + writer -> writer.addDocuments(docs), + internalSum -> verify.apply(finalSum, docs, internalSum), + singleton(fieldType) ); } From 7ceb965957b735980e4beca2b77044c72de3c7a9 Mon Sep 17 00:00:00 2001 From: Andy Bristol Date: Mon, 16 Mar 2020 13:17:46 -0700 Subject: [PATCH 3/3] use query constructor instead of convenience method --- .../metrics/SumAggregatorTests.java | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/SumAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/SumAggregatorTests.java index c25ce9216f371..ceeed769d3082 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/SumAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/SumAggregatorTests.java @@ -31,6 +31,7 @@ import org.apache.lucene.index.Term; import org.apache.lucene.search.DocValuesFieldExistsQuery; import org.apache.lucene.search.IndexSearcher; +import org.apache.lucene.search.MatchAllDocsQuery; import org.apache.lucene.search.Query; import org.apache.lucene.search.TermQuery; import org.apache.lucene.store.Directory; @@ -71,7 +72,6 @@ import static java.util.Collections.singletonList; import static java.util.Collections.singletonMap; import static java.util.stream.Collectors.toList; -import static org.elasticsearch.common.lucene.search.Queries.newMatchAllQuery; import static org.elasticsearch.search.aggregations.AggregationBuilders.sum; public class SumAggregatorTests extends AggregatorTestCase { @@ -81,7 +81,7 @@ public class SumAggregatorTests extends AggregatorTestCase { private static final String FIELD_SCRIPT_NAME = "field_script"; public void testNoDocs() throws IOException { - testCase(newMatchAllQuery(), iw -> { + testCase(new MatchAllDocsQuery(), iw -> { // Intentionally not writing any docs }, count -> { assertEquals(0L, count.getValue(), 0d); @@ -90,7 +90,7 @@ public void testNoDocs() throws IOException { } public void testNoMatchingField() throws IOException { - testCase(newMatchAllQuery(), iw -> { + testCase(new MatchAllDocsQuery(), iw -> { iw.addDocument(singleton(new NumericDocValuesField("wrong_number", 7))); iw.addDocument(singleton(new NumericDocValuesField("wrong_number", 1))); }, count -> { @@ -100,7 +100,7 @@ public void testNoMatchingField() throws IOException { } public void testNumericDocValues() throws IOException { - testCase(newMatchAllQuery(), iw -> { + testCase(new MatchAllDocsQuery(), iw -> { iw.addDocument(singleton(new NumericDocValuesField(FIELD_NAME, 1))); iw.addDocument(singleton(new NumericDocValuesField(FIELD_NAME, 2))); iw.addDocument(singleton(new NumericDocValuesField(FIELD_NAME, 1))); @@ -151,7 +151,7 @@ public void testQueryFiltering() throws IOException { public void testStringField() throws IOException { IllegalStateException e = expectThrows(IllegalStateException.class, () -> { - testCase(newMatchAllQuery(), iw -> { + testCase(new MatchAllDocsQuery(), iw -> { iw.addDocument(singleton(new SortedDocValuesField(FIELD_NAME, new BytesRef("1")))); }, count -> { assertEquals(0L, count.getValue(), 0d); @@ -194,7 +194,7 @@ public void testSummationAccuracy() throws IOException { } private void verifySummationOfDoubles(double[] values, double expected, double delta) throws IOException { - testCase(newMatchAllQuery(), + testCase(new MatchAllDocsQuery(), sum("_name").field(FIELD_NAME), iw -> { for (double value : values) { @@ -246,7 +246,7 @@ public void testPartiallyUnmapped() throws IOException { final IndexSearcher searcher = newSearcher(multiReader, true, true); - final InternalSum internalSum = search(searcher, newMatchAllQuery(), builder, fieldType); + final InternalSum internalSum = search(searcher, new MatchAllDocsQuery(), builder, fieldType); assertEquals(sum, internalSum.getValue(), 0d); assertTrue(AggregationInspectionHelper.hasValue(internalSum)); } @@ -322,7 +322,7 @@ public void testMissing() throws IOException { } final long finalSum = sum; - testCase(newMatchAllQuery(), + testCase(new MatchAllDocsQuery(), sum("_name") .field(aggField.name()) .missing(missingValue), @@ -368,7 +368,7 @@ private void sumRandomDocsTestCase(int valuesPerField, } final long finalSum = sum; - testCase(newMatchAllQuery(), + testCase(new MatchAllDocsQuery(), builder, writer -> writer.addDocuments(docs), internalSum -> verify.apply(finalSum, docs, internalSum),