SQL: Allow min/max aggregates on date fields (#34699)

Allow `MIN()` and `MAX()` aggregate functions to operate also on arguments of type DATE apart from the numeric ones. Fixes: #34477
elastic · Oct 23, 2018 · 7e6b914 · 7e6b914
1 parent 9ce0acf
commit 7e6b914
Show file tree

Hide file tree

Showing 5 changed files with 40 additions and 12 deletions.
diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Expressions.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Expressions.java
@@ -8,6 +8,7 @@
 import org.elasticsearch.xpack.sql.SqlIllegalArgumentException;
 import org.elasticsearch.xpack.sql.expression.Expression.TypeResolution;
 import org.elasticsearch.xpack.sql.expression.gen.pipeline.Pipe;
+import org.elasticsearch.xpack.sql.type.DataType;
 
 import java.util.ArrayList;
 import java.util.Collection;
@@ -16,15 +17,10 @@
 
 import static java.util.Collections.emptyList;
 import static java.util.Collections.emptyMap;
-import static java.util.stream.Collectors.toList;
 
-public abstract class Expressions {
+public final class Expressions {
 
-    public static List<NamedExpression> asNamed(List<? extends Expression> exp) {
-        return exp.stream()
-                .map(NamedExpression.class::cast)
-                .collect(toList());
-    }
+    private Expressions() {}
 
     public static NamedExpression wrapAsNamed(Expression exp) {
         return exp instanceof NamedExpression ? (NamedExpression) exp : new Alias(exp.location(), exp.nodeName(), exp);
@@ -126,7 +122,16 @@ public static TypeResolution typeMustBe(Expression e, Predicate<Expression> pred
     }
 
     public static TypeResolution typeMustBeNumeric(Expression e) {
-        return e.dataType().isNumeric()? TypeResolution.TYPE_RESOLVED : new TypeResolution(
-                "Argument required to be numeric ('" + Expressions.name(e) + "' of type '" + e.dataType().esType + "')");
+        return e.dataType().isNumeric() ? TypeResolution.TYPE_RESOLVED : new TypeResolution(numericErrorMessage(e));
+    }
+
+    public static TypeResolution typeMustBeNumericOrDate(Expression e) {
+        return e.dataType().isNumeric() || e.dataType() == DataType.DATE ?
+            TypeResolution.TYPE_RESOLVED :
+            new TypeResolution(numericErrorMessage(e));
+    }
+
+    private static String numericErrorMessage(Expression e) {
+        return "Argument required to be numeric ('" + Expressions.name(e) + "' of type '" + e.dataType().esType + "')";
     }
 }
diff --git a/...ugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/Max.java b/...ugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/Max.java
@@ -5,12 +5,14 @@
  */
 package org.elasticsearch.xpack.sql.expression.function.aggregate;
 
-import java.util.List;
 import org.elasticsearch.xpack.sql.expression.Expression;
+import org.elasticsearch.xpack.sql.expression.Expressions;
 import org.elasticsearch.xpack.sql.tree.Location;
 import org.elasticsearch.xpack.sql.tree.NodeInfo;
 import org.elasticsearch.xpack.sql.type.DataType;
 
+import java.util.List;
+
 /**
  * Find the maximum value in matching documents.
  */
@@ -39,4 +41,9 @@ public DataType dataType() {
     public String innerName() {
         return "max";
     }
+
+    @Override
+    protected TypeResolution resolveType() {
+        return Expressions.typeMustBeNumericOrDate(field());
+    }
 }
diff --git a/...ugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/Min.java b/...ugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/Min.java
@@ -5,12 +5,14 @@
  */
 package org.elasticsearch.xpack.sql.expression.function.aggregate;
 
-import java.util.List;
 import org.elasticsearch.xpack.sql.expression.Expression;
+import org.elasticsearch.xpack.sql.expression.Expressions;
 import org.elasticsearch.xpack.sql.tree.Location;
 import org.elasticsearch.xpack.sql.tree.NodeInfo;
 import org.elasticsearch.xpack.sql.type.DataType;
 
+import java.util.List;
+
 /**
  * Find the minimum value in matched documents.
  */
@@ -42,4 +44,9 @@ public DataType dataType() {
     public String innerName() {
         return "min";
     }
+
+    @Override
+    protected TypeResolution resolveType() {
+        return Expressions.typeMustBeNumericOrDate(field());
+    }
 }
diff --git a/...c/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/VerifierErrorMessagesTests.java b/...c/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/VerifierErrorMessagesTests.java
@@ -125,6 +125,11 @@ public void testGroupByAggregate() {
                 verify("SELECT AVG(int) FROM test GROUP BY AVG(int)"));
     }
 
+    public void testNotSupportedAggregateOnDate() {
+        assertEquals("1:8: Argument required to be numeric ('date' of type 'date')",
+            verify("SELECT AVG(date) FROM test"));
+    }
+
     public void testGroupByOnNested() {
         assertEquals("1:38: Grouping isn't (yet) compatible with nested fields [dep.dep_id]",
                 verify("SELECT dep.dep_id FROM test GROUP BY dep.dep_id"));
@@ -169,4 +174,4 @@ public void testHavingOnScalar() {
         assertEquals("1:42: Cannot filter HAVING on non-aggregate [int]; consider using WHERE instead",
                 verify("SELECT int FROM test GROUP BY int HAVING 2 < ABS(int)"));
     }
-}
+}
diff --git a/x-pack/qa/sql/src/main/resources/agg.sql-spec b/x-pack/qa/sql/src/main/resources/agg.sql-spec
@@ -216,6 +216,8 @@ aggMinWithCastAndFilter
 SELECT gender g, CAST(MIN(emp_no) AS SMALLINT) m, COUNT(1) c FROM "test_emp" WHERE emp_no < 10020 GROUP BY gender ORDER BY gender;
 aggMinWithAlias
 SELECT gender g, MIN(emp_no) m FROM "test_emp" GROUP BY g ORDER BY gender;
+aggMinOnDate
+SELECT gender, MIN(birth_date) m FROM "test_emp" GROUP BY gender ORDER BY gender;
 
 // Conditional MIN
 aggMinWithHaving
@@ -270,6 +272,8 @@ aggMaxAndCountWithFilterAndLimit
 SELECT gender g, MAX(emp_no) m, COUNT(1) c FROM "test_emp" WHERE emp_no > 10000 GROUP BY gender ORDER BY gender LIMIT 1;
 aggMaxWithAlias
 SELECT gender g, MAX(emp_no) m FROM "test_emp" GROUP BY g ORDER BY gender;
+aggMaxOnDate
+SELECT gender, MAX(birth_date) m FROM "test_emp" GROUP BY gender ORDER BY gender;
 
 // Conditional MAX
 aggMaxWithHaving