From 9fb2f670dc9c946fb0c0392d30442a52abd3d720 Mon Sep 17 00:00:00 2001 From: Marios Trivyzas Date: Fri, 1 Mar 2019 00:10:30 +0100 Subject: [PATCH] SQL: Enhance checks for inexact fields (#39427) For functions: move checks for `text` fields without underlying `keyword` fields or with many of them (ambiguity) to the type resolution stage. For Order By/Group By: move checks to the `Verifier` to catch early before `QueryTranslator` or execution. Closes: #38501 Fixes: #35203 --- .../sql/qa/src/main/resources/docs.csv-spec | 22 +-- .../xpack/sql/analysis/analyzer/Verifier.java | 19 ++- .../sql/execution/search/SourceGenerator.java | 8 +- .../xpack/sql/expression/Expressions.java | 57 -------- .../xpack/sql/expression/FieldAttribute.java | 7 +- .../xpack/sql/expression/Order.java | 8 +- .../xpack/sql/expression/TypeResolutions.java | 129 ++++++++++++++++++ .../function/aggregate/AggregateFunction.java | 9 +- .../expression/function/aggregate/Max.java | 5 +- .../expression/function/aggregate/Min.java | 5 +- .../function/aggregate/NumericAggregate.java | 5 +- .../function/aggregate/Percentile.java | 18 +-- .../function/aggregate/PercentileRank.java | 18 +-- .../function/aggregate/TopHits.java | 39 +++--- .../function/grouping/Histogram.java | 11 +- .../scalar/datetime/BaseDateTimeFunction.java | 5 +- .../scalar/math/BinaryNumericFunction.java | 6 +- .../function/scalar/math/MathFunction.java | 4 +- .../scalar/string/BinaryStringFunction.java | 6 +- .../string/BinaryStringNumericFunction.java | 4 +- .../string/BinaryStringStringFunction.java | 4 +- .../function/scalar/string/Concat.java | 11 +- .../function/scalar/string/Insert.java | 14 +- .../function/scalar/string/Locate.java | 16 +-- .../function/scalar/string/Replace.java | 11 +- .../function/scalar/string/Substring.java | 12 +- .../scalar/string/UnaryStringFunction.java | 6 +- .../scalar/string/UnaryStringIntFunction.java | 4 +- .../predicate/logical/BinaryLogic.java | 4 +- .../sql/expression/predicate/logical/Not.java | 5 +- .../arithmetic/ArithmeticOperation.java | 6 +- .../predicate/operator/arithmetic/Neg.java | 5 +- .../operator/comparison/BinaryComparison.java | 5 +- .../predicate/operator/comparison/In.java | 13 +- .../predicate/regex/RegexMatch.java | 8 ++ .../xpack/sql/planner/QueryTranslator.java | 18 +-- .../elasticsearch/xpack/sql/type/EsField.java | 41 ++++-- .../xpack/sql/type/InvalidMappedField.java | 10 +- .../xpack/sql/type/KeywordEsField.java | 6 +- .../xpack/sql/type/TextEsField.java | 40 ++++-- .../xpack/sql/type/UnsupportedEsField.java | 13 +- .../analyzer/FieldAttributeTests.java | 18 +-- .../analyzer/VerifierErrorMessagesTests.java | 125 +++++++++++++---- .../sql/planner/QueryTranslatorTests.java | 19 +-- 44 files changed, 513 insertions(+), 286 deletions(-) create mode 100644 x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/TypeResolutions.java diff --git a/x-pack/plugin/sql/qa/src/main/resources/docs.csv-spec b/x-pack/plugin/sql/qa/src/main/resources/docs.csv-spec index b61c44e887c..1f7734ff92c 100644 --- a/x-pack/plugin/sql/qa/src/main/resources/docs.csv-spec +++ b/x-pack/plugin/sql/qa/src/main/resources/docs.csv-spec @@ -2348,25 +2348,25 @@ Arumugam //////////// limitationSubSelect // tag::limitationSubSelect -SELECT * FROM (SELECT first_name, last_name FROM emp WHERE last_name NOT LIKE '%a%') WHERE first_name LIKE 'A%'; +SELECT * FROM (SELECT first_name, last_name FROM emp WHERE last_name NOT LIKE '%a%') WHERE first_name LIKE 'A%' ORDER BY 1; first_name | last_name ---------------+--------------- -Anneke |Preusig -Alejandro |McAlpine -Anoosh |Peyn -Arumugam |Ossenbruggen + Alejandro |McAlpine + Anneke |Preusig + Anoosh |Peyn + Arumugam |Ossenbruggen // end::limitationSubSelect ; -limitationSubSelect +limitationSubSelectRewritten // tag::limitationSubSelectRewritten -SELECT first_name, last_name FROM emp WHERE last_name NOT LIKE '%a%' AND first_name LIKE 'A%'; +SELECT first_name, last_name FROM emp WHERE last_name NOT LIKE '%a%' AND first_name LIKE 'A%' ORDER BY 1; // end::limitationSubSelectRewritten first_name | last_name ---------------+--------------- -Anneke |Preusig -Alejandro |McAlpine -Anoosh |Peyn -Arumugam |Ossenbruggen + Alejandro |McAlpine + Anneke |Preusig + Anoosh |Peyn + Arumugam |Ossenbruggen ; diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/Verifier.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/Verifier.java index 52d53538bb2..47c53e772d5 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/Verifier.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/Verifier.java @@ -41,6 +41,7 @@ import org.elasticsearch.xpack.sql.stats.FeatureMetric; import org.elasticsearch.xpack.sql.stats.Metrics; import org.elasticsearch.xpack.sql.tree.Node; import org.elasticsearch.xpack.sql.type.DataType; +import org.elasticsearch.xpack.sql.type.EsField; import org.elasticsearch.xpack.sql.util.StringUtils; import java.util.ArrayList; @@ -294,7 +295,8 @@ public final class Verifier { */ private static boolean checkGroupBy(LogicalPlan p, Set localFailures, Map resolvedFunctions, Set groupingFailures) { - return checkGroupByAgg(p, localFailures, resolvedFunctions) + return checkGroupByInexactField(p, localFailures) + && checkGroupByAgg(p, localFailures, resolvedFunctions) && checkGroupByOrder(p, localFailures, groupingFailures) && checkGroupByHaving(p, localFailures, groupingFailures, resolvedFunctions); } @@ -463,6 +465,21 @@ public final class Verifier { return false; } + private static boolean checkGroupByInexactField(LogicalPlan p, Set localFailures) { + if (p instanceof Aggregate) { + Aggregate a = (Aggregate) p; + + // The grouping can not be an aggregate function or an inexact field (e.g. text without a keyword) + a.groupings().forEach(e -> e.forEachUp(c -> { + EsField.Exact exact = c.getExactInfo(); + if (exact.hasExact() == false) { + localFailures.add(fail(c, "Field of data type [" + c.dataType().typeName + "] cannot be used for grouping; " + + exact.errorMsg())); + } + }, FieldAttribute.class)); + } + return true; + } // check whether plain columns specified in an agg are mentioned in the group-by private static boolean checkGroupByAgg(LogicalPlan p, Set localFailures, Map functions) { diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/SourceGenerator.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/SourceGenerator.java index c22b1213d09..47ffcf69732 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/SourceGenerator.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/SourceGenerator.java @@ -33,6 +33,8 @@ import static org.elasticsearch.search.sort.SortBuilders.scriptSort; public abstract class SourceGenerator { + private SourceGenerator() {} + private static final List NO_STORED_FIELD = singletonList(StoredFieldsContext._NONE_); public static SearchSourceBuilder sourceBuilder(QueryContainer container, QueryBuilder filter, Integer size) { @@ -107,8 +109,7 @@ public abstract class SourceGenerator { // sorting only works on not-analyzed fields - look for a multi-field replacement if (attr instanceof FieldAttribute) { - FieldAttribute fa = (FieldAttribute) attr; - fa = fa.isInexact() ? fa.exactAttribute() : fa; + FieldAttribute fa = ((FieldAttribute) attr).exactAttribute(); sortBuilder = fieldSort(fa.name()) .missing(as.missing().position()) @@ -125,7 +126,8 @@ public abstract class SourceGenerator { if (nestedSort == null) { fieldSort.setNestedSort(newSort); } else { - for (; nestedSort.getNestedSort() != null; nestedSort = nestedSort.getNestedSort()) { + while (nestedSort.getNestedSort() != null) { + nestedSort = nestedSort.getNestedSort(); } nestedSort.setNestedSort(newSort); } 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 index 648aff52545..ca5e4b75756 100644 --- 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 @@ -6,22 +6,16 @@ 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 org.elasticsearch.xpack.sql.type.DataTypes; import java.util.ArrayList; import java.util.Collection; import java.util.List; -import java.util.Locale; -import java.util.StringJoiner; import java.util.function.Predicate; import static java.util.Collections.emptyList; import static java.util.Collections.emptyMap; -import static org.elasticsearch.common.logging.LoggerMessageFormat.format; -import static org.elasticsearch.xpack.sql.type.DataType.BOOLEAN; public final class Expressions { @@ -154,55 +148,4 @@ public final class Expressions { } return pipes; } - - public static TypeResolution typeMustBeBoolean(Expression e, String operationName, ParamOrdinal paramOrd) { - return typeMustBe(e, dt -> dt == BOOLEAN, operationName, paramOrd, "boolean"); - } - - public static TypeResolution typeMustBeInteger(Expression e, String operationName, ParamOrdinal paramOrd) { - return typeMustBe(e, DataType::isInteger, operationName, paramOrd, "integer"); - } - - public static TypeResolution typeMustBeNumeric(Expression e, String operationName, ParamOrdinal paramOrd) { - return typeMustBe(e, DataType::isNumeric, operationName, paramOrd, "numeric"); - } - - public static TypeResolution typeMustBeString(Expression e, String operationName, ParamOrdinal paramOrd) { - return typeMustBe(e, DataType::isString, operationName, paramOrd, "string"); - } - - public static TypeResolution typeMustBeDate(Expression e, String operationName, ParamOrdinal paramOrd) { - return typeMustBe(e, DataType::isDateBased, operationName, paramOrd, "date", "datetime"); - } - - public static TypeResolution typeMustBeNumericOrDate(Expression e, String operationName, ParamOrdinal paramOrd) { - return typeMustBe(e, dt -> dt.isNumeric() || dt.isDateBased(), operationName, paramOrd, "date", "datetime", "numeric"); - } - - public static TypeResolution typeMustBe(Expression e, - Predicate predicate, - String operationName, - ParamOrdinal paramOrd, - String... acceptedTypes) { - return predicate.test(e.dataType()) || DataTypes.isNull(e.dataType())? - TypeResolution.TYPE_RESOLVED : - new TypeResolution(format(null, "[{}]{} argument must be [{}], found value [{}] type [{}]", - operationName, - paramOrd == null || paramOrd == ParamOrdinal.DEFAULT ? "" : " " + paramOrd.name().toLowerCase(Locale.ROOT), - acceptedTypesForErrorMsg(acceptedTypes), - Expressions.name(e), - e.dataType().typeName)); - } - - private static String acceptedTypesForErrorMsg(String... acceptedTypes) { - StringJoiner sj = new StringJoiner(", "); - for (int i = 0; i < acceptedTypes.length - 1; i++) { - sj.add(acceptedTypes[i]); - } - if (acceptedTypes.length > 1) { - return sj.toString() + " or " + acceptedTypes[acceptedTypes.length - 1]; - } else { - return acceptedTypes[0]; - } - } } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/FieldAttribute.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/FieldAttribute.java index 832af029df3..811cc299ccb 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/FieldAttribute.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/FieldAttribute.java @@ -81,12 +81,13 @@ public class FieldAttribute extends TypedAttribute { return nestedParent; } - public boolean isInexact() { - return field.isExact() == false; + public EsField.Exact getExactInfo() { + return field.getExactInfo(); } public FieldAttribute exactAttribute() { - if (field.isExact() == false) { + EsField exactField = field.getExactField(); + if (exactField.equals(field) == false) { return innerField(field.getExactField()); } return this; diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Order.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Order.java index 6a57c3275d4..267a8827d8c 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Order.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Order.java @@ -5,14 +5,15 @@ */ package org.elasticsearch.xpack.sql.expression; -import org.elasticsearch.xpack.sql.tree.Source; import org.elasticsearch.xpack.sql.tree.NodeInfo; +import org.elasticsearch.xpack.sql.tree.Source; import org.elasticsearch.xpack.sql.type.DataType; import java.util.List; import java.util.Objects; import static java.util.Collections.singletonList; +import static org.elasticsearch.xpack.sql.expression.TypeResolutions.isExact; public class Order extends Expression { @@ -45,6 +46,11 @@ public class Order extends Expression { return Nullability.FALSE; } + @Override + protected TypeResolution resolveType() { + return isExact(child, "ORDER BY cannot be applied to field of data type [{}]: {}"); + } + @Override public DataType dataType() { return child.dataType(); diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/TypeResolutions.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/TypeResolutions.java new file mode 100644 index 00000000000..61bc8ed44a9 --- /dev/null +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/TypeResolutions.java @@ -0,0 +1,129 @@ +/* + * 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; + +import org.elasticsearch.xpack.sql.type.DataType; +import org.elasticsearch.xpack.sql.type.DataTypes; +import org.elasticsearch.xpack.sql.type.EsField; + +import java.util.Locale; +import java.util.StringJoiner; +import java.util.function.Predicate; + +import static org.elasticsearch.common.logging.LoggerMessageFormat.format; +import static org.elasticsearch.xpack.sql.expression.Expression.TypeResolution; +import static org.elasticsearch.xpack.sql.expression.Expressions.ParamOrdinal; +import static org.elasticsearch.xpack.sql.expression.Expressions.name; +import static org.elasticsearch.xpack.sql.type.DataType.BOOLEAN; + +public final class TypeResolutions { + + private TypeResolutions() {} + + public static TypeResolution isBoolean(Expression e, String operationName, ParamOrdinal paramOrd) { + return isType(e, dt -> dt == BOOLEAN, operationName, paramOrd, "boolean"); + } + + public static TypeResolution isInteger(Expression e, String operationName, ParamOrdinal paramOrd) { + return isType(e, DataType::isInteger, operationName, paramOrd, "integer"); + } + + public static TypeResolution isNumeric(Expression e, String operationName, ParamOrdinal paramOrd) { + return isType(e, DataType::isNumeric, operationName, paramOrd, "numeric"); + } + + public static TypeResolution isString(Expression e, String operationName, ParamOrdinal paramOrd) { + return isType(e, DataType::isString, operationName, paramOrd, "string"); + } + + public static TypeResolution isDate(Expression e, String operationName, ParamOrdinal paramOrd) { + return isType(e, DataType::isDateBased, operationName, paramOrd, "date", "datetime"); + } + + public static TypeResolution isNumericOrDate(Expression e, String operationName, ParamOrdinal paramOrd) { + return isType(e, dt -> dt.isNumeric() || dt.isDateBased(), operationName, paramOrd, "date", "datetime", "numeric"); + } + + public static TypeResolution isExact(Expression e, String message) { + if (e instanceof FieldAttribute) { + EsField.Exact exact = ((FieldAttribute) e).getExactInfo(); + if (exact.hasExact() == false) { + return new TypeResolution(format(null, message, e.dataType().typeName, exact.errorMsg())); + } + } + return TypeResolution.TYPE_RESOLVED; + } + + public static TypeResolution isExact(Expression e, String operationName, ParamOrdinal paramOrd) { + if (e instanceof FieldAttribute) { + EsField.Exact exact = ((FieldAttribute) e).getExactInfo(); + if (exact.hasExact() == false) { + return new TypeResolution(format(null, "[{}] cannot operate on {}field of data type [{}]: {}", + operationName, + paramOrd == null || paramOrd == ParamOrdinal.DEFAULT ? + "" : paramOrd.name().toLowerCase(Locale.ROOT) + " argument ", + e.dataType().typeName, exact.errorMsg())); + } + } + return TypeResolution.TYPE_RESOLVED; + } + + public static TypeResolution isStringAndExact(Expression e, String operationName, ParamOrdinal paramOrd) { + TypeResolution resolution = isString(e, operationName, paramOrd); + if (resolution.unresolved()) { + return resolution; + } + + return isExact(e, operationName, paramOrd); + } + + public static TypeResolution isFoldable(Expression e, String operationName, ParamOrdinal paramOrd) { + if (!e.foldable()) { + return new TypeResolution(format(null, "{}argument of [{}] must be a constant, received [{}]", + paramOrd == null || paramOrd == ParamOrdinal.DEFAULT ? "" : paramOrd.name().toLowerCase(Locale.ROOT) + " ", + operationName, + Expressions.name(e))); + } + return TypeResolution.TYPE_RESOLVED; + } + + public static TypeResolution isNotFoldable(Expression e, String operationName, ParamOrdinal paramOrd) { + if (e.foldable()) { + return new TypeResolution(format(null, "{}argument of [{}] must be a table column, found constant [{}]", + paramOrd == null || paramOrd == ParamOrdinal.DEFAULT ? "" : paramOrd.name().toLowerCase(Locale.ROOT) + " ", + operationName, + Expressions.name(e))); + } + return TypeResolution.TYPE_RESOLVED; + } + + public static TypeResolution isType(Expression e, + Predicate predicate, + String operationName, + ParamOrdinal paramOrd, + String... acceptedTypes) { + return predicate.test(e.dataType()) || DataTypes.isNull(e.dataType())? + TypeResolution.TYPE_RESOLVED : + new TypeResolution(format(null, "{}argument of [{}] must be [{}], found value [{}] type [{}]", + paramOrd == null || paramOrd == ParamOrdinal.DEFAULT ? "" : paramOrd.name().toLowerCase(Locale.ROOT) + " ", + operationName, + acceptedTypesForErrorMsg(acceptedTypes), + name(e), + e.dataType().typeName)); + } + + private static String acceptedTypesForErrorMsg(String... acceptedTypes) { + StringJoiner sj = new StringJoiner(", "); + for (int i = 0; i < acceptedTypes.length - 1; i++) { + sj.add(acceptedTypes[i]); + } + if (acceptedTypes.length > 1) { + return sj.toString() + " or " + acceptedTypes[acceptedTypes.length - 1]; + } else { + return acceptedTypes[0]; + } + } +} diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/AggregateFunction.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/AggregateFunction.java index b432c5063a6..177f598dc9a 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/AggregateFunction.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/AggregateFunction.java @@ -7,6 +7,8 @@ package org.elasticsearch.xpack.sql.expression.function.aggregate; import org.elasticsearch.xpack.sql.SqlIllegalArgumentException; import org.elasticsearch.xpack.sql.expression.Expression; +import org.elasticsearch.xpack.sql.expression.Expressions; +import org.elasticsearch.xpack.sql.expression.TypeResolutions; import org.elasticsearch.xpack.sql.expression.function.Function; import org.elasticsearch.xpack.sql.expression.gen.pipeline.AggNameInput; import org.elasticsearch.xpack.sql.expression.gen.pipeline.Pipe; @@ -78,8 +80,13 @@ public abstract class AggregateFunction extends Function { && Objects.equals(other.parameters(), parameters()); } + @Override + protected TypeResolution resolveType() { + return TypeResolutions.isExact(field, sourceText(), Expressions.ParamOrdinal.DEFAULT); + } + @Override public int hashCode() { return Objects.hash(field(), parameters()); } -} \ No newline at end of file +} diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/Max.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/Max.java index 898c9846344..cd03ea85e45 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/Max.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/Max.java @@ -6,7 +6,6 @@ package org.elasticsearch.xpack.sql.expression.function.aggregate; import org.elasticsearch.xpack.sql.expression.Expression; -import org.elasticsearch.xpack.sql.expression.Expressions; import org.elasticsearch.xpack.sql.expression.Expressions.ParamOrdinal; import org.elasticsearch.xpack.sql.tree.NodeInfo; import org.elasticsearch.xpack.sql.tree.Source; @@ -14,6 +13,8 @@ import org.elasticsearch.xpack.sql.type.DataType; import java.util.List; +import static org.elasticsearch.xpack.sql.expression.TypeResolutions.isNumericOrDate; + /** * Find the maximum value in matching documents. */ @@ -48,7 +49,7 @@ public class Max extends NumericAggregate implements EnclosedAgg { if (field().dataType().isString()) { return TypeResolution.TYPE_RESOLVED; } else { - return Expressions.typeMustBeNumericOrDate(field(), sourceText(), ParamOrdinal.DEFAULT); + return isNumericOrDate(field(), sourceText(), ParamOrdinal.DEFAULT); } } } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/Min.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/Min.java index 8652759fca4..07fa44769b2 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/Min.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/Min.java @@ -6,7 +6,6 @@ package org.elasticsearch.xpack.sql.expression.function.aggregate; import org.elasticsearch.xpack.sql.expression.Expression; -import org.elasticsearch.xpack.sql.expression.Expressions; import org.elasticsearch.xpack.sql.expression.Expressions.ParamOrdinal; import org.elasticsearch.xpack.sql.tree.NodeInfo; import org.elasticsearch.xpack.sql.tree.Source; @@ -14,6 +13,8 @@ import org.elasticsearch.xpack.sql.type.DataType; import java.util.List; +import static org.elasticsearch.xpack.sql.expression.TypeResolutions.isNumericOrDate; + /** * Find the minimum value in matched documents. */ @@ -51,7 +52,7 @@ public class Min extends NumericAggregate implements EnclosedAgg { if (field().dataType().isString()) { return TypeResolution.TYPE_RESOLVED; } else { - return Expressions.typeMustBeNumericOrDate(field(), sourceText(), ParamOrdinal.DEFAULT); + return isNumericOrDate(field(), sourceText(), ParamOrdinal.DEFAULT); } } } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/NumericAggregate.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/NumericAggregate.java index bfe0d2ded7e..21d5c23d23a 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/NumericAggregate.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/NumericAggregate.java @@ -6,13 +6,14 @@ package org.elasticsearch.xpack.sql.expression.function.aggregate; import org.elasticsearch.xpack.sql.expression.Expression; -import org.elasticsearch.xpack.sql.expression.Expressions; import org.elasticsearch.xpack.sql.expression.Expressions.ParamOrdinal; import org.elasticsearch.xpack.sql.tree.Source; import org.elasticsearch.xpack.sql.type.DataType; import java.util.List; +import static org.elasticsearch.xpack.sql.expression.TypeResolutions.isNumeric; + abstract class NumericAggregate extends AggregateFunction { NumericAggregate(Source source, Expression field, List parameters) { @@ -25,7 +26,7 @@ abstract class NumericAggregate extends AggregateFunction { @Override protected TypeResolution resolveType() { - return Expressions.typeMustBeNumeric(field(), sourceText(), ParamOrdinal.DEFAULT); + return isNumeric(field(), sourceText(), ParamOrdinal.DEFAULT); } @Override diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/Percentile.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/Percentile.java index 76c7bda3200..a0585f4c021 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/Percentile.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/Percentile.java @@ -6,7 +6,6 @@ package org.elasticsearch.xpack.sql.expression.function.aggregate; import org.elasticsearch.xpack.sql.expression.Expression; -import org.elasticsearch.xpack.sql.expression.Expressions; import org.elasticsearch.xpack.sql.expression.Expressions.ParamOrdinal; import org.elasticsearch.xpack.sql.expression.Foldables; import org.elasticsearch.xpack.sql.tree.NodeInfo; @@ -16,7 +15,8 @@ import org.elasticsearch.xpack.sql.type.DataType; import java.util.List; import static java.util.Collections.singletonList; -import static org.elasticsearch.common.logging.LoggerMessageFormat.format; +import static org.elasticsearch.xpack.sql.expression.TypeResolutions.isFoldable; +import static org.elasticsearch.xpack.sql.expression.TypeResolutions.isNumeric; public class Percentile extends NumericAggregate implements EnclosedAgg { @@ -42,17 +42,17 @@ public class Percentile extends NumericAggregate implements EnclosedAgg { @Override protected TypeResolution resolveType() { - if (!percent.foldable()) { - return new TypeResolution(format(null, "Second argument of PERCENTILE must be a constant, received [{}]", - Expressions.name(percent))); - } - - TypeResolution resolution = super.resolveType(); + TypeResolution resolution = isFoldable(percent, sourceText(), ParamOrdinal.SECOND); if (resolution.unresolved()) { return resolution; } - return Expressions.typeMustBeNumeric(percent, sourceText(), ParamOrdinal.DEFAULT); + resolution = super.resolveType(); + if (resolution.unresolved()) { + return resolution; + } + + return isNumeric(percent, sourceText(), ParamOrdinal.DEFAULT); } public Expression percent() { diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/PercentileRank.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/PercentileRank.java index b30b38a01b6..da8c487ff31 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/PercentileRank.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/PercentileRank.java @@ -6,7 +6,6 @@ package org.elasticsearch.xpack.sql.expression.function.aggregate; import org.elasticsearch.xpack.sql.expression.Expression; -import org.elasticsearch.xpack.sql.expression.Expressions; import org.elasticsearch.xpack.sql.expression.Expressions.ParamOrdinal; import org.elasticsearch.xpack.sql.expression.Foldables; import org.elasticsearch.xpack.sql.tree.NodeInfo; @@ -16,7 +15,8 @@ import org.elasticsearch.xpack.sql.type.DataType; import java.util.List; import static java.util.Collections.singletonList; -import static org.elasticsearch.common.logging.LoggerMessageFormat.format; +import static org.elasticsearch.xpack.sql.expression.TypeResolutions.isFoldable; +import static org.elasticsearch.xpack.sql.expression.TypeResolutions.isNumeric; public class PercentileRank extends AggregateFunction implements EnclosedAgg { @@ -42,17 +42,17 @@ public class PercentileRank extends AggregateFunction implements EnclosedAgg { @Override protected TypeResolution resolveType() { - if (!value.foldable()) { - return new TypeResolution(format(null, "Second argument of PERCENTILE_RANK must be a constant, received [{}]", - Expressions.name(value))); - } - - TypeResolution resolution = super.resolveType(); + TypeResolution resolution = isFoldable(value, sourceText(), ParamOrdinal.SECOND); if (resolution.unresolved()) { return resolution; } - return Expressions.typeMustBeNumeric(value, sourceText(), ParamOrdinal.DEFAULT); + resolution = super.resolveType(); + if (resolution.unresolved()) { + return resolution; + } + + return isNumeric(value, sourceText(), ParamOrdinal.DEFAULT); } public Expression value() { diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/TopHits.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/TopHits.java index 227ca9b8db3..9364f5f4fc5 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/TopHits.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/TopHits.java @@ -5,16 +5,15 @@ */ package org.elasticsearch.xpack.sql.expression.function.aggregate; -import org.elasticsearch.xpack.sql.analysis.index.MappingException; import org.elasticsearch.xpack.sql.expression.Expression; -import org.elasticsearch.xpack.sql.expression.Expressions; -import org.elasticsearch.xpack.sql.expression.FieldAttribute; +import org.elasticsearch.xpack.sql.expression.TypeResolutions; import org.elasticsearch.xpack.sql.tree.Source; import org.elasticsearch.xpack.sql.type.DataType; import java.util.Collections; -import static org.elasticsearch.common.logging.LoggerMessageFormat.format; +import static org.elasticsearch.xpack.sql.expression.Expressions.ParamOrdinal; +import static org.elasticsearch.xpack.sql.expression.TypeResolutions.isNotFoldable; /** * Super class of Aggregation functions on field types other than numeric, that need to be @@ -37,29 +36,25 @@ public abstract class TopHits extends AggregateFunction { @Override protected TypeResolution resolveType() { - if (field().foldable()) { - return new TypeResolution(format(null, "First argument of [{}] must be a table column, found constant [{}]", - functionName(), - Expressions.name(field()))); + TypeResolution resolution = isNotFoldable(field(), sourceText(), ParamOrdinal.FIRST); + if (resolution.unresolved()) { + return resolution; } - try { - ((FieldAttribute) field()).exactAttribute(); - } catch (MappingException ex) { - return new TypeResolution(format(null, "[{}] cannot operate on first argument field of data type [{}]", - functionName(), field().dataType().typeName)); + + resolution = TypeResolutions.isExact(field(), sourceText(), ParamOrdinal.FIRST); + if (resolution.unresolved()) { + return resolution; } if (orderField() != null) { - if (orderField().foldable()) { - return new TypeResolution(format(null, "Second argument of [{}] must be a table column, found constant [{}]", - functionName(), - Expressions.name(orderField()))); + resolution = isNotFoldable(orderField(), sourceText(), ParamOrdinal.SECOND); + if (resolution.unresolved()) { + return resolution; } - try { - ((FieldAttribute) orderField()).exactAttribute(); - } catch (MappingException ex) { - return new TypeResolution(format(null, "[{}] cannot operate on second argument field of data type [{}]", - functionName(), orderField().dataType().typeName)); + + resolution = TypeResolutions.isExact(orderField(), sourceText(), ParamOrdinal.SECOND); + if (resolution.unresolved()) { + return resolution; } } return TypeResolution.TYPE_RESOLVED; diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/grouping/Histogram.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/grouping/Histogram.java index 23061bfea18..9cb752de5e6 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/grouping/Histogram.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/grouping/Histogram.java @@ -7,7 +7,6 @@ package org.elasticsearch.xpack.sql.expression.function.grouping; import org.elasticsearch.xpack.sql.expression.Expression; -import org.elasticsearch.xpack.sql.expression.Expressions; import org.elasticsearch.xpack.sql.expression.Expressions.ParamOrdinal; import org.elasticsearch.xpack.sql.expression.Literal; import org.elasticsearch.xpack.sql.tree.NodeInfo; @@ -20,6 +19,10 @@ import java.util.Collections; import java.util.List; import java.util.Objects; +import static org.elasticsearch.xpack.sql.expression.TypeResolutions.isNumeric; +import static org.elasticsearch.xpack.sql.expression.TypeResolutions.isNumericOrDate; +import static org.elasticsearch.xpack.sql.expression.TypeResolutions.isType; + public class Histogram extends GroupingFunction { private final Literal interval; @@ -41,13 +44,13 @@ public class Histogram extends GroupingFunction { @Override protected TypeResolution resolveType() { - TypeResolution resolution = Expressions.typeMustBeNumericOrDate(field(), "HISTOGRAM", ParamOrdinal.FIRST); + TypeResolution resolution = isNumericOrDate(field(), "HISTOGRAM", ParamOrdinal.FIRST); if (resolution == TypeResolution.TYPE_RESOLVED) { // interval must be Literal interval if (field().dataType().isDateBased()) { - resolution = Expressions.typeMustBe(interval, DataTypes::isInterval, "(Date) HISTOGRAM", ParamOrdinal.SECOND, "interval"); + resolution = isType(interval, DataTypes::isInterval, "(Date) HISTOGRAM", ParamOrdinal.SECOND, "interval"); } else { - resolution = Expressions.typeMustBeNumeric(interval, "(Numeric) HISTOGRAM", ParamOrdinal.SECOND); + resolution = isNumeric(interval, "(Numeric) HISTOGRAM", ParamOrdinal.SECOND); } } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/datetime/BaseDateTimeFunction.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/datetime/BaseDateTimeFunction.java index fa949007ef5..cae78a42e55 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/datetime/BaseDateTimeFunction.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/datetime/BaseDateTimeFunction.java @@ -7,7 +7,6 @@ package org.elasticsearch.xpack.sql.expression.function.scalar.datetime; import org.elasticsearch.xpack.sql.expression.Expression; -import org.elasticsearch.xpack.sql.expression.Expressions; import org.elasticsearch.xpack.sql.expression.Expressions.ParamOrdinal; import org.elasticsearch.xpack.sql.expression.function.scalar.UnaryScalarFunction; import org.elasticsearch.xpack.sql.tree.NodeInfo; @@ -17,6 +16,8 @@ import java.time.ZoneId; import java.time.ZonedDateTime; import java.util.Objects; +import static org.elasticsearch.xpack.sql.expression.TypeResolutions.isDate; + abstract class BaseDateTimeFunction extends UnaryScalarFunction { private final ZoneId zoneId; @@ -35,7 +36,7 @@ abstract class BaseDateTimeFunction extends UnaryScalarFunction { @Override protected TypeResolution resolveType() { - return Expressions.typeMustBeDate(field(), sourceText(), ParamOrdinal.DEFAULT); + return isDate(field(), sourceText(), ParamOrdinal.DEFAULT); } public ZoneId zoneId() { diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/math/BinaryNumericFunction.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/math/BinaryNumericFunction.java index f3369bf14a4..82303294549 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/math/BinaryNumericFunction.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/math/BinaryNumericFunction.java @@ -16,6 +16,8 @@ import org.elasticsearch.xpack.sql.type.DataType; import java.util.Objects; +import static org.elasticsearch.xpack.sql.expression.TypeResolutions.isNumeric; + public abstract class BinaryNumericFunction extends BinaryScalarFunction { private final BinaryMathOperation operation; @@ -36,12 +38,12 @@ public abstract class BinaryNumericFunction extends BinaryScalarFunction { return new TypeResolution("Unresolved children"); } - TypeResolution resolution = Expressions.typeMustBeNumeric(left(), sourceText(), ParamOrdinal.FIRST); + TypeResolution resolution = isNumeric(left(), sourceText(), ParamOrdinal.FIRST); if (resolution.unresolved()) { return resolution; } - return Expressions.typeMustBeNumeric(right(), sourceText(), ParamOrdinal.SECOND); + return isNumeric(right(), sourceText(), ParamOrdinal.SECOND); } @Override diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/math/MathFunction.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/math/MathFunction.java index e0555ab0ea3..4389e1ac814 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/math/MathFunction.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/math/MathFunction.java @@ -6,7 +6,6 @@ package org.elasticsearch.xpack.sql.expression.function.scalar.math; import org.elasticsearch.xpack.sql.expression.Expression; -import org.elasticsearch.xpack.sql.expression.Expressions; import org.elasticsearch.xpack.sql.expression.Expressions.ParamOrdinal; import org.elasticsearch.xpack.sql.expression.function.scalar.UnaryScalarFunction; import org.elasticsearch.xpack.sql.expression.function.scalar.math.MathProcessor.MathOperation; @@ -18,6 +17,7 @@ import java.util.Locale; import java.util.Objects; import static java.lang.String.format; +import static org.elasticsearch.xpack.sql.expression.TypeResolutions.isNumeric; public abstract class MathFunction extends UnaryScalarFunction { @@ -56,7 +56,7 @@ public abstract class MathFunction extends UnaryScalarFunction { return new TypeResolution("Unresolved children"); } - return Expressions.typeMustBeNumeric(field(), operation().toString(), ParamOrdinal.DEFAULT); + return isNumeric(field(), sourceText(), ParamOrdinal.DEFAULT); } @Override diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/BinaryStringFunction.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/BinaryStringFunction.java index fd294564b64..611e86507ee 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/BinaryStringFunction.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/BinaryStringFunction.java @@ -16,7 +16,7 @@ import java.util.Locale; import java.util.Objects; import java.util.function.BiFunction; -import static org.elasticsearch.xpack.sql.expression.Expressions.typeMustBeString; +import static org.elasticsearch.xpack.sql.expression.TypeResolutions.isStringAndExact; import static org.elasticsearch.xpack.sql.expression.gen.script.ParamsBuilder.paramsBuilder; /** @@ -42,7 +42,7 @@ public abstract class BinaryStringFunction extends BinaryScalarFunction { return new TypeResolution("Unresolved children"); } - TypeResolution resolution = typeMustBeString(left(), sourceText(), ParamOrdinal.FIRST); + TypeResolution resolution = isStringAndExact(left(), sourceText(), ParamOrdinal.FIRST); if (resolution.unresolved()) { return resolution; } @@ -67,7 +67,7 @@ public abstract class BinaryStringFunction extends BinaryScalarFunction { @Override public ScriptTemplate scriptWithField(FieldAttribute field) { return new ScriptTemplate(processScript("doc[{}].value"), - paramsBuilder().variable(field.isInexact() ? field.exactAttribute().name() : field.name()).build(), + paramsBuilder().variable(field.exactAttribute().name()).build(), dataType()); } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/BinaryStringNumericFunction.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/BinaryStringNumericFunction.java index d9f767d1ce8..fac0646c2c6 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/BinaryStringNumericFunction.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/BinaryStringNumericFunction.java @@ -12,6 +12,8 @@ import org.elasticsearch.xpack.sql.expression.gen.pipeline.Pipe; import org.elasticsearch.xpack.sql.tree.Source; import org.elasticsearch.xpack.sql.type.DataType; +import static org.elasticsearch.xpack.sql.expression.TypeResolutions.isNumeric; + /** * A binary string function with a numeric second parameter and a string result */ @@ -26,7 +28,7 @@ public abstract class BinaryStringNumericFunction extends BinaryStringFunction { protected BinaryLogic(Source source, Expression left, Expression right, BinaryLogicOperation operation) { @@ -27,7 +29,7 @@ public abstract class BinaryLogic extends BinaryOperator { private DataType dataType; @@ -24,7 +26,7 @@ public abstract class ArithmeticOperation extends BinaryOperator properties; @@ -58,7 +59,9 @@ public class EsField { /** * Returns the path to the keyword version of this field if this field is text and it has a subfield that is - * indexed as keyword, null if such field is not found or the field name itself in all other cases + * indexed as keyword, throws an exception if such field is not found or the field name itself in all other cases. + * To avoid the exception {@link EsField#getExactInfo()} should be used beforehand, to check if an exact field exists + * and if not get the errorMessage which explains why is that. */ public EsField getExactField() { return this; @@ -76,13 +79,14 @@ public class EsField { } /** - * True if this field name can be used in sorting, aggregations and term queries as is - *

- * This will be true for most fields except analyzed text fields that cannot be used directly and should be - * replaced with the field returned by {@link EsField#getExactField()} instead. + * Returns and {@link Exact} object with all the necessary info about the field: + *

    + *
  • If it has an exact underlying field or not
    • + *
  • and if not an error message why it doesn't
    • + *
*/ - public boolean isExact() { - return true; + public Exact getExactInfo() { + return Exact.EXACT_FIELD; } @Override @@ -108,4 +112,25 @@ public class EsField { public int hashCode() { return Objects.hash(esDataType, aggregatable, properties, name); } -} \ No newline at end of file + + public static final class Exact { + + private static Exact EXACT_FIELD = new Exact(true, null); + + private boolean hasExact; + private String errorMsg; + + public Exact(boolean hasExact, String errorMsg) { + this.hasExact = hasExact; + this.errorMsg = errorMsg; + } + + public boolean hasExact() { + return hasExact; + } + + public String errorMsg() { + return errorMsg; + } + } +} diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/InvalidMappedField.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/InvalidMappedField.java index 59bb94c78c8..79f8eb1c20c 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/InvalidMappedField.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/InvalidMappedField.java @@ -6,7 +6,7 @@ package org.elasticsearch.xpack.sql.type; -import org.elasticsearch.xpack.sql.analysis.index.MappingException; +import org.elasticsearch.xpack.sql.SqlIllegalArgumentException; import java.util.Objects; @@ -46,12 +46,12 @@ public class InvalidMappedField extends EsField { @Override public EsField getExactField() { - throw new MappingException("Field [" + getName() + "] is invalid, cannot access it"); + throw new SqlIllegalArgumentException("Field [" + getName() + "] is invalid, cannot access it"); } @Override - public boolean isExact() { - return false; + public Exact getExactInfo() { + return new Exact(false, "Field [" + getName() + "] is invalid, cannot access it"); } -} \ No newline at end of file +} diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/KeywordEsField.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/KeywordEsField.java index d40fa7b19af..3b77608fc8b 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/KeywordEsField.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/KeywordEsField.java @@ -33,8 +33,8 @@ public class KeywordEsField extends EsField { } @Override - public boolean isExact() { - return normalized == false; + public Exact getExactInfo() { + return new Exact(normalized == false, "Normalized keyword field cannot be used for exact match operations"); } @Override @@ -52,4 +52,4 @@ public class KeywordEsField extends EsField { return Objects.hash(super.hashCode(), precision, normalized); } -} \ No newline at end of file +} diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/TextEsField.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/TextEsField.java index f1c596a301c..4944a472e21 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/TextEsField.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/TextEsField.java @@ -5,9 +5,11 @@ */ package org.elasticsearch.xpack.sql.type; -import org.elasticsearch.xpack.sql.analysis.index.MappingException; +import org.elasticsearch.common.collect.Tuple; +import org.elasticsearch.xpack.sql.SqlIllegalArgumentException; import java.util.Map; +import java.util.function.Function; /** * SQL-related information about an index field with text type @@ -20,25 +22,41 @@ public class TextEsField extends EsField { @Override public EsField getExactField() { + Tuple findExact = findExact(); + if (findExact.v1() == null) { + throw new SqlIllegalArgumentException(findExact.v2()); + } + return findExact.v1(); + } + + @Override + public Exact getExactInfo() { + return PROCESS_EXACT_FIELD.apply(findExact()); + } + + private Tuple findExact() { EsField field = null; for (EsField property : getProperties().values()) { - if (property.getDataType() == DataType.KEYWORD && property.isExact()) { + if (property.getDataType() == DataType.KEYWORD && property.getExactInfo().hasExact()) { if (field != null) { - throw new MappingException("Multiple exact keyword candidates available for [" + getName() + - "]; specify which one to use"); + return new Tuple<>(null, "Multiple exact keyword candidates available for [" + getName() + + "]; specify which one to use"); } field = property; } } if (field == null) { - throw new MappingException("No keyword/multi-field defined exact matches for [" + getName() + - "]; define one or use MATCH/QUERY instead"); + return new Tuple<>(null, "No keyword/multi-field defined exact matches for [" + getName() + + "]; define one or use MATCH/QUERY instead"); } - return field; + return new Tuple<>(field, null); } - @Override - public boolean isExact() { - return false; - } + private Function, Exact> PROCESS_EXACT_FIELD = tuple -> { + if (tuple.v1() == null) { + return new Exact(false, tuple.v2()); + } else { + return new Exact(true, null); + } + }; } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/UnsupportedEsField.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/UnsupportedEsField.java index c88d676c223..2909c5f1990 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/UnsupportedEsField.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/UnsupportedEsField.java @@ -26,16 +26,21 @@ public class UnsupportedEsField extends EsField { @Override public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - if (!super.equals(o)) return false; + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + if (!super.equals(o)) { + return false; + } UnsupportedEsField that = (UnsupportedEsField) o; return Objects.equals(originalType, that.originalType); } @Override public int hashCode() { - return Objects.hash(super.hashCode(), originalType); } } diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/FieldAttributeTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/FieldAttributeTests.java index 607810efc66..bc7b85b5392 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/FieldAttributeTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/FieldAttributeTests.java @@ -6,10 +6,10 @@ package org.elasticsearch.xpack.sql.analysis.analyzer; import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.xpack.sql.SqlIllegalArgumentException; import org.elasticsearch.xpack.sql.TestUtils; import org.elasticsearch.xpack.sql.analysis.index.EsIndex; import org.elasticsearch.xpack.sql.analysis.index.IndexResolution; -import org.elasticsearch.xpack.sql.analysis.index.MappingException; import org.elasticsearch.xpack.sql.expression.Attribute; import org.elasticsearch.xpack.sql.expression.Expressions; import org.elasticsearch.xpack.sql.expression.FieldAttribute; @@ -113,9 +113,9 @@ public class FieldAttributeTests extends ESTestCase { assertThat(attr.path(), is("some")); assertThat(attr.name(), is("some.string")); assertThat(attr.dataType(), is(DataType.TEXT)); - assertThat(attr.isInexact(), is(true)); + assertTrue(attr.getExactInfo().hasExact()); FieldAttribute exact = attr.exactAttribute(); - assertThat(exact.isInexact(), is(false)); + assertTrue(exact.getExactInfo().hasExact()); assertThat(exact.name(), is("some.string.typical")); assertThat(exact.dataType(), is(KEYWORD)); } @@ -125,9 +125,11 @@ public class FieldAttributeTests extends ESTestCase { assertThat(attr.path(), is("some")); assertThat(attr.name(), is("some.ambiguous")); assertThat(attr.dataType(), is(DataType.TEXT)); - assertThat(attr.isInexact(), is(true)); - MappingException me = expectThrows(MappingException.class, () -> attr.exactAttribute()); - assertThat(me.getMessage(), + assertFalse(attr.getExactInfo().hasExact()); + assertThat(attr.getExactInfo().errorMsg(), + is("Multiple exact keyword candidates available for [ambiguous]; specify which one to use")); + SqlIllegalArgumentException e = expectThrows(SqlIllegalArgumentException.class, () -> attr.exactAttribute()); + assertThat(e.getMessage(), is("Multiple exact keyword candidates available for [ambiguous]; specify which one to use")); } @@ -136,7 +138,7 @@ public class FieldAttributeTests extends ESTestCase { assertThat(attr.path(), is("some.string")); assertThat(attr.name(), is("some.string.normalized")); assertThat(attr.dataType(), is(KEYWORD)); - assertThat(attr.isInexact(), is(true)); + assertFalse(attr.getExactInfo().hasExact()); } public void testDottedFieldPath() { @@ -197,4 +199,4 @@ public class FieldAttributeTests extends ESTestCase { assertThat(attribute.qualifier(), is("test")); assertThat(attribute.name(), is("test.test")); } -} \ No newline at end of file +} diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/VerifierErrorMessagesTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/VerifierErrorMessagesTests.java index eec483ca219..dfeb44dfe21 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/VerifierErrorMessagesTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/VerifierErrorMessagesTests.java @@ -259,7 +259,7 @@ public class VerifierErrorMessagesTests extends ESTestCase { } public void testGroupByOrderByAliasedInSelectAllowed() { - LogicalPlan lp = accept("SELECT text t FROM test GROUP BY text ORDER BY t"); + LogicalPlan lp = accept("SELECT int i FROM test GROUP BY int ORDER BY i"); assertNotNull(lp); } @@ -292,6 +292,12 @@ public class VerifierErrorMessagesTests extends ESTestCase { assertNotNull(accept("SELECT dep.* FROM test")); } + public void testGroupByOnInexact() { + assertEquals("1:36: Field of data type [text] cannot be used for grouping; " + + "No keyword/multi-field defined exact matches for [text]; define one or use MATCH/QUERY instead", + error("SELECT COUNT(*) FROM test GROUP BY text")); + } + public void testGroupByOnNested() { assertEquals("1:38: Grouping isn't (yet) compatible with nested fields [dep.dep_id]", error("SELECT dep.dep_id FROM test GROUP BY dep.dep_id")); @@ -322,6 +328,18 @@ public class VerifierErrorMessagesTests extends ESTestCase { error("SELECT * FROM test WHERE unsupported > 1")); } + public void testTermEqualitOnInexact() { + assertEquals("1:26: [text = 'value'] cannot operate on first argument field of data type [text]: " + + "No keyword/multi-field defined exact matches for [text]; define one or use MATCH/QUERY instead", + error("SELECT * FROM test WHERE text = 'value'")); + } + + public void testTermEqualityOnAmbiguous() { + assertEquals("1:26: [some.ambiguous = 'value'] cannot operate on first argument field of data type [text]: " + + "Multiple exact keyword candidates available for [ambiguous]; specify which one to use", + error("SELECT * FROM test WHERE some.ambiguous = 'value'")); + } + public void testUnsupportedTypeInFunction() { assertEquals("1:12: Cannot use field [unsupported] type [ip_range] as is unsupported", error("SELECT ABS(unsupported) FROM test")); @@ -332,6 +350,12 @@ public class VerifierErrorMessagesTests extends ESTestCase { error("SELECT * FROM test ORDER BY unsupported")); } + public void testInexactFieldInOrder() { + assertEquals("1:29: ORDER BY cannot be applied to field of data type [text]: " + + "No keyword/multi-field defined exact matches for [text]; define one or use MATCH/QUERY instead", + error("SELECT * FROM test ORDER BY text")); + } + public void testGroupByOrderByAggregate() { accept("SELECT AVG(int) a FROM test GROUP BY bool ORDER BY a"); } @@ -416,65 +440,106 @@ public class VerifierErrorMessagesTests extends ESTestCase { } public void testNotSupportedAggregateOnDate() { - assertEquals("1:8: [AVG(date)] argument must be [numeric], found value [date] type [datetime]", + assertEquals("1:8: argument of [AVG(date)] must be [numeric], found value [date] type [datetime]", error("SELECT AVG(date) FROM test")); } - public void testInvalidTypeForStringFunction_WithOneArg() { - assertEquals("1:8: [LENGTH] argument must be [string], found value [1] type [integer]", + public void testInvalidTypeForStringFunction_WithOneArgString() { + assertEquals("1:8: argument of [LENGTH(1)] must be [string], found value [1] type [integer]", error("SELECT LENGTH(1)")); } + public void testInvalidTypeForStringFunction_WithOneArgNumeric() { + assertEquals("1:8: argument of [CHAR('foo')] must be [integer], found value ['foo'] type [keyword]", + error("SELECT CHAR('foo')")); + } + + public void testInvalidTypeForNestedStringFunctions_WithOneArg() { + assertEquals("1:14: argument of [CHAR('foo')] must be [integer], found value ['foo'] type [keyword]", + error("SELECT ASCII(CHAR('foo'))")); + } + public void testInvalidTypeForNumericFunction_WithOneArg() { - assertEquals("1:8: [COS] argument must be [numeric], found value ['foo'] type [keyword]", + assertEquals("1:8: argument of [COS('foo')] must be [numeric], found value ['foo'] type [keyword]", error("SELECT COS('foo')")); } public void testInvalidTypeForBooleanFunction_WithOneArg() { - assertEquals("1:8: [NOT 'foo'] argument must be [boolean], found value ['foo'] type [keyword]", + assertEquals("1:8: argument of [NOT 'foo'] must be [boolean], found value ['foo'] type [keyword]", error("SELECT NOT 'foo'")); } public void testInvalidTypeForStringFunction_WithTwoArgs() { - assertEquals("1:8: [CONCAT(1, 'bar')] first argument must be [string], found value [1] type [integer]", + assertEquals("1:8: first argument of [CONCAT] must be [string], found value [1] type [integer]", error("SELECT CONCAT(1, 'bar')")); - assertEquals("1:8: [CONCAT('foo', 2)] second argument must be [string], found value [2] type [integer]", + assertEquals("1:8: second argument of [CONCAT] must be [string], found value [2] type [integer]", error("SELECT CONCAT('foo', 2)")); } public void testInvalidTypeForNumericFunction_WithTwoArgs() { - assertEquals("1:8: [TRUNCATE('foo', 2)] first argument must be [numeric], found value ['foo'] type [keyword]", + assertEquals("1:8: first argument of [TRUNCATE('foo', 2)] must be [numeric], found value ['foo'] type [keyword]", error("SELECT TRUNCATE('foo', 2)")); - assertEquals("1:8: [TRUNCATE(1.2, 'bar')] second argument must be [numeric], found value ['bar'] type [keyword]", + assertEquals("1:8: second argument of [TRUNCATE(1.2, 'bar')] must be [numeric], found value ['bar'] type [keyword]", error("SELECT TRUNCATE(1.2, 'bar')")); } public void testInvalidTypeForBooleanFuntion_WithTwoArgs() { - assertEquals("1:8: [1 OR true] first argument must be [boolean], found value [1] type [integer]", + assertEquals("1:8: first argument of [1 OR true] must be [boolean], found value [1] type [integer]", error("SELECT 1 OR true")); - assertEquals("1:8: [true OR 2] second argument must be [boolean], found value [2] type [integer]", + assertEquals("1:8: second argument of [true OR 2] must be [boolean], found value [2] type [integer]", error("SELECT true OR 2")); } - public void testInvalidTypeForFunction_WithThreeArgs() { - assertEquals("1:8: [REPLACE(1, 'foo', 'bar')] first argument must be [string], found value [1] type [integer]", + public void testInvalidTypeForReplace() { + assertEquals("1:8: first argument of [REPLACE(1, 'foo', 'bar')] must be [string], found value [1] type [integer]", error("SELECT REPLACE(1, 'foo', 'bar')")); - assertEquals("1:8: [REPLACE('text', 2, 'bar')] second argument must be [string], found value [2] type [integer]", - error("SELECT REPLACE('text', 2, 'bar')")); - assertEquals("1:8: [REPLACE('text', 'foo', 3)] third argument must be [string], found value [3] type [integer]", - error("SELECT REPLACE('text', 'foo', 3)")); + assertEquals("1:8: [REPLACE(text, 'foo', 'bar')] cannot operate on first argument field of data type [text]: " + + "No keyword/multi-field defined exact matches for [text]; define one or use MATCH/QUERY instead", + error("SELECT REPLACE(text, 'foo', 'bar') FROM test")); + + assertEquals("1:8: second argument of [REPLACE('foo', 2, 'bar')] must be [string], found value [2] type [integer]", + error("SELECT REPLACE('foo', 2, 'bar')")); + assertEquals("1:8: [REPLACE('foo', text, 'bar')] cannot operate on second argument field of data type [text]: " + + "No keyword/multi-field defined exact matches for [text]; define one or use MATCH/QUERY instead", + error("SELECT REPLACE('foo', text, 'bar') FROM test")); + + assertEquals("1:8: third argument of [REPLACE('foo', 'bar', 3)] must be [string], found value [3] type [integer]", + error("SELECT REPLACE('foo', 'bar', 3)")); + assertEquals("1:8: [REPLACE('foo', 'bar', text)] cannot operate on third argument field of data type [text]: " + + "No keyword/multi-field defined exact matches for [text]; define one or use MATCH/QUERY instead", + error("SELECT REPLACE('foo', 'bar', text) FROM test")); + } + + public void testInvalidTypeForSubString() { + assertEquals("1:8: first argument of [SUBSTRING(1, 2, 3)] must be [string], found value [1] type [integer]", + error("SELECT SUBSTRING(1, 2, 3)")); + assertEquals("1:8: [SUBSTRING(text, 2, 3)] cannot operate on first argument field of data type [text]: " + + "No keyword/multi-field defined exact matches for [text]; define one or use MATCH/QUERY instead", + error("SELECT SUBSTRING(text, 2, 3) FROM test")); + + assertEquals("1:8: second argument of [SUBSTRING('foo', 'bar', 3)] must be [integer], found value ['bar'] type [keyword]", + error("SELECT SUBSTRING('foo', 'bar', 3)")); + + assertEquals("1:8: third argument of [SUBSTRING('foo', 2, 'bar')] must be [integer], found value ['bar'] type [keyword]", + error("SELECT SUBSTRING('foo', 2, 'bar')")); } public void testInvalidTypeForFunction_WithFourArgs() { - assertEquals("1:8: [INSERT(1, 1, 2, 'new')] first argument must be [string], found value [1] type [integer]", + assertEquals("1:8: first argument of [INSERT(1, 1, 2, 'new')] must be [string], found value [1] type [integer]", error("SELECT INSERT(1, 1, 2, 'new')")); - assertEquals("1:8: [INSERT('text', 'foo', 2, 'new')] second argument must be [numeric], found value ['foo'] type [keyword]", + assertEquals("1:8: second argument of [INSERT('text', 'foo', 2, 'new')] must be [numeric], found value ['foo'] type [keyword]", error("SELECT INSERT('text', 'foo', 2, 'new')")); - assertEquals("1:8: [INSERT('text', 1, 'bar', 'new')] third argument must be [numeric], found value ['bar'] type [keyword]", + assertEquals("1:8: third argument of [INSERT('text', 1, 'bar', 'new')] must be [numeric], found value ['bar'] type [keyword]", error("SELECT INSERT('text', 1, 'bar', 'new')")); - assertEquals("1:8: [INSERT('text', 1, 2, 3)] fourth argument must be [string], found value [3] type [integer]", + assertEquals("1:8: fourth argument of [INSERT('text', 1, 2, 3)] must be [string], found value [3] type [integer]", error("SELECT INSERT('text', 1, 2, 3)")); } + + public void testInvalidTypeForRegexMatch() { + assertEquals("1:26: [text LIKE 'foo'] cannot operate on field of data type [text]: " + + "No keyword/multi-field defined exact matches for [text]; define one or use MATCH/QUERY instead", + error("SELECT * FROM test WHERE text LIKE 'foo'")); + } public void testAllowCorrectFieldsInIncompatibleMappings() { assertNotNull(incompatibleAccept("SELECT languages FROM \"*\"")); @@ -616,32 +681,34 @@ public class VerifierErrorMessagesTests extends ESTestCase { } public void testErrorMessageForPercentileWithSecondArgBasedOnAField() { - assertEquals("1:8: Second argument of PERCENTILE must be a constant, received [ABS(int)]", + assertEquals("1:8: second argument of [PERCENTILE(int, ABS(int))] must be a constant, received [ABS(int)]", error("SELECT PERCENTILE(int, ABS(int)) FROM test")); } public void testErrorMessageForPercentileRankWithSecondArgBasedOnAField() { - assertEquals("1:8: Second argument of PERCENTILE_RANK must be a constant, received [ABS(int)]", + assertEquals("1:8: second argument of [PERCENTILE_RANK(int, ABS(int))] must be a constant, received [ABS(int)]", error("SELECT PERCENTILE_RANK(int, ABS(int)) FROM test")); } public void testTopHitsFirstArgConstant() { - assertEquals("1:8: First argument of [FIRST] must be a table column, found constant ['foo']", + assertEquals("1:8: first argument of [FIRST('foo', int)] must be a table column, found constant ['foo']", error("SELECT FIRST('foo', int) FROM test")); } public void testTopHitsSecondArgConstant() { - assertEquals("1:8: Second argument of [LAST] must be a table column, found constant [10]", + assertEquals("1:8: second argument of [LAST(int, 10)] must be a table column, found constant [10]", error("SELECT LAST(int, 10) FROM test")); } public void testTopHitsFirstArgTextWithNoKeyword() { - assertEquals("1:8: [FIRST] cannot operate on first argument field of data type [text]", + assertEquals("1:8: [FIRST(text)] cannot operate on first argument field of data type [text]: " + + "No keyword/multi-field defined exact matches for [text]; define one or use MATCH/QUERY instead", error("SELECT FIRST(text) FROM test")); } public void testTopHitsSecondArgTextWithNoKeyword() { - assertEquals("1:8: [LAST] cannot operate on second argument field of data type [text]", + assertEquals("1:8: [LAST(keyword, text)] cannot operate on second argument field of data type [text]: " + + "No keyword/multi-field defined exact matches for [text]; define one or use MATCH/QUERY instead", error("SELECT LAST(keyword, text) FROM test")); } @@ -671,4 +738,4 @@ public class VerifierErrorMessagesTests extends ESTestCase { public void testProjectUnresolvedAliasInFilter() { assertEquals("1:8: Unknown column [tni]", error("SELECT tni AS i FROM test WHERE i > 10 GROUP BY i")); } -} \ No newline at end of file +} diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryTranslatorTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryTranslatorTests.java index b26034f24c3..d3f58cf91fe 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryTranslatorTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryTranslatorTests.java @@ -17,7 +17,6 @@ import org.elasticsearch.xpack.sql.analysis.analyzer.Analyzer; import org.elasticsearch.xpack.sql.analysis.analyzer.Verifier; import org.elasticsearch.xpack.sql.analysis.index.EsIndex; import org.elasticsearch.xpack.sql.analysis.index.IndexResolution; -import org.elasticsearch.xpack.sql.analysis.index.MappingException; import org.elasticsearch.xpack.sql.expression.Expression; import org.elasticsearch.xpack.sql.expression.FieldAttribute; import org.elasticsearch.xpack.sql.expression.function.FunctionRegistry; @@ -110,16 +109,6 @@ public class QueryTranslatorTests extends ESTestCase { assertEquals("value", tq.value()); } - public void testTermEqualityAnalyzerAmbiguous() { - LogicalPlan p = plan("SELECT some.string FROM test WHERE some.ambiguous = 'value'"); - assertTrue(p instanceof Project); - p = ((Project) p).child(); - assertTrue(p instanceof Filter); - Expression condition = ((Filter) p).condition(); - // the message is checked elsewhere (in FieldAttributeTests) - expectThrows(MappingException.class, () -> QueryTranslator.toQuery(condition, false)); - } - public void testTermEqualityNotAnalyzed() { LogicalPlan p = plan("SELECT some.string FROM test WHERE int = 5"); assertTrue(p instanceof Project); @@ -640,7 +629,7 @@ public class QueryTranslatorTests extends ESTestCase { EsQueryExec eqe = (EsQueryExec) p; assertEquals(1, eqe.output().size()); assertEquals("FIRST(keyword)", eqe.output().get(0).qualifiedName()); - assertTrue(eqe.output().get(0).dataType() == DataType.KEYWORD); + assertEquals(DataType.KEYWORD, eqe.output().get(0).dataType()); assertThat(eqe.queryContainer().aggs().asAggBuilder().toString().replaceAll("\\s+", ""), endsWith("\"top_hits\":{\"from\":0,\"size\":1,\"version\":false,\"seq_no_primary_term\":false," + "\"explain\":false,\"docvalue_fields\":[{\"field\":\"keyword\"}]," + @@ -652,7 +641,7 @@ public class QueryTranslatorTests extends ESTestCase { EsQueryExec eqe = (EsQueryExec) p; assertEquals(1, eqe.output().size()); assertEquals("LAST(date)", eqe.output().get(0).qualifiedName()); - assertTrue(eqe.output().get(0).dataType() == DataType.DATETIME); + assertEquals(DataType.DATETIME, eqe.output().get(0).dataType()); assertThat(eqe.queryContainer().aggs().asAggBuilder().toString().replaceAll("\\s+", ""), endsWith("\"top_hits\":{\"from\":0,\"size\":1,\"version\":false,\"seq_no_primary_term\":false," + "\"explain\":false,\"docvalue_fields\":[{\"field\":\"date\",\"format\":\"epoch_millis\"}]," + @@ -667,7 +656,7 @@ public class QueryTranslatorTests extends ESTestCase { EsQueryExec eqe = (EsQueryExec) p; assertEquals(1, eqe.output().size()); assertEquals("FIRST(keyword, int)", eqe.output().get(0).qualifiedName()); - assertTrue(eqe.output().get(0).dataType() == DataType.KEYWORD); + assertEquals(DataType.KEYWORD, eqe.output().get(0).dataType()); assertThat(eqe.queryContainer().aggs().asAggBuilder().toString().replaceAll("\\s+", ""), endsWith("\"top_hits\":{\"from\":0,\"size\":1,\"version\":false,\"seq_no_primary_term\":false," + "\"explain\":false,\"docvalue_fields\":[{\"field\":\"keyword\"}]," + @@ -681,7 +670,7 @@ public class QueryTranslatorTests extends ESTestCase { EsQueryExec eqe = (EsQueryExec) p; assertEquals(1, eqe.output().size()); assertEquals("LAST(date, int)", eqe.output().get(0).qualifiedName()); - assertTrue(eqe.output().get(0).dataType() == DataType.DATETIME); + assertEquals(DataType.DATETIME, eqe.output().get(0).dataType()); assertThat(eqe.queryContainer().aggs().asAggBuilder().toString().replaceAll("\\s+", ""), endsWith("\"top_hits\":{\"from\":0,\"size\":1,\"version\":false,\"seq_no_primary_term\":false," + "\"explain\":false,\"docvalue_fields\":[{\"field\":\"date\",\"format\":\"epoch_millis\"}]," +