Browse Source

SQL: Improve verifier errors on nested aggregations (#75517)

* SQL: verify nested aggregations

* address comments

* address comments
Lukas Wegmann 4 years ago
parent
commit
65aace97c5

+ 12 - 0
x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/Verifier.java

@@ -214,6 +214,8 @@ public final class Verifier {
                 checkFilterOnAggs(p, localFailures, attributeRefs);
                 checkFilterOnGrouping(p, localFailures, attributeRefs);
 
+                checkNestedAggregation(p, localFailures, attributeRefs);
+
                 if (groupingFailures.contains(p) == false) {
                     checkGroupBy(p, localFailures, attributeRefs, groupingFailures);
                 }
@@ -266,6 +268,16 @@ public final class Verifier {
         return failures;
     }
 
+    private void checkNestedAggregation(LogicalPlan p, Set<Failure> localFailures, AttributeMap<Expression> attributeRefs) {
+        if (p instanceof Aggregate) {
+            ((Aggregate) p).child()
+                .forEachDown(
+                    Aggregate.class,
+                    a -> { localFailures.add(fail(a, "Nested aggregations in sub-selects are not supported.")); }
+                );
+        }
+    }
+
     private void checkFullTextSearchInSelect(LogicalPlan plan, Set<Failure> localFailures) {
         plan.forEachUp(Project.class, p -> {
             for (NamedExpression ne : p.projections()) {

+ 15 - 0
x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/VerifierErrorMessagesTests.java

@@ -32,6 +32,7 @@ import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import java.util.function.Consumer;
 
 import static java.util.Collections.emptyMap;
 import static java.util.Collections.singletonMap;
@@ -1360,6 +1361,20 @@ public class VerifierErrorMessagesTests extends ESTestCase {
         accept("SELECT * FROM (SELECT bool as b, AVG(int) as a FROM test GROUP BY bool ORDER BY bool) WHERE a > 10");
     }
 
+    public void testNestedAggregate() {
+        Consumer<String> checkMsg = (String sql) -> {
+            var actual = error(sql);
+            assertTrue(actual, actual.contains("Nested aggregations in sub-selects are not supported."));
+        };
+
+        checkMsg.accept("SELECT SUM(c) FROM (SELECT COUNT(*) c FROM test)");
+        checkMsg.accept("SELECT COUNT(*) FROM (SELECT SUM(int) c FROM test)");
+        checkMsg.accept("SELECT i FROM (SELECT int i FROM test GROUP BY i) GROUP BY i");
+        checkMsg.accept("SELECT c FROM (SELECT SUM(int) c FROM test) GROUP BY c HAVING COUNT(*) > 10");
+        checkMsg.accept("SELECT COUNT(*) FROM (SELECT int i FROM test GROUP BY i)");
+        checkMsg.accept("SELECT a.i, COUNT(a.c) FROM (SELECT int i, COUNT(int) c FROM test GROUP BY int) a GROUP BY c");
+    }
+
     private String randomTopHitsFunction() {
         return randomFrom(Arrays.asList(First.class, Last.class)).getSimpleName().toUpperCase(Locale.ROOT);
     }