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
This commit is contained in:
Marios Trivyzas 2018-10-23 11:11:22 +02:00 committed by GitHub
parent 0536635c44
commit 1eb76f16b1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 40 additions and 12 deletions

View File

@ -8,6 +8,7 @@ package org.elasticsearch.xpack.sql.expression;
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 java.util.function.Predicate;
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 abstract class Expressions {
}
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 + "')";
}
}

View File

@ -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 class Max extends NumericAggregate implements EnclosedAgg {
public String innerName() {
return "max";
}
@Override
protected TypeResolution resolveType() {
return Expressions.typeMustBeNumericOrDate(field());
}
}

View File

@ -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 class Min extends NumericAggregate implements EnclosedAgg {
public String innerName() {
return "min";
}
@Override
protected TypeResolution resolveType() {
return Expressions.typeMustBeNumericOrDate(field());
}
}

View File

@ -125,6 +125,11 @@ public class VerifierErrorMessagesTests extends ESTestCase {
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 class VerifierErrorMessagesTests extends ESTestCase {
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)"));
}
}
}

View File

@ -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