Clarify the error message when a pipeline agg is used in the 'order' …

…parameter. (#32522)
elastic · Aug 1, 2018 · 5efc2ec · 5efc2ec
1 parent 16fe220
commit 5efc2ec
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 3 deletions.
diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/support/AggregationPath.java b/server/src/main/java/org/elasticsearch/search/aggregations/support/AggregationPath.java
@@ -288,11 +288,14 @@ public Aggregator resolveTopmostAggregator(Aggregator root) {
     public void validate(Aggregator root) throws AggregationExecutionException {
         Aggregator aggregator = root;
         for (int i = 0; i < pathElements.size(); i++) {
-            aggregator = ProfilingAggregator.unwrap(aggregator.subAggregator(pathElements.get(i).name));
+            String name = pathElements.get(i).name;
+            aggregator = ProfilingAggregator.unwrap(aggregator.subAggregator(name));
             if (aggregator == null) {
-                throw new AggregationExecutionException("Invalid aggregator order path [" + this + "]. Unknown aggregation ["
-                        + pathElements.get(i).name + "]");
+                throw new AggregationExecutionException("Invalid aggregator order path [" + this + "]. The " +
+                    "provided aggregation [" + name + "] either does not exist, or is a pipeline aggregation " +
+                    "and cannot be used to sort the buckets.");
             }
+
             if (i < pathElements.size() - 1) {
 
                 // we're in the middle of the path, so the aggregator can only be a single-bucket aggregator

diff --git a/...rc/test/java/org/elasticsearch/search/aggregations/bucket/terms/TermsAggregatorTests.java b/...rc/test/java/org/elasticsearch/search/aggregations/bucket/terms/TermsAggregatorTests.java
@@ -50,9 +50,11 @@
 import org.elasticsearch.index.mapper.Uid;
 import org.elasticsearch.index.query.QueryBuilders;
 import org.elasticsearch.indices.breaker.NoneCircuitBreakerService;
+import org.elasticsearch.script.Script;
 import org.elasticsearch.search.SearchHit;
 import org.elasticsearch.search.aggregations.AggregationBuilder;
 import org.elasticsearch.search.aggregations.AggregationBuilders;
+import org.elasticsearch.search.aggregations.AggregationExecutionException;
 import org.elasticsearch.search.aggregations.Aggregator;
 import org.elasticsearch.search.aggregations.AggregatorTestCase;
 import org.elasticsearch.search.aggregations.BucketOrder;
@@ -67,6 +69,7 @@
 import org.elasticsearch.search.aggregations.bucket.nested.NestedAggregationBuilder;
 import org.elasticsearch.search.aggregations.metrics.tophits.InternalTopHits;
 import org.elasticsearch.search.aggregations.metrics.tophits.TopHitsAggregationBuilder;
+import org.elasticsearch.search.aggregations.pipeline.bucketscript.BucketScriptPipelineAggregationBuilder;
 import org.elasticsearch.search.aggregations.support.ValueType;
 import org.elasticsearch.search.sort.FieldSortBuilder;
 import org.elasticsearch.search.sort.ScoreSortBuilder;
@@ -83,6 +86,8 @@
 import java.util.function.Function;
 
 import static org.elasticsearch.index.mapper.SeqNoFieldMapper.PRIMARY_TERM_NAME;
+import static org.elasticsearch.search.aggregations.AggregationBuilders.terms;
+import static org.elasticsearch.search.aggregations.pipeline.PipelineAggregatorBuilders.bucketScript;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.greaterThan;
 import static org.hamcrest.Matchers.instanceOf;
@@ -1060,6 +1065,34 @@ public void testWithNestedAggregations() throws IOException {
         }
     }
 
+    public void testOrderByPipelineAggregation() throws Exception {
+        try (Directory directory = newDirectory()) {
+            try (RandomIndexWriter indexWriter = new RandomIndexWriter(random(), directory)) {
+                try (IndexReader indexReader = maybeWrapReaderEs(indexWriter.getReader())) {
+                    IndexSearcher indexSearcher = newIndexSearcher(indexReader);
+
+                    BucketScriptPipelineAggregationBuilder bucketScriptAgg = bucketScript(
+                        "script", new Script("2.718"));
+                    TermsAggregationBuilder termsAgg = terms("terms")
+                        .field("field")
+                        .valueType(ValueType.STRING)
+                        .order(BucketOrder.aggregation("script", true))
+                        .subAggregation(bucketScriptAgg);
+
+                    MappedFieldType fieldType = new KeywordFieldMapper.KeywordFieldType();
+                    fieldType.setName("field");
+                    fieldType.setHasDocValues(true);
+
+                    AggregationExecutionException e = expectThrows(AggregationExecutionException.class,
+                        () -> createAggregator(termsAgg, indexSearcher, fieldType));
+                    assertEquals("Invalid aggregator order path [script]. The provided aggregation [script] " +
+                        "either does not exist, or is a pipeline aggregation and cannot be used to sort the buckets.",
+                        e.getMessage());
+                }
+            }
+        }
+    }
+
     private final SeqNoFieldMapper.SequenceIDFields sequenceIDFields = SeqNoFieldMapper.SequenceIDFields.emptySeqID();
     private List<Document> generateDocsWithNested(String id, int value, int[] nestedValues) {
         List<Document> documents = new ArrayList<>();