From a8e9272994522cc1c49208fdcd6d4707450027af Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Fri, 15 Dec 2017 14:33:58 -0700 Subject: [PATCH] SQL: Add remaining matrix aggregations (elastic/x-pack-elasticsearch#3330) * Add remaining matrix aggregations This adds the remaining [matrix aggregations](https://www.elastic.co/guide/en/elasticsearch/reference/6.1/search-aggregations-matrix-stats-aggregation.html). These aggregations aren't currently implemented due to the inter-plugin communication not being set up, so they return "innerkey/matrix stats not handled (yet)". For matrix aggs that share a name with an existing aggregation (like 'count'), they have be prefixed with "matrix_", so, "matrix_count", "matrix_mean", and "matrix_variance". Relates to elastic/x-pack-elasticsearch#2885 * Return HTTP 400 for innerkey/matrix stats aggs * Add integration test for unimplemented matrix aggs Original commit: elastic/x-pack-elasticsearch@34459c59aa4bc303f3d2623e8f70151cee28962c --- .../xpack/qa/sql/rest/RestSqlTestCase.java | 10 +++++++++ qa/sql/src/main/resources/command.csv-spec | 11 +++++++--- .../function/DefaultFunctionRegistry.java | 19 ++++++++++++----- .../function/aggregate/Correlation.java | 21 +++++++++++++++++++ .../function/aggregate/Covariance.java | 21 +++++++++++++++++++ .../function/aggregate/MatrixCount.java | 21 +++++++++++++++++++ .../function/aggregate/MatrixMean.java | 21 +++++++++++++++++++ .../function/aggregate/MatrixVariance.java | 21 +++++++++++++++++++ .../xpack/sql/planner/PlanningException.java | 16 ++++++++++++++ .../xpack/sql/planner/QueryFolder.java | 5 +++-- 10 files changed, 156 insertions(+), 10 deletions(-) create mode 100644 sql/server/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/Correlation.java create mode 100644 sql/server/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/Covariance.java create mode 100644 sql/server/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/MatrixCount.java create mode 100644 sql/server/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/MatrixMean.java create mode 100644 sql/server/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/MatrixVariance.java diff --git a/qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/rest/RestSqlTestCase.java b/qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/rest/RestSqlTestCase.java index 3f299c372cd..64aa45e2c8c 100644 --- a/qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/rest/RestSqlTestCase.java +++ b/qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/rest/RestSqlTestCase.java @@ -335,6 +335,16 @@ public abstract class RestSqlTestCase extends ESRestTestCase implements ErrorsTe assertEquals(0, getNumberOfSearchContexts("test")); } + public void testSelectUnimplementedMatrixAggs() throws IOException { + StringBuilder bulk = new StringBuilder(); + bulk.append("{\"index\":{\"_id\":\"1\"}}\n"); + bulk.append("{\"foo\":1}\n"); + client().performRequest("POST", "/test/doc/_bulk", singletonMap("refresh", "true"), + new StringEntity(bulk.toString(), ContentType.APPLICATION_JSON)); + expectBadRequest(() -> runSql("SELECT covariance(foo) FROM test"), containsString("innerkey/matrix stats not handled (yet)")); + } + + private Tuple runSqlAsText(String sql) throws IOException { return runSqlAsText("", new StringEntity("{\"query\":\"" + sql + "\"}", ContentType.APPLICATION_JSON)); } diff --git a/qa/sql/src/main/resources/command.csv-spec b/qa/sql/src/main/resources/command.csv-spec index 921828c1999..23c45baaa72 100644 --- a/qa/sql/src/main/resources/command.csv-spec +++ b/qa/sql/src/main/resources/command.csv-spec @@ -15,11 +15,16 @@ SUM |AGGREGATE MEAN |AGGREGATE STDDEV_POP |AGGREGATE VAR_POP |AGGREGATE -SUM_OF_SQUARES |AGGREGATE -SKEWNESS |AGGREGATE -KURTOSIS |AGGREGATE PERCENTILE |AGGREGATE PERCENTILE_RANK |AGGREGATE +SUM_OF_SQUARES |AGGREGATE +MATRIX_COUNT |AGGREGATE +MATRIX_MEAN |AGGREGATE +MATRIX_VARIANCE |AGGREGATE +SKEWNESS |AGGREGATE +KURTOSIS |AGGREGATE +COVARIANCE |AGGREGATE +CORRELATION |AGGREGATE DAY_OF_MONTH |SCALAR DAY |SCALAR DOM |SCALAR diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/expression/function/DefaultFunctionRegistry.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/expression/function/DefaultFunctionRegistry.java index bf81661af4e..13771b36256 100644 --- a/sql/server/src/main/java/org/elasticsearch/xpack/sql/expression/function/DefaultFunctionRegistry.java +++ b/sql/server/src/main/java/org/elasticsearch/xpack/sql/expression/function/DefaultFunctionRegistry.java @@ -7,8 +7,13 @@ package org.elasticsearch.xpack.sql.expression.function; import org.elasticsearch.xpack.sql.expression.function.aggregate.AggregateFunction; import org.elasticsearch.xpack.sql.expression.function.aggregate.Avg; +import org.elasticsearch.xpack.sql.expression.function.aggregate.Correlation; import org.elasticsearch.xpack.sql.expression.function.aggregate.Count; +import org.elasticsearch.xpack.sql.expression.function.aggregate.Covariance; import org.elasticsearch.xpack.sql.expression.function.aggregate.Kurtosis; +import org.elasticsearch.xpack.sql.expression.function.aggregate.MatrixCount; +import org.elasticsearch.xpack.sql.expression.function.aggregate.MatrixMean; +import org.elasticsearch.xpack.sql.expression.function.aggregate.MatrixVariance; import org.elasticsearch.xpack.sql.expression.function.aggregate.Max; import org.elasticsearch.xpack.sql.expression.function.aggregate.Mean; import org.elasticsearch.xpack.sql.expression.function.aggregate.Min; @@ -99,13 +104,17 @@ public class DefaultFunctionRegistry extends AbstractFunctionRegistry { Mean.class, StddevPop.class, VarPop.class, + Percentile.class, + PercentileRank.class, SumOfSquares.class, + // Matrix aggs + MatrixCount.class, + MatrixMean.class, + MatrixVariance.class, Skewness.class, Kurtosis.class, - Percentile.class, - PercentileRank.class - // TODO: add multi arg functions like Covariance, Correlate - + Covariance.class, + Correlation.class ); } @@ -154,4 +163,4 @@ public class DefaultFunctionRegistry extends AbstractFunctionRegistry { Tan.class ); } -} \ No newline at end of file +} diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/Correlation.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/Correlation.java new file mode 100644 index 00000000000..a49d6fa6978 --- /dev/null +++ b/sql/server/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/Correlation.java @@ -0,0 +1,21 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.xpack.sql.expression.function.aggregate; + +import org.elasticsearch.xpack.sql.expression.Expression; +import org.elasticsearch.xpack.sql.tree.Location; + +public class Correlation extends NumericAggregate implements MatrixStatsEnclosed { + + public Correlation(Location location, Expression field) { + super(location, field); + } + + @Override + public String innerName() { + return "correlation"; + } +} diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/Covariance.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/Covariance.java new file mode 100644 index 00000000000..bd8d5f83ccb --- /dev/null +++ b/sql/server/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/Covariance.java @@ -0,0 +1,21 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.xpack.sql.expression.function.aggregate; + +import org.elasticsearch.xpack.sql.expression.Expression; +import org.elasticsearch.xpack.sql.tree.Location; + +public class Covariance extends NumericAggregate implements MatrixStatsEnclosed { + + public Covariance(Location location, Expression field) { + super(location, field); + } + + @Override + public String innerName() { + return "covariance"; + } +} diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/MatrixCount.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/MatrixCount.java new file mode 100644 index 00000000000..e84cabf8d04 --- /dev/null +++ b/sql/server/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/MatrixCount.java @@ -0,0 +1,21 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.xpack.sql.expression.function.aggregate; + +import org.elasticsearch.xpack.sql.expression.Expression; +import org.elasticsearch.xpack.sql.tree.Location; + +public class MatrixCount extends NumericAggregate implements MatrixStatsEnclosed { + + public MatrixCount(Location location, Expression field) { + super(location, field); + } + + @Override + public String innerName() { + return "matrix_count"; + } +} diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/MatrixMean.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/MatrixMean.java new file mode 100644 index 00000000000..7fe9be31a14 --- /dev/null +++ b/sql/server/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/MatrixMean.java @@ -0,0 +1,21 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.xpack.sql.expression.function.aggregate; + +import org.elasticsearch.xpack.sql.expression.Expression; +import org.elasticsearch.xpack.sql.tree.Location; + +public class MatrixMean extends NumericAggregate implements MatrixStatsEnclosed { + + public MatrixMean(Location location, Expression field) { + super(location, field); + } + + @Override + public String innerName() { + return "matrix_mean"; + } +} diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/MatrixVariance.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/MatrixVariance.java new file mode 100644 index 00000000000..549317da58f --- /dev/null +++ b/sql/server/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/MatrixVariance.java @@ -0,0 +1,21 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.xpack.sql.expression.function.aggregate; + +import org.elasticsearch.xpack.sql.expression.Expression; +import org.elasticsearch.xpack.sql.tree.Location; + +public class MatrixVariance extends NumericAggregate implements MatrixStatsEnclosed { + + public MatrixVariance(Location location, Expression field) { + super(location, field); + } + + @Override + public String innerName() { + return "matrix_variance"; + } +} diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/planner/PlanningException.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/planner/PlanningException.java index 0537550944b..555c5106664 100644 --- a/sql/server/src/main/java/org/elasticsearch/xpack/sql/planner/PlanningException.java +++ b/sql/server/src/main/java/org/elasticsearch/xpack/sql/planner/PlanningException.java @@ -5,24 +5,35 @@ */ package org.elasticsearch.xpack.sql.planner; +import org.elasticsearch.rest.RestStatus; import org.elasticsearch.xpack.sql.ClientSqlException; import org.elasticsearch.xpack.sql.planner.Verifier.Failure; import org.elasticsearch.xpack.sql.util.StringUtils; import java.util.Collection; import java.util.Locale; +import java.util.Optional; import java.util.stream.Collectors; import static java.lang.String.format; public class PlanningException extends ClientSqlException { + private final Optional status; + public PlanningException(String message, Object... args) { super(message, args); + status = Optional.empty(); + } + + public PlanningException(String message, RestStatus restStatus, Object... args) { + super(message, args); + status = Optional.of(restStatus); } public PlanningException(Collection sources) { super(extractMessage(sources)); + status = Optional.empty(); } private static String extractMessage(Collection failures) { @@ -30,4 +41,9 @@ public class PlanningException extends ClientSqlException { .map(f -> format(Locale.ROOT, "line %s:%s: %s", f.source().location().getLineNumber(), f.source().location().getColumnNumber(), f.message())) .collect(Collectors.joining(StringUtils.NEW_LINE, "Found " + failures.size() + " problem(s)\n", StringUtils.EMPTY)); } + + @Override + public RestStatus status() { + return status.orElse(super.status()); + } } diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/planner/QueryFolder.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/planner/QueryFolder.java index b62ac5aaea7..3970a67da18 100644 --- a/sql/server/src/main/java/org/elasticsearch/xpack/sql/planner/QueryFolder.java +++ b/sql/server/src/main/java/org/elasticsearch/xpack/sql/planner/QueryFolder.java @@ -6,6 +6,7 @@ package org.elasticsearch.xpack.sql.planner; import org.elasticsearch.common.collect.Tuple; +import org.elasticsearch.rest.RestStatus; import org.elasticsearch.xpack.sql.expression.Alias; import org.elasticsearch.xpack.sql.expression.Attribute; import org.elasticsearch.xpack.sql.expression.Expression; @@ -354,7 +355,7 @@ class QueryFolder extends RuleExecutor { //FIXME: what about inner key queryC = withAgg.v1().addAggColumn(withAgg.v2().context()); if (withAgg.v2().innerKey() != null) { - throw new PlanningException("innerkey/matrix stats not handled (yet)"); + throw new PlanningException("innerkey/matrix stats not handled (yet)", RestStatus.BAD_REQUEST); } } } @@ -567,4 +568,4 @@ class QueryFolder extends RuleExecutor { @Override protected abstract PhysicalPlan rule(SubPlan plan); } -} \ No newline at end of file +}