Browse Source

Support for aggregation names with dots in first element path of a pipeline aggregation (#77481)

This commit changes the strategy during validation is it uses AggregationPath to resolve the name.
Ignacio Vera 4 years ago
parent
commit
3c6e7a0389

+ 56 - 41
server/src/internalClusterTest/java/org/elasticsearch/search/aggregations/pipeline/AvgBucketIT.java

@@ -45,6 +45,9 @@ public class AvgBucketIT extends ESIntegTestCase {
     static int numValueBuckets;
     static long[] valueCounts;
 
+    static String histoName;
+    static String termsName;
+
     @Override
     public void setupSuiteScopeCluster() throws Exception {
         assertAcked(client().admin().indices().prepareCreate("idx").setMapping("tag", "type=keyword").get());
@@ -86,21 +89,29 @@ public class AvgBucketIT extends ESIntegTestCase {
         }
         indexRandom(true, builders);
         ensureSearchable();
+        histoName = randomName();
+        termsName = randomName();
+    }
+
+    private static String randomName() {
+        return randomBoolean()
+            ? randomAlphaOfLengthBetween(3, 12)
+            : randomAlphaOfLengthBetween(3, 6) + "." + randomAlphaOfLengthBetween(3, 6);
     }
 
     public void testDocCountTopLevel() throws Exception {
         SearchResponse response = client().prepareSearch("idx")
             .addAggregation(
-                histogram("histo").field(SINGLE_VALUED_FIELD_NAME).interval(interval).extendedBounds(minRandomValue, maxRandomValue)
+                histogram(histoName).field(SINGLE_VALUED_FIELD_NAME).interval(interval).extendedBounds(minRandomValue, maxRandomValue)
             )
-            .addAggregation(avgBucket("avg_bucket", "histo>_count"))
+            .addAggregation(avgBucket("avg_bucket", histoName + ">_count"))
             .get();
 
         assertSearchResponse(response);
 
-        Histogram histo = response.getAggregations().get("histo");
+        Histogram histo = response.getAggregations().get(histoName);
         assertThat(histo, notNullValue());
-        assertThat(histo.getName(), equalTo("histo"));
+        assertThat(histo.getName(), equalTo(histoName));
         List<? extends Bucket> buckets = histo.getBuckets();
         assertThat(buckets.size(), equalTo(numValueBuckets));
 
@@ -125,20 +136,22 @@ public class AvgBucketIT extends ESIntegTestCase {
     public void testDocCountAsSubAgg() throws Exception {
         SearchResponse response = client().prepareSearch("idx")
             .addAggregation(
-                terms("terms").field("tag")
+                terms(termsName).field("tag")
                     .order(BucketOrder.key(true))
                     .subAggregation(
-                        histogram("histo").field(SINGLE_VALUED_FIELD_NAME).interval(interval).extendedBounds(minRandomValue, maxRandomValue)
+                        histogram(histoName).field(SINGLE_VALUED_FIELD_NAME)
+                            .interval(interval)
+                            .extendedBounds(minRandomValue, maxRandomValue)
                     )
-                    .subAggregation(avgBucket("avg_bucket", "histo>_count"))
+                    .subAggregation(avgBucket("avg_bucket", histoName + ">_count"))
             )
             .get();
 
         assertSearchResponse(response);
 
-        Terms terms = response.getAggregations().get("terms");
+        Terms terms = response.getAggregations().get(termsName);
         assertThat(terms, notNullValue());
-        assertThat(terms.getName(), equalTo("terms"));
+        assertThat(terms.getName(), equalTo(termsName));
         List<? extends Terms.Bucket> termsBuckets = terms.getBuckets();
         assertThat(termsBuckets.size(), equalTo(interval));
 
@@ -147,9 +160,9 @@ public class AvgBucketIT extends ESIntegTestCase {
             assertThat(termsBucket, notNullValue());
             assertThat((String) termsBucket.getKey(), equalTo("tag" + (i % interval)));
 
-            Histogram histo = termsBucket.getAggregations().get("histo");
+            Histogram histo = termsBucket.getAggregations().get(histoName);
             assertThat(histo, notNullValue());
-            assertThat(histo.getName(), equalTo("histo"));
+            assertThat(histo.getName(), equalTo(histoName));
             List<? extends Bucket> buckets = histo.getBuckets();
 
             double sum = 0;
@@ -172,15 +185,15 @@ public class AvgBucketIT extends ESIntegTestCase {
 
     public void testMetricTopLevel() throws Exception {
         SearchResponse response = client().prepareSearch("idx")
-            .addAggregation(terms("terms").field("tag").subAggregation(sum("sum").field(SINGLE_VALUED_FIELD_NAME)))
-            .addAggregation(avgBucket("avg_bucket", "terms>sum"))
+            .addAggregation(terms(termsName).field("tag").subAggregation(sum("sum").field(SINGLE_VALUED_FIELD_NAME)))
+            .addAggregation(avgBucket("avg_bucket", termsName + ">sum"))
             .get();
 
         assertSearchResponse(response);
 
-        Terms terms = response.getAggregations().get("terms");
+        Terms terms = response.getAggregations().get(termsName);
         assertThat(terms, notNullValue());
-        assertThat(terms.getName(), equalTo("terms"));
+        assertThat(terms.getName(), equalTo(termsName));
         List<? extends Terms.Bucket> buckets = terms.getBuckets();
         assertThat(buckets.size(), equalTo(interval));
 
@@ -207,23 +220,23 @@ public class AvgBucketIT extends ESIntegTestCase {
     public void testMetricAsSubAgg() throws Exception {
         SearchResponse response = client().prepareSearch("idx")
             .addAggregation(
-                terms("terms").field("tag")
+                terms(termsName).field("tag")
                     .order(BucketOrder.key(true))
                     .subAggregation(
-                        histogram("histo").field(SINGLE_VALUED_FIELD_NAME)
+                        histogram(histoName).field(SINGLE_VALUED_FIELD_NAME)
                             .interval(interval)
                             .extendedBounds(minRandomValue, maxRandomValue)
                             .subAggregation(sum("sum").field(SINGLE_VALUED_FIELD_NAME))
                     )
-                    .subAggregation(avgBucket("avg_bucket", "histo>sum"))
+                    .subAggregation(avgBucket("avg_bucket", histoName + ">sum"))
             )
             .get();
 
         assertSearchResponse(response);
 
-        Terms terms = response.getAggregations().get("terms");
+        Terms terms = response.getAggregations().get(termsName);
         assertThat(terms, notNullValue());
-        assertThat(terms.getName(), equalTo("terms"));
+        assertThat(terms.getName(), equalTo(termsName));
         List<? extends Terms.Bucket> termsBuckets = terms.getBuckets();
         assertThat(termsBuckets.size(), equalTo(interval));
 
@@ -232,9 +245,9 @@ public class AvgBucketIT extends ESIntegTestCase {
             assertThat(termsBucket, notNullValue());
             assertThat((String) termsBucket.getKey(), equalTo("tag" + (i % interval)));
 
-            Histogram histo = termsBucket.getAggregations().get("histo");
+            Histogram histo = termsBucket.getAggregations().get(histoName);
             assertThat(histo, notNullValue());
-            assertThat(histo.getName(), equalTo("histo"));
+            assertThat(histo.getName(), equalTo(histoName));
             List<? extends Bucket> buckets = histo.getBuckets();
 
             double bucketSum = 0;
@@ -262,23 +275,23 @@ public class AvgBucketIT extends ESIntegTestCase {
     public void testMetricAsSubAggWithInsertZeros() throws Exception {
         SearchResponse response = client().prepareSearch("idx")
             .addAggregation(
-                terms("terms").field("tag")
+                terms(termsName).field("tag")
                     .order(BucketOrder.key(true))
                     .subAggregation(
-                        histogram("histo").field(SINGLE_VALUED_FIELD_NAME)
+                        histogram(histoName).field(SINGLE_VALUED_FIELD_NAME)
                             .interval(interval)
                             .extendedBounds(minRandomValue, maxRandomValue)
                             .subAggregation(sum("sum").field(SINGLE_VALUED_FIELD_NAME))
                     )
-                    .subAggregation(avgBucket("avg_bucket", "histo>sum").gapPolicy(GapPolicy.INSERT_ZEROS))
+                    .subAggregation(avgBucket("avg_bucket", histoName + ">sum").gapPolicy(GapPolicy.INSERT_ZEROS))
             )
             .get();
 
         assertSearchResponse(response);
 
-        Terms terms = response.getAggregations().get("terms");
+        Terms terms = response.getAggregations().get(termsName);
         assertThat(terms, notNullValue());
-        assertThat(terms.getName(), equalTo("terms"));
+        assertThat(terms.getName(), equalTo(termsName));
         List<? extends Terms.Bucket> termsBuckets = terms.getBuckets();
         assertThat(termsBuckets.size(), equalTo(interval));
 
@@ -287,9 +300,9 @@ public class AvgBucketIT extends ESIntegTestCase {
             assertThat(termsBucket, notNullValue());
             assertThat((String) termsBucket.getKey(), equalTo("tag" + (i % interval)));
 
-            Histogram histo = termsBucket.getAggregations().get("histo");
+            Histogram histo = termsBucket.getAggregations().get(histoName);
             assertThat(histo, notNullValue());
-            assertThat(histo.getName(), equalTo("histo"));
+            assertThat(histo.getName(), equalTo(histoName));
             List<? extends Bucket> buckets = histo.getBuckets();
 
             double bucketSum = 0;
@@ -316,18 +329,18 @@ public class AvgBucketIT extends ESIntegTestCase {
     public void testNoBuckets() throws Exception {
         SearchResponse response = client().prepareSearch("idx")
             .addAggregation(
-                terms("terms").field("tag")
+                terms(termsName).field("tag")
                     .includeExclude(new IncludeExclude(null, "tag.*"))
                     .subAggregation(sum("sum").field(SINGLE_VALUED_FIELD_NAME))
             )
-            .addAggregation(avgBucket("avg_bucket", "terms>sum"))
+            .addAggregation(avgBucket("avg_bucket", termsName + ">sum"))
             .get();
 
         assertSearchResponse(response);
 
-        Terms terms = response.getAggregations().get("terms");
+        Terms terms = response.getAggregations().get(termsName);
         assertThat(terms, notNullValue());
-        assertThat(terms.getName(), equalTo("terms"));
+        assertThat(terms.getName(), equalTo(termsName));
         List<? extends Terms.Bucket> buckets = terms.getBuckets();
         assertThat(buckets.size(), equalTo(0));
 
@@ -340,21 +353,23 @@ public class AvgBucketIT extends ESIntegTestCase {
     public void testNested() throws Exception {
         SearchResponse response = client().prepareSearch("idx")
             .addAggregation(
-                terms("terms").field("tag")
+                terms(termsName).field("tag")
                     .order(BucketOrder.key(true))
                     .subAggregation(
-                        histogram("histo").field(SINGLE_VALUED_FIELD_NAME).interval(interval).extendedBounds(minRandomValue, maxRandomValue)
+                        histogram(histoName).field(SINGLE_VALUED_FIELD_NAME)
+                            .interval(interval)
+                            .extendedBounds(minRandomValue, maxRandomValue)
                     )
-                    .subAggregation(avgBucket("avg_histo_bucket", "histo>_count"))
+                    .subAggregation(avgBucket("avg_histo_bucket", histoName + ">_count"))
             )
-            .addAggregation(avgBucket("avg_terms_bucket", "terms>avg_histo_bucket"))
+            .addAggregation(avgBucket("avg_terms_bucket", termsName + ">avg_histo_bucket"))
             .get();
 
         assertSearchResponse(response);
 
-        Terms terms = response.getAggregations().get("terms");
+        Terms terms = response.getAggregations().get(termsName);
         assertThat(terms, notNullValue());
-        assertThat(terms.getName(), equalTo("terms"));
+        assertThat(terms.getName(), equalTo(termsName));
         List<? extends Terms.Bucket> termsBuckets = terms.getBuckets();
         assertThat(termsBuckets.size(), equalTo(interval));
 
@@ -365,9 +380,9 @@ public class AvgBucketIT extends ESIntegTestCase {
             assertThat(termsBucket, notNullValue());
             assertThat((String) termsBucket.getKey(), equalTo("tag" + (i % interval)));
 
-            Histogram histo = termsBucket.getAggregations().get("histo");
+            Histogram histo = termsBucket.getAggregations().get(histoName);
             assertThat(histo, notNullValue());
-            assertThat(histo.getName(), equalTo("histo"));
+            assertThat(histo.getName(), equalTo(histoName));
             List<? extends Bucket> buckets = histo.getBuckets();
 
             double aggHistoSum = 0;

+ 3 - 5
server/src/main/java/org/elasticsearch/search/aggregations/pipeline/BucketMetricsPipelineAggregationBuilder.java

@@ -14,6 +14,7 @@ import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.search.DocValueFormat;
 import org.elasticsearch.search.aggregations.AggregationBuilder;
 import org.elasticsearch.search.aggregations.pipeline.BucketHelpers.GapPolicy;
+import org.elasticsearch.search.aggregations.support.AggregationPath;
 
 import java.io.IOException;
 import java.util.Map;
@@ -103,11 +104,8 @@ public abstract class BucketMetricsPipelineAggregationBuilder<AF extends BucketM
             context.addBucketPathValidationError("must contain a single entry for aggregation [" + name + "]");
             return;
         }
-        // Need to find the first agg name in the buckets path to check its a
-        // multi bucket agg: aggs are split with '>' and can optionally have a
-        // metric name after them by using '.' so need to split on both to get
-        // just the agg name
-        final String firstAgg = bucketsPaths[0].split("[>\\.]")[0];
+        // find the first agg name in the buckets path to check its a multi bucket agg
+        final String firstAgg = AggregationPath.parse(bucketsPaths[0]).getPathElementsAsStringList().get(0);
         Optional<AggregationBuilder> aggBuilder = context.getSiblingAggregations()
             .stream()
             .filter(builder -> builder.getName().equals(firstAgg))