From 462e4a24efd612aac3c15a27c3fd07603aff54c5 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Mon, 7 Nov 2022 09:58:18 -0500 Subject: [PATCH 1/5] Move bucket_selector agg to the agg module This moves the bucket selector aggregation to the aggregations module and ports all of it's ESIntegTestCase tests to REST tests which gets us mixed cluster testing. Relates to #26220 Relates to #90283 --- .../pipeline/BucketSelectorIT.java | 638 ------------------ .../aggregations/AggregationsPlugin.java | 6 + ...ketSelectorPipelineAggregationBuilder.java | 16 +- .../BucketSelectorPipelineAggregator.java | 3 +- ...BucketSelectorAggregationBuilderTests.java | 12 +- .../test/aggregations/bucket_selector.yml | 352 ++++++++++ .../BucketAggregationSelectorScript.java | 2 +- .../elasticsearch/search/SearchModule.java | 8 - .../PipelineAggregatorBuilders.java | 9 - .../script/MockScriptEngine.java | 9 - x-pack/plugin/sql/build.gradle | 1 + .../execution/search/CompositeAggCursor.java | 2 +- .../xpack/sql/querydsl/agg/AggFilter.java | 5 +- 13 files changed, 378 insertions(+), 685 deletions(-) delete mode 100644 modules/aggregations/src/internalClusterTest/java/org/elasticsearch/aggregations/pipeline/BucketSelectorIT.java rename {server/src/main/java/org/elasticsearch/search => modules/aggregations/src/main/java/org/elasticsearch}/aggregations/pipeline/BucketSelectorPipelineAggregationBuilder.java (94%) rename {server/src/main/java/org/elasticsearch/search => modules/aggregations/src/main/java/org/elasticsearch}/aggregations/pipeline/BucketSelectorPipelineAggregator.java (96%) rename server/src/test/java/org/elasticsearch/search/aggregations/pipeline/BucketSelectorTests.java => modules/aggregations/src/test/java/org/elasticsearch/aggregations/pipeline/BucketSelectorAggregationBuilderTests.java (84%) create mode 100644 modules/aggregations/src/yamlRestTest/resources/rest-api-spec/test/aggregations/bucket_selector.yml diff --git a/modules/aggregations/src/internalClusterTest/java/org/elasticsearch/aggregations/pipeline/BucketSelectorIT.java b/modules/aggregations/src/internalClusterTest/java/org/elasticsearch/aggregations/pipeline/BucketSelectorIT.java deleted file mode 100644 index 534f515ddb777..0000000000000 --- a/modules/aggregations/src/internalClusterTest/java/org/elasticsearch/aggregations/pipeline/BucketSelectorIT.java +++ /dev/null @@ -1,638 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0 and the Server Side Public License, v 1; you may not use this file except - * in compliance with, at your election, the Elastic License 2.0 or the Server - * Side Public License, v 1. - */ - -package org.elasticsearch.aggregations.pipeline; - -import org.elasticsearch.action.index.IndexRequestBuilder; -import org.elasticsearch.action.search.SearchResponse; -import org.elasticsearch.aggregations.AggregationsPlugin; -import org.elasticsearch.common.bytes.BytesArray; -import org.elasticsearch.plugins.Plugin; -import org.elasticsearch.script.MockScriptPlugin; -import org.elasticsearch.script.Script; -import org.elasticsearch.script.ScriptType; -import org.elasticsearch.search.aggregations.bucket.histogram.Histogram; -import org.elasticsearch.search.aggregations.bucket.histogram.Histogram.Bucket; -import org.elasticsearch.search.aggregations.metrics.Sum; -import org.elasticsearch.search.aggregations.pipeline.BucketHelpers.GapPolicy; -import org.elasticsearch.test.ESIntegTestCase; -import org.elasticsearch.xcontent.XContentBuilder; -import org.elasticsearch.xcontent.XContentType; - -import java.io.IOException; -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 java.util.function.Function; - -import static org.elasticsearch.search.aggregations.AggregationBuilders.histogram; -import static org.elasticsearch.search.aggregations.AggregationBuilders.sum; -import static org.elasticsearch.search.aggregations.PipelineAggregatorBuilders.bucketSelector; -import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; -import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse; -import static org.elasticsearch.xcontent.XContentFactory.jsonBuilder; -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.greaterThan; -import static org.hamcrest.Matchers.lessThan; -import static org.hamcrest.Matchers.notNullValue; -import static org.hamcrest.Matchers.nullValue; - -@ESIntegTestCase.SuiteScopeTestCase -public class BucketSelectorIT extends ESIntegTestCase { - - private static final String FIELD_1_NAME = "field1"; - private static final String FIELD_2_NAME = "field2"; - private static final String FIELD_3_NAME = "field3"; - private static final String FIELD_4_NAME = "field4"; - - private static int interval; - private static int numDocs; - private static int minNumber; - private static int maxNumber; - - @Override - protected Collection> nodePlugins() { - return List.of(CustomScriptPlugin.class, AggregationsPlugin.class); - } - - public static class CustomScriptPlugin extends MockScriptPlugin { - - @Override - protected Map, Object>> pluginScripts() { - Map, Object>> scripts = new HashMap<>(); - - scripts.put("Double.isNaN(_value0) ? false : (_value0 + _value1 > 100)", vars -> { - double value0 = (double) vars.get("_value0"); - double value1 = (double) vars.get("_value1"); - return Double.isNaN(value0) ? false : (value0 + value1 > 100); - }); - - scripts.put("Double.isNaN(_value0) ? true : (_value0 < 10000)", vars -> { - double value0 = (double) vars.get("_value0"); - return Double.isNaN(value0) ? true : (value0 < 10000); - }); - - scripts.put("Double.isNaN(_value0) ? false : (_value0 > 10000)", vars -> { - double value0 = (double) vars.get("_value0"); - return Double.isNaN(value0) ? false : (value0 > 10000); - }); - - scripts.put("Double.isNaN(_value0) ? false : (_value0 < _value1)", vars -> { - double value0 = (double) vars.get("_value0"); - double value1 = (double) vars.get("_value1"); - return Double.isNaN(value0) ? false : (value0 < value1); - }); - - scripts.put("Double.isNaN(_value0) ? false : (_value0 > 100)", vars -> { - double value0 = (double) vars.get("_value0"); - return Double.isNaN(value0) ? false : (value0 > 10000); - }); - - scripts.put("Double.isNaN(my_value1) ? false : (my_value1 + my_value2 > 100)", vars -> { - double myValue1 = (double) vars.get("my_value1"); - double myValue2 = (double) vars.get("my_value2"); - return Double.isNaN(myValue1) ? false : (myValue1 + myValue2 > 100); - }); - - scripts.put("Double.isNaN(_value0) ? false : (_value0 + _value1 > threshold)", vars -> { - double value0 = (double) vars.get("_value0"); - double value1 = (double) vars.get("_value1"); - int threshold = (int) vars.get("threshold"); - return Double.isNaN(value0) ? false : (value0 + value1 > threshold); - }); - - scripts.put("_value0 + _value1 > 100", vars -> { - double value0 = (double) vars.get("_value0"); - double value1 = (double) vars.get("_value1"); - return (value0 + value1 > 100); - }); - - scripts.put("my_script", vars -> { - double value0 = (double) vars.get("_value0"); - double value1 = (double) vars.get("_value1"); - return Double.isNaN(value0) ? false : (value0 + value1 > 100); - }); - - return scripts; - } - } - - @Override - public void setupSuiteScopeCluster() throws Exception { - createIndex("idx"); - createIndex("idx_unmapped"); - createIndex("idx_with_gaps"); - - interval = randomIntBetween(1, 50); - numDocs = randomIntBetween(10, 500); - minNumber = -200; - maxNumber = 200; - - List builders = new ArrayList<>(); - for (int docs = 0; docs < numDocs; docs++) { - builders.add(client().prepareIndex("idx").setSource(newDocBuilder())); - } - builders.add(client().prepareIndex("idx_with_gaps").setSource(newDocBuilder(1, 1, 0, 0))); - builders.add(client().prepareIndex("idx_with_gaps").setSource(newDocBuilder(1, 2, 0, 0))); - builders.add(client().prepareIndex("idx_with_gaps").setSource(newDocBuilder(3, 1, 0, 0))); - builders.add(client().prepareIndex("idx_with_gaps").setSource(newDocBuilder(3, 3, 0, 0))); - - indexRandom(true, builders); - ensureSearchable(); - } - - private XContentBuilder newDocBuilder() throws IOException { - return newDocBuilder( - randomIntBetween(minNumber, maxNumber), - randomIntBetween(minNumber, maxNumber), - randomIntBetween(minNumber, maxNumber), - randomIntBetween(minNumber, maxNumber) - ); - } - - private XContentBuilder newDocBuilder(int field1Value, int field2Value, int field3Value, int field4Value) throws IOException { - XContentBuilder jsonBuilder = jsonBuilder(); - jsonBuilder.startObject(); - jsonBuilder.field(FIELD_1_NAME, field1Value); - jsonBuilder.field(FIELD_2_NAME, field2Value); - jsonBuilder.field(FIELD_3_NAME, field3Value); - jsonBuilder.field(FIELD_4_NAME, field4Value); - jsonBuilder.endObject(); - return jsonBuilder; - } - - public void testInlineScript() { - Script script = new Script( - ScriptType.INLINE, - CustomScriptPlugin.NAME, - "Double.isNaN(_value0) ? false : (_value0 + _value1 > 100)", - Collections.emptyMap() - ); - - SearchResponse response = client().prepareSearch("idx") - .addAggregation( - histogram("histo").field(FIELD_1_NAME) - .interval(interval) - .subAggregation(sum("field2Sum").field(FIELD_2_NAME)) - .subAggregation(sum("field3Sum").field(FIELD_3_NAME)) - .subAggregation(bucketSelector("bucketSelector", script, "field2Sum", "field3Sum")) - ) - .get(); - - assertSearchResponse(response); - - Histogram histo = response.getAggregations().get("histo"); - assertThat(histo, notNullValue()); - assertThat(histo.getName(), equalTo("histo")); - List buckets = histo.getBuckets(); - - for (int i = 0; i < buckets.size(); ++i) { - Histogram.Bucket bucket = buckets.get(i); - Sum field2Sum = bucket.getAggregations().get("field2Sum"); - assertThat(field2Sum, notNullValue()); - double field2SumValue = field2Sum.value(); - Sum field3Sum = bucket.getAggregations().get("field3Sum"); - assertThat(field3Sum, notNullValue()); - double field3SumValue = field3Sum.value(); - assertThat(field2SumValue + field3SumValue, greaterThan(100.0)); - } - } - - public void testInlineScriptNoBucketsPruned() { - Script script = new Script( - ScriptType.INLINE, - CustomScriptPlugin.NAME, - "Double.isNaN(_value0) ? true : (_value0 < 10000)", - Collections.emptyMap() - ); - - SearchResponse response = client().prepareSearch("idx") - .addAggregation( - histogram("histo").field(FIELD_1_NAME) - .interval(interval) - .subAggregation(sum("field2Sum").field(FIELD_2_NAME)) - .subAggregation(sum("field3Sum").field(FIELD_3_NAME)) - .subAggregation(bucketSelector("bucketSelector", script, "field2Sum", "field3Sum")) - ) - .get(); - - assertSearchResponse(response); - - Histogram histo = response.getAggregations().get("histo"); - assertThat(histo, notNullValue()); - assertThat(histo.getName(), equalTo("histo")); - List buckets = histo.getBuckets(); - - for (int i = 0; i < buckets.size(); ++i) { - Histogram.Bucket bucket = buckets.get(i); - Sum field2Sum = bucket.getAggregations().get("field2Sum"); - assertThat(field2Sum, notNullValue()); - double field2SumValue = field2Sum.value(); - Sum field3Sum = bucket.getAggregations().get("field3Sum"); - assertThat(field3Sum, notNullValue()); - double field3SumValue = field3Sum.value(); - assertThat(field2SumValue + field3SumValue, lessThan(10000.0)); - } - } - - public void testInlineScriptNoBucketsLeft() { - Script script = new Script( - ScriptType.INLINE, - CustomScriptPlugin.NAME, - "Double.isNaN(_value0) ? false : (_value0 > 10000)", - Collections.emptyMap() - ); - - SearchResponse response = client().prepareSearch("idx") - .addAggregation( - histogram("histo").field(FIELD_1_NAME) - .interval(interval) - .subAggregation(sum("field2Sum").field(FIELD_2_NAME)) - .subAggregation(sum("field3Sum").field(FIELD_3_NAME)) - .subAggregation(bucketSelector("bucketSelector", script, "field2Sum", "field3Sum")) - ) - .get(); - - assertSearchResponse(response); - - Histogram histo = response.getAggregations().get("histo"); - assertThat(histo, notNullValue()); - assertThat(histo.getName(), equalTo("histo")); - List buckets = histo.getBuckets(); - assertThat(buckets.size(), equalTo(0)); - } - - public void testInlineScript2() { - Script script = new Script( - ScriptType.INLINE, - CustomScriptPlugin.NAME, - "Double.isNaN(_value0) ? false : (_value0 < _value1)", - Collections.emptyMap() - ); - - SearchResponse response = client().prepareSearch("idx") - .addAggregation( - histogram("histo").field(FIELD_1_NAME) - .interval(interval) - .subAggregation(sum("field2Sum").field(FIELD_2_NAME)) - .subAggregation(sum("field3Sum").field(FIELD_3_NAME)) - .subAggregation(bucketSelector("bucketSelector", script, "field2Sum", "field3Sum")) - ) - .get(); - - assertSearchResponse(response); - - Histogram histo = response.getAggregations().get("histo"); - assertThat(histo, notNullValue()); - assertThat(histo.getName(), equalTo("histo")); - List buckets = histo.getBuckets(); - - for (int i = 0; i < buckets.size(); ++i) { - Histogram.Bucket bucket = buckets.get(i); - Sum field2Sum = bucket.getAggregations().get("field2Sum"); - assertThat(field2Sum, notNullValue()); - double field2SumValue = field2Sum.value(); - Sum field3Sum = bucket.getAggregations().get("field3Sum"); - assertThat(field3Sum, notNullValue()); - double field3SumValue = field3Sum.value(); - assertThat(field3SumValue - field2SumValue, greaterThan(0.0)); - } - } - - public void testInlineScriptSingleVariable() { - Script script = new Script( - ScriptType.INLINE, - CustomScriptPlugin.NAME, - "Double.isNaN(_value0) ? false : (_value0 > 100)", - Collections.emptyMap() - ); - - SearchResponse response = client().prepareSearch("idx") - .addAggregation( - histogram("histo").field(FIELD_1_NAME) - .interval(interval) - .subAggregation(sum("field2Sum").field(FIELD_2_NAME)) - .subAggregation(bucketSelector("bucketSelector", script, "field2Sum")) - ) - .get(); - - assertSearchResponse(response); - - Histogram histo = response.getAggregations().get("histo"); - assertThat(histo, notNullValue()); - assertThat(histo.getName(), equalTo("histo")); - List buckets = histo.getBuckets(); - - for (int i = 0; i < buckets.size(); ++i) { - Histogram.Bucket bucket = buckets.get(i); - Sum field2Sum = bucket.getAggregations().get("field2Sum"); - assertThat(field2Sum, notNullValue()); - double field2SumValue = field2Sum.value(); - assertThat(field2SumValue, greaterThan(100.0)); - } - } - - public void testInlineScriptNamedVars() { - Script script = new Script( - ScriptType.INLINE, - CustomScriptPlugin.NAME, - "Double.isNaN(my_value1) ? false : (my_value1 + my_value2 > 100)", - Collections.emptyMap() - ); - - Map bucketPathsMap = new HashMap<>(); - bucketPathsMap.put("my_value1", "field2Sum"); - bucketPathsMap.put("my_value2", "field3Sum"); - - SearchResponse response = client().prepareSearch("idx") - .addAggregation( - histogram("histo").field(FIELD_1_NAME) - .interval(interval) - .subAggregation(sum("field2Sum").field(FIELD_2_NAME)) - .subAggregation(sum("field3Sum").field(FIELD_3_NAME)) - .subAggregation(bucketSelector("bucketSelector", bucketPathsMap, script)) - ) - .get(); - - assertSearchResponse(response); - - Histogram histo = response.getAggregations().get("histo"); - assertThat(histo, notNullValue()); - assertThat(histo.getName(), equalTo("histo")); - List buckets = histo.getBuckets(); - - for (int i = 0; i < buckets.size(); ++i) { - Histogram.Bucket bucket = buckets.get(i); - Sum field2Sum = bucket.getAggregations().get("field2Sum"); - assertThat(field2Sum, notNullValue()); - double field2SumValue = field2Sum.value(); - Sum field3Sum = bucket.getAggregations().get("field3Sum"); - assertThat(field3Sum, notNullValue()); - double field3SumValue = field3Sum.value(); - assertThat(field2SumValue + field3SumValue, greaterThan(100.0)); - } - } - - public void testInlineScriptWithParams() { - Script script = new Script( - ScriptType.INLINE, - CustomScriptPlugin.NAME, - "Double.isNaN(_value0) ? false : (_value0 + _value1 > threshold)", - Collections.singletonMap("threshold", 100) - ); - - SearchResponse response = client().prepareSearch("idx") - .addAggregation( - histogram("histo").field(FIELD_1_NAME) - .interval(interval) - .subAggregation(sum("field2Sum").field(FIELD_2_NAME)) - .subAggregation(sum("field3Sum").field(FIELD_3_NAME)) - .subAggregation(bucketSelector("bucketSelector", script, "field2Sum", "field3Sum")) - ) - .get(); - - assertSearchResponse(response); - - Histogram histo = response.getAggregations().get("histo"); - assertThat(histo, notNullValue()); - assertThat(histo.getName(), equalTo("histo")); - List buckets = histo.getBuckets(); - - for (int i = 0; i < buckets.size(); ++i) { - Histogram.Bucket bucket = buckets.get(i); - Sum field2Sum = bucket.getAggregations().get("field2Sum"); - assertThat(field2Sum, notNullValue()); - double field2SumValue = field2Sum.value(); - Sum field3Sum = bucket.getAggregations().get("field3Sum"); - assertThat(field3Sum, notNullValue()); - double field3SumValue = field3Sum.value(); - assertThat(field2SumValue + field3SumValue, greaterThan(100.0)); - } - } - - public void testInlineScriptInsertZeros() { - Script script = new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "_value0 + _value1 > 100", Collections.emptyMap()); - - SearchResponse response = client().prepareSearch("idx") - .addAggregation( - histogram("histo").field(FIELD_1_NAME) - .interval(interval) - .subAggregation(sum("field2Sum").field(FIELD_2_NAME)) - .subAggregation(sum("field3Sum").field(FIELD_3_NAME)) - .subAggregation(bucketSelector("bucketSelector", script, "field2Sum", "field3Sum").gapPolicy(GapPolicy.INSERT_ZEROS)) - ) - .get(); - - assertSearchResponse(response); - - Histogram histo = response.getAggregations().get("histo"); - assertThat(histo, notNullValue()); - assertThat(histo.getName(), equalTo("histo")); - List buckets = histo.getBuckets(); - - for (int i = 0; i < buckets.size(); ++i) { - Histogram.Bucket bucket = buckets.get(i); - Sum field2Sum = bucket.getAggregations().get("field2Sum"); - assertThat(field2Sum, notNullValue()); - double field2SumValue = field2Sum.value(); - Sum field3Sum = bucket.getAggregations().get("field3Sum"); - assertThat(field3Sum, notNullValue()); - double field3SumValue = field3Sum.value(); - assertThat(field2SumValue + field3SumValue, greaterThan(100.0)); - } - } - - public void testStoredScript() { - assertAcked( - client().admin() - .cluster() - .preparePutStoredScript() - .setId("my_script") - // Source is not interpreted but my_script is defined in CustomScriptPlugin - .setContent(new BytesArray(formatted(""" - { - "script": { - "lang": "%s", - "source": "Double.isNaN(_value0) ? false : (_value0 + _value1 > 100)" - } - }""", CustomScriptPlugin.NAME)), XContentType.JSON) - ); - - Script script = new Script(ScriptType.STORED, null, "my_script", Collections.emptyMap()); - - SearchResponse response = client().prepareSearch("idx") - .addAggregation( - histogram("histo").field(FIELD_1_NAME) - .interval(interval) - .subAggregation(sum("field2Sum").field(FIELD_2_NAME)) - .subAggregation(sum("field3Sum").field(FIELD_3_NAME)) - .subAggregation(bucketSelector("bucketSelector", script, "field2Sum", "field3Sum")) - ) - .get(); - - assertSearchResponse(response); - - Histogram histo = response.getAggregations().get("histo"); - assertThat(histo, notNullValue()); - assertThat(histo.getName(), equalTo("histo")); - List buckets = histo.getBuckets(); - - for (int i = 0; i < buckets.size(); ++i) { - Histogram.Bucket bucket = buckets.get(i); - Sum field2Sum = bucket.getAggregations().get("field2Sum"); - assertThat(field2Sum, notNullValue()); - double field2SumValue = field2Sum.value(); - Sum field3Sum = bucket.getAggregations().get("field3Sum"); - assertThat(field3Sum, notNullValue()); - double field3SumValue = field3Sum.value(); - assertThat(field2SumValue + field3SumValue, greaterThan(100.0)); - } - } - - public void testUnmapped() throws Exception { - Script script = new Script( - ScriptType.INLINE, - CustomScriptPlugin.NAME, - "Double.isNaN(_value0) ? false : (_value0 + _value1 > 100)", - Collections.emptyMap() - ); - - SearchResponse response = client().prepareSearch("idx_unmapped") - .addAggregation( - histogram("histo").field(FIELD_1_NAME) - .interval(interval) - .subAggregation(sum("field2Sum").field(FIELD_2_NAME)) - .subAggregation(sum("field3Sum").field(FIELD_3_NAME)) - .subAggregation(bucketSelector("bucketSelector", script, "field2Sum", "field3Sum")) - ) - .get(); - - assertSearchResponse(response); - - Histogram deriv = response.getAggregations().get("histo"); - assertThat(deriv, notNullValue()); - assertThat(deriv.getName(), equalTo("histo")); - assertThat(deriv.getBuckets().size(), equalTo(0)); - } - - public void testPartiallyUnmapped() throws Exception { - Script script = new Script( - ScriptType.INLINE, - CustomScriptPlugin.NAME, - "Double.isNaN(_value0) ? false : (_value0 + _value1 > 100)", - Collections.emptyMap() - ); - - SearchResponse response = client().prepareSearch("idx", "idx_unmapped") - .addAggregation( - histogram("histo").field(FIELD_1_NAME) - .interval(interval) - .subAggregation(sum("field2Sum").field(FIELD_2_NAME)) - .subAggregation(sum("field3Sum").field(FIELD_3_NAME)) - .subAggregation(bucketSelector("bucketSelector", script, "field2Sum", "field3Sum")) - ) - .get(); - - assertSearchResponse(response); - - Histogram histo = response.getAggregations().get("histo"); - assertThat(histo, notNullValue()); - assertThat(histo.getName(), equalTo("histo")); - List buckets = histo.getBuckets(); - - for (int i = 0; i < buckets.size(); ++i) { - Histogram.Bucket bucket = buckets.get(i); - Sum field2Sum = bucket.getAggregations().get("field2Sum"); - assertThat(field2Sum, notNullValue()); - double field2SumValue = field2Sum.value(); - Sum field3Sum = bucket.getAggregations().get("field3Sum"); - assertThat(field3Sum, notNullValue()); - double field3SumValue = field3Sum.value(); - assertThat(field2SumValue + field3SumValue, greaterThan(100.0)); - } - } - - public void testEmptyBuckets() { - SearchResponse response = client().prepareSearch("idx_with_gaps") - .addAggregation( - histogram("histo").field(FIELD_1_NAME) - .interval(1) - .subAggregation( - histogram("inner_histo").field(FIELD_1_NAME) - .interval(1) - .extendedBounds(1L, 4L) - .minDocCount(0) - .subAggregation( - new DerivativePipelineAggregationBuilder("derivative", "_count").gapPolicy(GapPolicy.INSERT_ZEROS) - ) - ) - ) - .get(); - - assertSearchResponse(response); - - Histogram histo = response.getAggregations().get("histo"); - assertThat(histo, notNullValue()); - assertThat(histo.getName(), equalTo("histo")); - List buckets = histo.getBuckets(); - assertThat(buckets.size(), equalTo(3)); - - Histogram.Bucket bucket = buckets.get(0); - assertThat(bucket, notNullValue()); - assertThat(bucket.getKeyAsString(), equalTo("1.0")); - Histogram innerHisto = bucket.getAggregations().get("inner_histo"); - assertThat(innerHisto, notNullValue()); - List innerBuckets = innerHisto.getBuckets(); - assertThat(innerBuckets, notNullValue()); - assertThat(innerBuckets.size(), equalTo(4)); - for (int i = 0; i < innerBuckets.size(); i++) { - Histogram.Bucket innerBucket = innerBuckets.get(i); - if (i == 0) { - assertThat(innerBucket.getAggregations().get("derivative"), nullValue()); - } else { - assertThat(innerBucket.getAggregations().get("derivative"), notNullValue()); - } - } - - bucket = buckets.get(1); - assertThat(bucket, notNullValue()); - assertThat(bucket.getKeyAsString(), equalTo("2.0")); - innerHisto = bucket.getAggregations().get("inner_histo"); - assertThat(innerHisto, notNullValue()); - innerBuckets = innerHisto.getBuckets(); - assertThat(innerBuckets, notNullValue()); - assertThat(innerBuckets.size(), equalTo(4)); - for (int i = 0; i < innerBuckets.size(); i++) { - Histogram.Bucket innerBucket = innerBuckets.get(i); - if (i == 0) { - assertThat(innerBucket.getAggregations().get("derivative"), nullValue()); - } else { - assertThat(innerBucket.getAggregations().get("derivative"), notNullValue()); - } - } - bucket = buckets.get(2); - assertThat(bucket, notNullValue()); - assertThat(bucket.getKeyAsString(), equalTo("3.0")); - innerHisto = bucket.getAggregations().get("inner_histo"); - assertThat(innerHisto, notNullValue()); - innerBuckets = innerHisto.getBuckets(); - assertThat(innerBuckets, notNullValue()); - assertThat(innerBuckets.size(), equalTo(4)); - for (int i = 0; i < innerBuckets.size(); i++) { - Histogram.Bucket innerBucket = innerBuckets.get(i); - if (i == 0) { - assertThat(innerBucket.getAggregations().get("derivative"), nullValue()); - } else { - assertThat(innerBucket.getAggregations().get("derivative"), notNullValue()); - } - } - } -} diff --git a/modules/aggregations/src/main/java/org/elasticsearch/aggregations/AggregationsPlugin.java b/modules/aggregations/src/main/java/org/elasticsearch/aggregations/AggregationsPlugin.java index b9c2723d4ad78..f304aa5422d78 100644 --- a/modules/aggregations/src/main/java/org/elasticsearch/aggregations/AggregationsPlugin.java +++ b/modules/aggregations/src/main/java/org/elasticsearch/aggregations/AggregationsPlugin.java @@ -12,6 +12,7 @@ import org.elasticsearch.aggregations.bucket.adjacency.InternalAdjacencyMatrix; import org.elasticsearch.aggregations.bucket.histogram.AutoDateHistogramAggregationBuilder; import org.elasticsearch.aggregations.bucket.histogram.InternalAutoDateHistogram; +import org.elasticsearch.aggregations.pipeline.BucketSelectorPipelineAggregationBuilder; import org.elasticsearch.aggregations.pipeline.BucketSortPipelineAggregationBuilder; import org.elasticsearch.aggregations.pipeline.Derivative; import org.elasticsearch.aggregations.pipeline.DerivativePipelineAggregationBuilder; @@ -45,6 +46,11 @@ public List getAggregations() { @Override public List getPipelineAggregations() { return List.of( + new PipelineAggregationSpec( + BucketSelectorPipelineAggregationBuilder.NAME, + BucketSelectorPipelineAggregationBuilder::new, + BucketSelectorPipelineAggregationBuilder::parse + ), new PipelineAggregationSpec( BucketSortPipelineAggregationBuilder.NAME, BucketSortPipelineAggregationBuilder::new, diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/BucketSelectorPipelineAggregationBuilder.java b/modules/aggregations/src/main/java/org/elasticsearch/aggregations/pipeline/BucketSelectorPipelineAggregationBuilder.java similarity index 94% rename from server/src/main/java/org/elasticsearch/search/aggregations/pipeline/BucketSelectorPipelineAggregationBuilder.java rename to modules/aggregations/src/main/java/org/elasticsearch/aggregations/pipeline/BucketSelectorPipelineAggregationBuilder.java index e11754c504488..15bd818ca98b7 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/BucketSelectorPipelineAggregationBuilder.java +++ b/modules/aggregations/src/main/java/org/elasticsearch/aggregations/pipeline/BucketSelectorPipelineAggregationBuilder.java @@ -6,14 +6,16 @@ * Side Public License, v 1. */ -package org.elasticsearch.search.aggregations.pipeline; +package org.elasticsearch.aggregations.pipeline; import org.elasticsearch.Version; import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.script.Script; +import org.elasticsearch.search.aggregations.pipeline.AbstractPipelineAggregationBuilder; import org.elasticsearch.search.aggregations.pipeline.BucketHelpers.GapPolicy; +import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator; import org.elasticsearch.xcontent.XContentBuilder; import org.elasticsearch.xcontent.XContentParser; @@ -41,10 +43,6 @@ public BucketSelectorPipelineAggregationBuilder(String name, Map this.script = script; } - public BucketSelectorPipelineAggregationBuilder(String name, Script script, String... bucketsPaths) { - this(name, convertToBucketsPathMap(bucketsPaths), script); - } - /** * Read from a stream. */ @@ -62,14 +60,6 @@ protected void doWriteTo(StreamOutput out) throws IOException { gapPolicy.writeTo(out); } - private static Map convertToBucketsPathMap(String[] bucketsPaths) { - Map bucketsPathsMap = new HashMap<>(); - for (int i = 0; i < bucketsPaths.length; i++) { - bucketsPathsMap.put("_value" + i, bucketsPaths[i]); - } - return bucketsPathsMap; - } - /** * Sets the gap policy to use for this aggregation. */ diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/BucketSelectorPipelineAggregator.java b/modules/aggregations/src/main/java/org/elasticsearch/aggregations/pipeline/BucketSelectorPipelineAggregator.java similarity index 96% rename from server/src/main/java/org/elasticsearch/search/aggregations/pipeline/BucketSelectorPipelineAggregator.java rename to modules/aggregations/src/main/java/org/elasticsearch/aggregations/pipeline/BucketSelectorPipelineAggregator.java index b4bf5bdb054a1..23abc8a328601 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/BucketSelectorPipelineAggregator.java +++ b/modules/aggregations/src/main/java/org/elasticsearch/aggregations/pipeline/BucketSelectorPipelineAggregator.java @@ -6,7 +6,7 @@ * Side Public License, v 1. */ -package org.elasticsearch.search.aggregations.pipeline; +package org.elasticsearch.aggregations.pipeline; import org.elasticsearch.script.BucketAggregationSelectorScript; import org.elasticsearch.script.Script; @@ -14,6 +14,7 @@ import org.elasticsearch.search.aggregations.InternalAggregation; import org.elasticsearch.search.aggregations.InternalMultiBucketAggregation; import org.elasticsearch.search.aggregations.pipeline.BucketHelpers.GapPolicy; +import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator; import java.util.ArrayList; import java.util.HashMap; diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/BucketSelectorTests.java b/modules/aggregations/src/test/java/org/elasticsearch/aggregations/pipeline/BucketSelectorAggregationBuilderTests.java similarity index 84% rename from server/src/test/java/org/elasticsearch/search/aggregations/pipeline/BucketSelectorTests.java rename to modules/aggregations/src/test/java/org/elasticsearch/aggregations/pipeline/BucketSelectorAggregationBuilderTests.java index 29949f24c87fa..59bdd52193bd6 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/BucketSelectorTests.java +++ b/modules/aggregations/src/test/java/org/elasticsearch/aggregations/pipeline/BucketSelectorAggregationBuilderTests.java @@ -6,21 +6,29 @@ * Side Public License, v 1. */ -package org.elasticsearch.search.aggregations.pipeline; +package org.elasticsearch.aggregations.pipeline; +import org.elasticsearch.aggregations.AggregationsPlugin; +import org.elasticsearch.plugins.SearchPlugin; import org.elasticsearch.script.Script; import org.elasticsearch.script.ScriptType; import org.elasticsearch.search.aggregations.BasePipelineAggregationTestCase; import org.elasticsearch.search.aggregations.pipeline.BucketHelpers.GapPolicy; import java.util.HashMap; +import java.util.List; import java.util.Map; import static java.util.Collections.emptyList; import static java.util.Collections.emptyMap; import static org.hamcrest.CoreMatchers.equalTo; -public class BucketSelectorTests extends BasePipelineAggregationTestCase { +public class BucketSelectorAggregationBuilderTests extends BasePipelineAggregationTestCase { + + @Override + protected List plugins() { + return List.of(new AggregationsPlugin()); + } @Override protected BucketSelectorPipelineAggregationBuilder createTestAggregatorFactory() { diff --git a/modules/aggregations/src/yamlRestTest/resources/rest-api-spec/test/aggregations/bucket_selector.yml b/modules/aggregations/src/yamlRestTest/resources/rest-api-spec/test/aggregations/bucket_selector.yml new file mode 100644 index 0000000000000..23f0a6bceb444 --- /dev/null +++ b/modules/aggregations/src/yamlRestTest/resources/rest-api-spec/test/aggregations/bucket_selector.yml @@ -0,0 +1,352 @@ +setup: + - do: + bulk: + index: test + refresh: true + body: + - { "index": { } } + - { "a": 2, "b": 12, "c": 21, "gappy": 99 } + - { "index": { } } + - { "a": 3, "b": 56, "c": 12 } + - { "index": { } } + - { "a": 4, "b": 82, "c": 13, "gappy": 98 } + - { "index": { } } + - { "a": 5, "b": 12, "c": 54 } + +--- +prune some buckets: + - do: + search: + body: + aggs: + a: + histogram: + field: a + interval: 1 + aggs: + b: + sum: + field: b + c: + sum: + field: c + selector: + bucket_selector: + buckets_path: + b: b + c: c + script: params.b > params.c + + - length: { aggregations.a.buckets: 2 } + - match: { aggregations.a.buckets.0.key: 3.0 } + - match: { aggregations.a.buckets.1.key: 4.0 } + +--- +prune no buckets: + - do: + search: + body: + aggs: + a: + histogram: + field: a + interval: 1 + aggs: + b: + sum: + field: b + c: + sum: + field: c + selector: + bucket_selector: + buckets_path: + b: b + script: params.b > 0 + + - length: { aggregations.a.buckets: 4 } + - match: { aggregations.a.buckets.0.key: 2.0 } + - match: { aggregations.a.buckets.1.key: 3.0 } + - match: { aggregations.a.buckets.2.key: 4.0 } + - match: { aggregations.a.buckets.3.key: 5.0 } + +--- +prune all buckets: + - do: + search: + body: + aggs: + a: + histogram: + field: a + interval: 1 + aggs: + b: + sum: + field: b + c: + sum: + field: c + selector: + bucket_selector: + buckets_path: + b: b + script: params.b < 0 + + - length: { aggregations.a.buckets: 0 } + +--- +input is unmapped sum: + - do: + search: + body: + aggs: + a: + histogram: + field: a + interval: 1 + aggs: + b: + sum: + field: b + unmapped: + sum: + field: unmapped + selector: + bucket_selector: + buckets_path: + b: b + unmapped: unmapped + script: params.b > params.unmapped + + # sum(unmapped) is 0 so all of these pass + - length: { aggregations.a.buckets: 4 } + - match: { aggregations.a.buckets.0.key: 2.0 } + - match: { aggregations.a.buckets.1.key: 3.0 } + - match: { aggregations.a.buckets.2.key: 4.0 } + - match: { aggregations.a.buckets.3.key: 5.0 } + +--- +input is unmapped avg: + - do: + search: + body: + aggs: + a: + histogram: + field: a + interval: 1 + aggs: + b: + avg: + field: b + unmapped: + avg: + field: unmapped + selector: + bucket_selector: + buckets_path: + b: b + unmapped: unmapped + script: params.b > params.unmapped + + # unmapped avg is null which we see as NaN + - length: { aggregations.a.buckets: 0 } + +--- +params: + - do: + search: + body: + aggs: + a: + histogram: + field: a + interval: 1 + aggs: + b: + sum: + field: b + c: + sum: + field: c + selector: + bucket_selector: + buckets_path: + b: b + script: + source: params.b > params.min + params: + min: 1000 + + - length: { aggregations.a.buckets: 0 } + +--- +default gap_policy is skip: + - do: + search: + body: + aggs: + a: + histogram: + field: a + interval: 1 + aggs: + b: + sum: + field: b + gappy: + sum: + field: gappy + selector: + bucket_selector: + buckets_path: + b: b + gappy: gappy + script: params.b < params.gappy + + - length: { aggregations.a.buckets: 2 } + - match: { aggregations.a.buckets.0.key: 2.0 } + - match: { aggregations.a.buckets.1.key: 4.0 } + +--- +gap_policy is skip: + - do: + search: + body: + aggs: + a: + histogram: + field: a + interval: 1 + aggs: + b: + sum: + field: b + gappy: + sum: + field: gappy + selector: + bucket_selector: + gap_policy: skip + buckets_path: + b: b + gappy: gappy + script: params.b < params.gappy + + - length: { aggregations.a.buckets: 2 } + - match: { aggregations.a.buckets.0.key: 2.0 } + - match: { aggregations.a.buckets.1.key: 4.0 } + +--- +gap_policy is insert_zeros: + - do: + search: + body: + aggs: + a: + histogram: + field: a + interval: 1 + aggs: + b: + sum: + field: b + gappy: + sum: + field: gappy + selector: + bucket_selector: + gap_policy: insert_zeros + buckets_path: + b: b + gappy: gappy + script: params.b > params.gappy + + - length: { aggregations.a.buckets: 2 } + - match: { aggregations.a.buckets.0.key: 3.0 } + - match: { aggregations.a.buckets.1.key: 5.0 } + +--- +gap_policy is keep_values: + - do: + search: + body: + aggs: + a: + histogram: + field: a + interval: 1 + aggs: + b: + sum: + field: b + gappy: + sum: + field: gappy + selector: + bucket_selector: + gap_policy: keep_values + buckets_path: + b: b + gappy: gappy + script: params.b > params.gappy + + - length: { aggregations.a.buckets: 2 } + - match: { aggregations.a.buckets.0.key: 3.0 } + - match: { aggregations.a.buckets.1.key: 5.0 } + +--- +bad script: + - do: + catch: /NullPointerException/ + search: + body: + aggs: + a: + histogram: + field: a + interval: 1 + aggs: + b: + sum: + field: b + selector: + bucket_selector: + buckets_path: + b: b + script: params.garbage > 0 + +--- +invalid path: + - do: + catch: /No aggregation found for path \[missing\]/ + search: + body: + aggs: + a: + histogram: + field: a + interval: 1 + aggs: + selector: + bucket_selector: + buckets_path: + missing: missing + script: params.missing > 0 + +--- +top level fails: + - do: + catch: /bucket_selector aggregation \[selector\] must be declared inside of another aggregation/ + search: + body: + aggs: + b: + sum: + field: b + selector: + bucket_selector: + buckets_path: + b: b + script: params.b > 0 diff --git a/server/src/main/java/org/elasticsearch/script/BucketAggregationSelectorScript.java b/server/src/main/java/org/elasticsearch/script/BucketAggregationSelectorScript.java index 7815f0bfe5788..168b47d300593 100644 --- a/server/src/main/java/org/elasticsearch/script/BucketAggregationSelectorScript.java +++ b/server/src/main/java/org/elasticsearch/script/BucketAggregationSelectorScript.java @@ -13,7 +13,7 @@ /** * A script used in bucket aggregations that returns a {@code boolean} value. */ -public abstract class BucketAggregationSelectorScript { +public abstract class BucketAggregationSelectorScript { // TODO move to the aggregation module public static final String[] PARAMETERS = {}; diff --git a/server/src/main/java/org/elasticsearch/search/SearchModule.java b/server/src/main/java/org/elasticsearch/search/SearchModule.java index 42c20def183d8..024f9fa301310 100644 --- a/server/src/main/java/org/elasticsearch/search/SearchModule.java +++ b/server/src/main/java/org/elasticsearch/search/SearchModule.java @@ -189,7 +189,6 @@ import org.elasticsearch.search.aggregations.metrics.WeightedAvgAggregationBuilder; import org.elasticsearch.search.aggregations.pipeline.AvgBucketPipelineAggregationBuilder; import org.elasticsearch.search.aggregations.pipeline.BucketScriptPipelineAggregationBuilder; -import org.elasticsearch.search.aggregations.pipeline.BucketSelectorPipelineAggregationBuilder; import org.elasticsearch.search.aggregations.pipeline.CumulativeSumPipelineAggregationBuilder; import org.elasticsearch.search.aggregations.pipeline.ExtendedStatsBucketParser; import org.elasticsearch.search.aggregations.pipeline.ExtendedStatsBucketPipelineAggregationBuilder; @@ -759,13 +758,6 @@ private void registerPipelineAggregations(List plugins) { BucketScriptPipelineAggregationBuilder.PARSER ) ); - registerPipelineAggregation( - new PipelineAggregationSpec( - BucketSelectorPipelineAggregationBuilder.NAME, - BucketSelectorPipelineAggregationBuilder::new, - BucketSelectorPipelineAggregationBuilder::parse - ) - ); registerPipelineAggregation( new PipelineAggregationSpec( SerialDiffPipelineAggregationBuilder.NAME, diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/PipelineAggregatorBuilders.java b/server/src/main/java/org/elasticsearch/search/aggregations/PipelineAggregatorBuilders.java index 3108739c8b071..f280eb4de61bb 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/PipelineAggregatorBuilders.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/PipelineAggregatorBuilders.java @@ -11,7 +11,6 @@ import org.elasticsearch.script.Script; import org.elasticsearch.search.aggregations.pipeline.AvgBucketPipelineAggregationBuilder; import org.elasticsearch.search.aggregations.pipeline.BucketScriptPipelineAggregationBuilder; -import org.elasticsearch.search.aggregations.pipeline.BucketSelectorPipelineAggregationBuilder; import org.elasticsearch.search.aggregations.pipeline.CumulativeSumPipelineAggregationBuilder; import org.elasticsearch.search.aggregations.pipeline.ExtendedStatsBucketPipelineAggregationBuilder; import org.elasticsearch.search.aggregations.pipeline.MaxBucketPipelineAggregationBuilder; @@ -63,14 +62,6 @@ public static BucketScriptPipelineAggregationBuilder bucketScript(String name, S return new BucketScriptPipelineAggregationBuilder(name, script, bucketsPaths); } - public static BucketSelectorPipelineAggregationBuilder bucketSelector(String name, Map bucketsPathsMap, Script script) { - return new BucketSelectorPipelineAggregationBuilder(name, bucketsPathsMap, script); - } - - public static BucketSelectorPipelineAggregationBuilder bucketSelector(String name, Script script, String... bucketsPaths) { - return new BucketSelectorPipelineAggregationBuilder(name, script, bucketsPaths); - } - public static CumulativeSumPipelineAggregationBuilder cumulativeSum(String name, String bucketsPath) { return new CumulativeSumPipelineAggregationBuilder(name, bucketsPath); } diff --git a/test/framework/src/main/java/org/elasticsearch/script/MockScriptEngine.java b/test/framework/src/main/java/org/elasticsearch/script/MockScriptEngine.java index ea02011459784..c475be4c6f7fc 100644 --- a/test/framework/src/main/java/org/elasticsearch/script/MockScriptEngine.java +++ b/test/framework/src/main/java/org/elasticsearch/script/MockScriptEngine.java @@ -217,14 +217,6 @@ public Double execute() { } }; return context.factoryClazz.cast(factory); - } else if (context.instanceClazz.equals(BucketAggregationSelectorScript.class)) { - BucketAggregationSelectorScript.Factory factory = parameters -> new BucketAggregationSelectorScript(parameters) { - @Override - public boolean execute() { - return (boolean) script.apply(getParams()); - } - }; - return context.factoryClazz.cast(factory); } else if (context.instanceClazz.equals(SignificantTermsHeuristicScoreScript.class)) { return context.factoryClazz.cast(new MockSignificantTermsHeuristicScoreScript(script)); } else if (context.instanceClazz.equals(TemplateScript.class)) { @@ -356,7 +348,6 @@ public Set> getSupportedContexts() { IngestConditionalScript.CONTEXT, UpdateScript.CONTEXT, BucketAggregationScript.CONTEXT, - BucketAggregationSelectorScript.CONTEXT, SignificantTermsHeuristicScoreScript.CONTEXT, TemplateScript.CONTEXT, FilterScript.CONTEXT, diff --git a/x-pack/plugin/sql/build.gradle b/x-pack/plugin/sql/build.gradle index 7b31b4047b4cf..2cc09acabeec4 100644 --- a/x-pack/plugin/sql/build.gradle +++ b/x-pack/plugin/sql/build.gradle @@ -32,6 +32,7 @@ dependencies { compileOnly project(path: xpackModule('core')) compileOnly(project(':modules:lang-painless:spi')) api project('sql-action') + api project(':modules:aggregations') api project(':modules:aggs-matrix-stats') compileOnly project(path: xpackModule('ql')) testImplementation project(':test:framework') diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/CompositeAggCursor.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/CompositeAggCursor.java index 009c845c3d943..a08b3a9ca8180 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/CompositeAggCursor.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/CompositeAggCursor.java @@ -11,6 +11,7 @@ import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.search.SearchRequest; import org.elasticsearch.action.search.SearchResponse; +import org.elasticsearch.aggregations.pipeline.BucketSelectorPipelineAggregationBuilder; import org.elasticsearch.client.internal.Client; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; @@ -18,7 +19,6 @@ import org.elasticsearch.search.aggregations.AggregationBuilder; import org.elasticsearch.search.aggregations.bucket.composite.CompositeAggregation; import org.elasticsearch.search.aggregations.bucket.composite.CompositeAggregationBuilder; -import org.elasticsearch.search.aggregations.pipeline.BucketSelectorPipelineAggregationBuilder; import org.elasticsearch.search.builder.SearchSourceBuilder; import org.elasticsearch.xpack.ql.execution.search.extractor.BucketExtractor; import org.elasticsearch.xpack.ql.util.StringUtils; diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/agg/AggFilter.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/agg/AggFilter.java index 56851bbac59f5..a6677a6929764 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/agg/AggFilter.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/agg/AggFilter.java @@ -6,6 +6,7 @@ */ package org.elasticsearch.xpack.sql.querydsl.agg; +import org.elasticsearch.aggregations.pipeline.BucketSelectorPipelineAggregationBuilder; import org.elasticsearch.script.Script; import org.elasticsearch.search.aggregations.PipelineAggregationBuilder; import org.elasticsearch.xpack.ql.expression.gen.script.ScriptTemplate; @@ -15,8 +16,6 @@ import java.util.Map; import java.util.Objects; -import static org.elasticsearch.search.aggregations.PipelineAggregatorBuilders.bucketSelector; - public class AggFilter extends PipelineAgg { private static final String BUCKET_SELECTOR_ID_PREFIX = "having."; @@ -39,7 +38,7 @@ public ScriptTemplate scriptTemplate() { @Override PipelineAggregationBuilder toBuilder() { Script script = scriptTemplate.toPainless(); - return bucketSelector(name(), aggPaths, script); + return new BucketSelectorPipelineAggregationBuilder(name(), aggPaths, script); } @Override From c8c7b50acf62ef43c8dcfaa8d7ce3e9fea306cfd Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Mon, 7 Nov 2022 13:56:22 -0500 Subject: [PATCH 2/5] Fix pivot test --- x-pack/plugin/transform/build.gradle | 1 + .../xpack/transform/transforms/pivot/PivotTests.java | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/transform/build.gradle b/x-pack/plugin/transform/build.gradle index 6e98c4e659f81..7115cb0c34506 100644 --- a/x-pack/plugin/transform/build.gradle +++ b/x-pack/plugin/transform/build.gradle @@ -14,6 +14,7 @@ dependencies { testImplementation(testArtifact(project(xpackModule('core')))) testImplementation project(path: xpackModule('analytics')) testImplementation project(path: ':modules:aggs-matrix-stats') + testImplementation project(path: ':modules:aggregations') testImplementation project(path: xpackModule('spatial')) testImplementation project(":test:x-content") diff --git a/x-pack/plugin/transform/src/test/java/org/elasticsearch/xpack/transform/transforms/pivot/PivotTests.java b/x-pack/plugin/transform/src/test/java/org/elasticsearch/xpack/transform/transforms/pivot/PivotTests.java index 762aa256b609e..0756b4ea61849 100644 --- a/x-pack/plugin/transform/src/test/java/org/elasticsearch/xpack/transform/transforms/pivot/PivotTests.java +++ b/x-pack/plugin/transform/src/test/java/org/elasticsearch/xpack/transform/transforms/pivot/PivotTests.java @@ -17,6 +17,7 @@ import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.action.search.SearchResponseSections; import org.elasticsearch.action.search.ShardSearchFailure; +import org.elasticsearch.aggregations.AggregationsPlugin; import org.elasticsearch.client.internal.Client; import org.elasticsearch.common.ValidationException; import org.elasticsearch.common.settings.Settings; @@ -91,7 +92,7 @@ public class PivotTests extends ESTestCase { @Before public void registerAggregationNamedObjects() throws Exception { // register aggregations as NamedWriteable - SearchModule searchModule = new SearchModule(Settings.EMPTY, List.of(new TestSpatialPlugin())); + SearchModule searchModule = new SearchModule(Settings.EMPTY, List.of(new TestSpatialPlugin(), new AggregationsPlugin())); namedXContentRegistry = new NamedXContentRegistry(searchModule.getNamedXContents()); } From 8072fcf184d99504a92db03777fe31e031bb24e7 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Tue, 8 Nov 2022 08:54:24 -0500 Subject: [PATCH 3/5] Didn't move context --- .../java/org/elasticsearch/script/MockScriptEngine.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/framework/src/main/java/org/elasticsearch/script/MockScriptEngine.java b/test/framework/src/main/java/org/elasticsearch/script/MockScriptEngine.java index c475be4c6f7fc..ea02011459784 100644 --- a/test/framework/src/main/java/org/elasticsearch/script/MockScriptEngine.java +++ b/test/framework/src/main/java/org/elasticsearch/script/MockScriptEngine.java @@ -217,6 +217,14 @@ public Double execute() { } }; return context.factoryClazz.cast(factory); + } else if (context.instanceClazz.equals(BucketAggregationSelectorScript.class)) { + BucketAggregationSelectorScript.Factory factory = parameters -> new BucketAggregationSelectorScript(parameters) { + @Override + public boolean execute() { + return (boolean) script.apply(getParams()); + } + }; + return context.factoryClazz.cast(factory); } else if (context.instanceClazz.equals(SignificantTermsHeuristicScoreScript.class)) { return context.factoryClazz.cast(new MockSignificantTermsHeuristicScoreScript(script)); } else if (context.instanceClazz.equals(TemplateScript.class)) { @@ -348,6 +356,7 @@ public Set> getSupportedContexts() { IngestConditionalScript.CONTEXT, UpdateScript.CONTEXT, BucketAggregationScript.CONTEXT, + BucketAggregationSelectorScript.CONTEXT, SignificantTermsHeuristicScoreScript.CONTEXT, TemplateScript.CONTEXT, FilterScript.CONTEXT, From b72d164580266ecb1cdbb2044b6ba671f4de6a20 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Tue, 8 Nov 2022 13:37:49 -0500 Subject: [PATCH 4/5] Fix? --- .../xpack/sql/execution/search/CompositeAggCursor.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/CompositeAggCursor.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/CompositeAggCursor.java index a08b3a9ca8180..74b9f5b89ca6e 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/CompositeAggCursor.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/CompositeAggCursor.java @@ -219,7 +219,8 @@ static CompositeAggregationBuilder getCompositeBuilder(SearchSourceBuilder sourc static boolean couldProducePartialPages(CompositeAggregationBuilder aggregation) { for (var agg : aggregation.getPipelineAggregations()) { - if (agg instanceof BucketSelectorPipelineAggregationBuilder) { + // Use type.equals because there are two copies of BucketSelectorPipelineAggregationBuilder on the classpath + if (agg.getType().equals(BucketSelectorPipelineAggregationBuilder.NAME)) { return true; } } From 882c4df83e0ad4ad1373d5ca2ac438601bc7ca22 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Thu, 10 Nov 2022 13:52:01 -0500 Subject: [PATCH 5/5] fmt --- .../java/org/elasticsearch/aggregations/AggregationsPlugin.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/aggregations/src/main/java/org/elasticsearch/aggregations/AggregationsPlugin.java b/modules/aggregations/src/main/java/org/elasticsearch/aggregations/AggregationsPlugin.java index d0f93eef5746c..c46f4b444dcf4 100644 --- a/modules/aggregations/src/main/java/org/elasticsearch/aggregations/AggregationsPlugin.java +++ b/modules/aggregations/src/main/java/org/elasticsearch/aggregations/AggregationsPlugin.java @@ -12,9 +12,9 @@ import org.elasticsearch.aggregations.bucket.adjacency.InternalAdjacencyMatrix; import org.elasticsearch.aggregations.bucket.histogram.AutoDateHistogramAggregationBuilder; import org.elasticsearch.aggregations.bucket.histogram.InternalAutoDateHistogram; -import org.elasticsearch.aggregations.pipeline.BucketSelectorPipelineAggregationBuilder; import org.elasticsearch.aggregations.bucket.timeseries.InternalTimeSeries; import org.elasticsearch.aggregations.bucket.timeseries.TimeSeriesAggregationBuilder; +import org.elasticsearch.aggregations.pipeline.BucketSelectorPipelineAggregationBuilder; import org.elasticsearch.aggregations.pipeline.BucketSortPipelineAggregationBuilder; import org.elasticsearch.aggregations.pipeline.Derivative; import org.elasticsearch.aggregations.pipeline.DerivativePipelineAggregationBuilder;