From 2c39bb352f719df4db96a64777c0a3a00f89f0eb 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 | 14 +- .../function/aggregate/PercentileRank.java | 14 +- .../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, 509 insertions(+), 282 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 c3a3d3ce305d5..6ac08259146a0 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 @@ -2338,25 +2338,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 52d53538bb2f8..47c53e772d5dd 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.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 @@ Collection verify(LogicalPlan plan) { */ 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 @@ private static boolean checkGroupByHavingHasOnlyAggs(Expression e, 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 c22b1213d09dc..47ffcf697328c 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 @@ 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 @@ private static void sorting(QueryContainer container, SearchSourceBuilder source // 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 @@ private static void sorting(QueryContainer container, SearchSourceBuilder source 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 648aff5254561..ca5e4b757567c 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 static List pipe(List 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 832af029df315..811cc299ccb97 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 FieldAttribute nestedParent() { 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 6a57c3275d4d1..267a8827d8cd6 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 Nullability nullable() { 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 0000000000000..61bc8ed44a9a8 --- /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 b432c5063a64b..177f598dc9a46 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 @@ 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 boolean equals(Object obj) { && 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 898c98463445e..cd03ea85e4558 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 java.util.List; +import static org.elasticsearch.xpack.sql.expression.TypeResolutions.isNumericOrDate; + /** * Find the maximum value in matching documents. */ @@ -48,7 +49,7 @@ protected TypeResolution resolveType() { 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 8652759fca486..07fa44769b2db 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 java.util.List; +import static org.elasticsearch.xpack.sql.expression.TypeResolutions.isNumericOrDate; + /** * Find the minimum value in matched documents. */ @@ -51,7 +52,7 @@ protected TypeResolution resolveType() { 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 bfe0d2ded7e34..21d5c23d23a5a 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 76c7bda320012..a0585f4c02176 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 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 Percentile replaceChildren(List newChildren) { @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 = isFoldable(percent, sourceText(), ParamOrdinal.SECOND); + if (resolution.unresolved()) { + return resolution; } - TypeResolution resolution = super.resolveType(); + resolution = super.resolveType(); if (resolution.unresolved()) { return resolution; } - return Expressions.typeMustBeNumeric(percent, sourceText(), ParamOrdinal.DEFAULT); + 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 b30b38a01b6c5..da8c487ff31f9 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 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 Expression replaceChildren(List newChildren) { @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 = isFoldable(value, sourceText(), ParamOrdinal.SECOND); + if (resolution.unresolved()) { + return resolution; } - TypeResolution resolution = super.resolveType(); + resolution = super.resolveType(); if (resolution.unresolved()) { return resolution; } - return Expressions.typeMustBeNumeric(value, sourceText(), ParamOrdinal.DEFAULT); + 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 227ca9b8db3d1..9364f5f4fc53f 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 DataType dataType() { @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 23061bfea1859..9cb752de5e69a 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.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 ZoneId zoneId() { @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 fa949007ef58a..cae78a42e55e9 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.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 @@ protected final NodeInfo info() { @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 f3369bf14a457..8230329454992 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 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 @@ protected TypeResolution resolveType() { 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 e0555ab0ea3bf..4389e1ac814a9 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.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 @@ protected TypeResolution resolveType() { 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 fd294564b642c..611e86507ee5b 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.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 @@ protected TypeResolution resolveType() { 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 @@ protected String scriptMethodName() { @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 d9f767d1ce81a..fac0646c2c611 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.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 BinaryStringNumericFunction(Source source, Expression left, Expression ri @Override protected TypeResolution resolveSecondParameterInputType(Expression e) { - return Expressions.typeMustBeNumeric(e, sourceText(), Expressions.ParamOrdinal.SECOND); + return isNumeric(e, sourceText(), Expressions.ParamOrdinal.SECOND); } @Override diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/BinaryStringStringFunction.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/BinaryStringStringFunction.java index 51189f6a4efff..eaa6ac428c85f 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/BinaryStringStringFunction.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/BinaryStringStringFunction.java @@ -10,6 +10,8 @@ import org.elasticsearch.xpack.sql.tree.Source; import org.elasticsearch.xpack.sql.type.DataType; +import static org.elasticsearch.xpack.sql.expression.TypeResolutions.isStringAndExact; + /** * A binary string function with two string parameters and a numeric result */ @@ -21,7 +23,7 @@ public BinaryStringStringFunction(Source source, Expression left, Expression rig @Override protected TypeResolution resolveSecondParameterInputType(Expression e) { - return Expressions.typeMustBeString(e, sourceText(), Expressions.ParamOrdinal.SECOND); + return isStringAndExact(e, sourceText(), Expressions.ParamOrdinal.SECOND); } @Override diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/Concat.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/Concat.java index 1f9833133a98a..4e461d919a93a 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/Concat.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/Concat.java @@ -17,6 +17,7 @@ import org.elasticsearch.xpack.sql.tree.Source; import org.elasticsearch.xpack.sql.type.DataType; +import static org.elasticsearch.xpack.sql.expression.TypeResolutions.isStringAndExact; import static org.elasticsearch.xpack.sql.expression.function.scalar.string.ConcatFunctionProcessor.process; import static org.elasticsearch.xpack.sql.expression.gen.script.ParamsBuilder.paramsBuilder; @@ -37,12 +38,12 @@ protected TypeResolution resolveType() { return new TypeResolution("Unresolved children"); } - TypeResolution sourceResolution = Expressions.typeMustBeString(left(), sourceText(), ParamOrdinal.FIRST); - if (sourceResolution.unresolved()) { - return sourceResolution; + TypeResolution resolution = isStringAndExact(left(), functionName(), ParamOrdinal.FIRST); + if (resolution.unresolved()) { + return resolution; } - return Expressions.typeMustBeString(right(), sourceText(), ParamOrdinal.SECOND); + return isStringAndExact(right(), functionName(), ParamOrdinal.SECOND); } @Override @@ -78,7 +79,7 @@ protected NodeInfo info() { @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/Insert.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/Insert.java index 8e6fc2052928e..8cff98d4c7c80 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/Insert.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/Insert.java @@ -21,6 +21,8 @@ import java.util.Locale; import static java.lang.String.format; +import static org.elasticsearch.xpack.sql.expression.TypeResolutions.isNumeric; +import static org.elasticsearch.xpack.sql.expression.TypeResolutions.isStringAndExact; import static org.elasticsearch.xpack.sql.expression.function.scalar.string.InsertFunctionProcessor.doProcess; import static org.elasticsearch.xpack.sql.expression.gen.script.ParamsBuilder.paramsBuilder; @@ -46,22 +48,22 @@ protected TypeResolution resolveType() { return new TypeResolution("Unresolved children"); } - TypeResolution sourceResolution = Expressions.typeMustBeString(source, sourceText(), ParamOrdinal.FIRST); + TypeResolution sourceResolution = isStringAndExact(source, sourceText(), ParamOrdinal.FIRST); if (sourceResolution.unresolved()) { return sourceResolution; } - - TypeResolution startResolution = Expressions.typeMustBeNumeric(start, sourceText(), ParamOrdinal.SECOND); + + TypeResolution startResolution = isNumeric(start, sourceText(), ParamOrdinal.SECOND); if (startResolution.unresolved()) { return startResolution; } - TypeResolution lengthResolution = Expressions.typeMustBeNumeric(length, sourceText(), ParamOrdinal.THIRD); + TypeResolution lengthResolution = isNumeric(length, sourceText(), ParamOrdinal.THIRD); if (lengthResolution.unresolved()) { return lengthResolution; } - return Expressions.typeMustBeString(replacement, sourceText(), ParamOrdinal.FOURTH); + return isStringAndExact(replacement, sourceText(), ParamOrdinal.FOURTH); } @Override @@ -119,7 +121,7 @@ private ScriptTemplate asScriptFrom(ScriptTemplate sourceScript, ScriptTemplate @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/Locate.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/Locate.java index 042ec1a736373..806e6fab8e465 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/Locate.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/Locate.java @@ -21,6 +21,8 @@ import java.util.Locale; import static java.lang.String.format; +import static org.elasticsearch.xpack.sql.expression.TypeResolutions.isNumeric; +import static org.elasticsearch.xpack.sql.expression.TypeResolutions.isStringAndExact; import static org.elasticsearch.xpack.sql.expression.function.scalar.string.LocateFunctionProcessor.doProcess; import static org.elasticsearch.xpack.sql.expression.gen.script.ParamsBuilder.paramsBuilder; @@ -48,19 +50,17 @@ protected TypeResolution resolveType() { return new TypeResolution("Unresolved children"); } - TypeResolution patternResolution = Expressions.typeMustBeString(pattern, sourceText(), ParamOrdinal.FIRST); + TypeResolution patternResolution = isStringAndExact(pattern, sourceText(), ParamOrdinal.FIRST); if (patternResolution.unresolved()) { return patternResolution; } - TypeResolution sourceResolution = Expressions.typeMustBeString(source, sourceText(), ParamOrdinal.SECOND); + TypeResolution sourceResolution = isStringAndExact(source, sourceText(), ParamOrdinal.SECOND); if (sourceResolution.unresolved()) { return sourceResolution; } - - return start == null ? - TypeResolution.TYPE_RESOLVED : - Expressions.typeMustBeNumeric(start, sourceText(), ParamOrdinal.THIRD); + + return start == null ? TypeResolution.TYPE_RESOLVED : isNumeric(start, sourceText(), ParamOrdinal.THIRD); } @Override @@ -80,7 +80,7 @@ protected NodeInfo info() { public boolean foldable() { return pattern.foldable() && source.foldable() - && (start == null? true : start.foldable()); + && (start == null || start.foldable()); } @Override @@ -122,7 +122,7 @@ private ScriptTemplate asScriptFrom(ScriptTemplate patternScript, ScriptTemplate @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/Replace.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/Replace.java index cd960f1f3b3ea..a1150fc5d38af 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/Replace.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/Replace.java @@ -21,6 +21,7 @@ import java.util.Locale; import static java.lang.String.format; +import static org.elasticsearch.xpack.sql.expression.TypeResolutions.isStringAndExact; import static org.elasticsearch.xpack.sql.expression.function.scalar.string.ReplaceFunctionProcessor.doProcess; import static org.elasticsearch.xpack.sql.expression.gen.script.ParamsBuilder.paramsBuilder; @@ -44,17 +45,17 @@ protected TypeResolution resolveType() { return new TypeResolution("Unresolved children"); } - TypeResolution sourceResolution = Expressions.typeMustBeString(source, sourceText(), ParamOrdinal.FIRST); + TypeResolution sourceResolution = isStringAndExact(source, sourceText(), ParamOrdinal.FIRST); if (sourceResolution.unresolved()) { return sourceResolution; } - TypeResolution patternResolution = Expressions.typeMustBeString(pattern, sourceText(), ParamOrdinal.SECOND); + TypeResolution patternResolution = isStringAndExact(pattern, sourceText(), ParamOrdinal.SECOND); if (patternResolution.unresolved()) { return patternResolution; } - return Expressions.typeMustBeString(replacement, sourceText(), ParamOrdinal.THIRD); + return isStringAndExact(replacement, sourceText(), ParamOrdinal.THIRD); } @Override @@ -107,7 +108,7 @@ private ScriptTemplate asScriptFrom(ScriptTemplate sourceScript, ScriptTemplate @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()); } @@ -124,4 +125,4 @@ public Expression replaceChildren(List newChildren) { return new Replace(source(), newChildren.get(0), newChildren.get(1), newChildren.get(2)); } -} \ No newline at end of file +} diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/Substring.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/Substring.java index 75178e73fce46..a341a6bb8c203 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/Substring.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/Substring.java @@ -21,6 +21,8 @@ import java.util.Locale; import static java.lang.String.format; +import static org.elasticsearch.xpack.sql.expression.TypeResolutions.isInteger; +import static org.elasticsearch.xpack.sql.expression.TypeResolutions.isStringAndExact; import static org.elasticsearch.xpack.sql.expression.function.scalar.string.SubstringFunctionProcessor.doProcess; import static org.elasticsearch.xpack.sql.expression.gen.script.ParamsBuilder.paramsBuilder; @@ -45,17 +47,17 @@ protected TypeResolution resolveType() { return new TypeResolution("Unresolved children"); } - TypeResolution sourceResolution = Expressions.typeMustBeString(source, sourceText(), ParamOrdinal.FIRST); + TypeResolution sourceResolution = isStringAndExact(source, sourceText(), ParamOrdinal.FIRST); if (sourceResolution.unresolved()) { return sourceResolution; } - TypeResolution startResolution = Expressions.typeMustBeNumeric(start, sourceText(), ParamOrdinal.SECOND); + TypeResolution startResolution = isInteger(start, sourceText(), ParamOrdinal.SECOND); if (startResolution.unresolved()) { return startResolution; } - return Expressions.typeMustBeNumeric(length, sourceText(), ParamOrdinal.THIRD); + return isInteger(length, sourceText(), ParamOrdinal.THIRD); } @Override @@ -107,7 +109,7 @@ protected ScriptTemplate asScriptFrom(ScriptTemplate sourceScript, ScriptTemplat @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()); } @@ -124,4 +126,4 @@ public Expression replaceChildren(List newChildren) { return new Substring(source(), newChildren.get(0), newChildren.get(1), newChildren.get(2)); } -} \ No newline at end of file +} diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/UnaryStringFunction.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/UnaryStringFunction.java index ef3944a9093a4..b2c72f0f5b6d7 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/UnaryStringFunction.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/UnaryStringFunction.java @@ -6,7 +6,6 @@ package org.elasticsearch.xpack.sql.expression.function.scalar.string; 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.FieldAttribute; import org.elasticsearch.xpack.sql.expression.function.scalar.UnaryScalarFunction; @@ -20,6 +19,7 @@ import java.util.Objects; import static java.lang.String.format; +import static org.elasticsearch.xpack.sql.expression.TypeResolutions.isStringAndExact; import static org.elasticsearch.xpack.sql.expression.gen.script.ParamsBuilder.paramsBuilder; public abstract class UnaryStringFunction extends UnaryScalarFunction { @@ -43,7 +43,7 @@ protected TypeResolution resolveType() { if (!childrenResolved()) { return new TypeResolution("Unresolved children"); } - return Expressions.typeMustBeString(field(), operation().toString(), ParamOrdinal.DEFAULT); + return isStringAndExact(field(), sourceText(), ParamOrdinal.DEFAULT); } @Override @@ -57,7 +57,7 @@ protected Processor makeProcessor() { public ScriptTemplate scriptWithField(FieldAttribute field) { //TODO change this to use _source instead of the exact form (aka field.keyword for text fields) 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/UnaryStringIntFunction.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/UnaryStringIntFunction.java index 5603a29d81d7c..7d9a64e59cdc2 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/UnaryStringIntFunction.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/UnaryStringIntFunction.java @@ -6,7 +6,6 @@ package org.elasticsearch.xpack.sql.expression.function.scalar.string; 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.FieldAttribute; import org.elasticsearch.xpack.sql.expression.function.scalar.UnaryScalarFunction; @@ -19,6 +18,7 @@ import java.util.Objects; import static java.lang.String.format; +import static org.elasticsearch.xpack.sql.expression.TypeResolutions.isInteger; import static org.elasticsearch.xpack.sql.expression.gen.script.ParamsBuilder.paramsBuilder; /** @@ -45,7 +45,7 @@ protected TypeResolution resolveType() { if (!childrenResolved()) { return new TypeResolution("Unresolved children"); } - return Expressions.typeMustBeInteger(field(), operation().toString(), ParamOrdinal.DEFAULT); + return isInteger(field(), sourceText(), ParamOrdinal.DEFAULT); } @Override diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/logical/BinaryLogic.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/logical/BinaryLogic.java index 930636657fc48..dfc5232cddd2d 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/logical/BinaryLogic.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/logical/BinaryLogic.java @@ -14,6 +14,8 @@ import org.elasticsearch.xpack.sql.tree.Source; import org.elasticsearch.xpack.sql.type.DataType; +import static org.elasticsearch.xpack.sql.expression.TypeResolutions.isBoolean; + public abstract class BinaryLogic extends BinaryOperator { protected BinaryLogic(Source source, Expression left, Expression right, BinaryLogicOperation operation) { @@ -27,7 +29,7 @@ public DataType dataType() { @Override protected TypeResolution resolveInputType(Expression e, Expressions.ParamOrdinal paramOrdinal) { - return Expressions.typeMustBeBoolean(e, sourceText(), paramOrdinal); + return isBoolean(e, sourceText(), paramOrdinal); } @Override diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/logical/Not.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/logical/Not.java index bbadaa6ef6430..88e20d187f343 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/logical/Not.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/logical/Not.java @@ -6,7 +6,6 @@ package org.elasticsearch.xpack.sql.expression.predicate.logical; 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.gen.processor.Processor; @@ -16,6 +15,8 @@ import org.elasticsearch.xpack.sql.tree.Source; import org.elasticsearch.xpack.sql.type.DataType; +import static org.elasticsearch.xpack.sql.expression.TypeResolutions.isBoolean; + public class Not extends UnaryScalarFunction { public Not(Source source, Expression child) { @@ -37,7 +38,7 @@ protected TypeResolution resolveType() { if (DataType.BOOLEAN == field().dataType()) { return TypeResolution.TYPE_RESOLVED; } - return Expressions.typeMustBeBoolean(field(), sourceText(), ParamOrdinal.DEFAULT); + return isBoolean(field(), sourceText(), ParamOrdinal.DEFAULT); } @Override diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/operator/arithmetic/ArithmeticOperation.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/operator/arithmetic/ArithmeticOperation.java index d1d28e3683863..157f34dc404a2 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/operator/arithmetic/ArithmeticOperation.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/operator/arithmetic/ArithmeticOperation.java @@ -14,6 +14,8 @@ import org.elasticsearch.xpack.sql.type.DataType; import org.elasticsearch.xpack.sql.type.DataTypeConversion; +import static org.elasticsearch.xpack.sql.expression.TypeResolutions.isNumeric; + public abstract class ArithmeticOperation extends BinaryOperator { private DataType dataType; @@ -24,7 +26,7 @@ protected ArithmeticOperation(Source source, Expression left, Expression right, @Override protected TypeResolution resolveInputType(Expression e, Expressions.ParamOrdinal paramOrdinal) { - return Expressions.typeMustBeNumeric(e, symbol(), paramOrdinal); + return isNumeric(e, sourceText(), paramOrdinal); } @Override @@ -44,4 +46,4 @@ public DataType dataType() { protected Pipe makePipe() { return new BinaryArithmeticPipe(source(), this, Expressions.pipe(left()), Expressions.pipe(right()), function()); } -} \ No newline at end of file +} diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/operator/arithmetic/Neg.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/operator/arithmetic/Neg.java index 3ada9a523a13c..a8c7f87d6380d 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/operator/arithmetic/Neg.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/operator/arithmetic/Neg.java @@ -6,7 +6,6 @@ package org.elasticsearch.xpack.sql.expression.predicate.operator.arithmetic; 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.gen.processor.Processor; @@ -16,6 +15,8 @@ import org.elasticsearch.xpack.sql.tree.Source; import org.elasticsearch.xpack.sql.type.DataType; +import static org.elasticsearch.xpack.sql.expression.TypeResolutions.isNumeric; + /** * Negation function (@{code -x}). */ @@ -37,7 +38,7 @@ protected Neg replaceChild(Expression newChild) { @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/predicate/operator/comparison/BinaryComparison.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/operator/comparison/BinaryComparison.java index 4e7473907c890..fdd33af077b40 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/operator/comparison/BinaryComparison.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/operator/comparison/BinaryComparison.java @@ -7,6 +7,7 @@ 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.gen.pipeline.Pipe; import org.elasticsearch.xpack.sql.expression.predicate.BinaryOperator; import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.BinaryComparisonProcessor.BinaryComparisonOperation; @@ -22,7 +23,7 @@ protected BinaryComparison(Source source, Expression left, Expression right, Bin @Override protected TypeResolution resolveInputType(Expression e, Expressions.ParamOrdinal paramOrdinal) { - return TypeResolution.TYPE_RESOLVED; + return TypeResolutions.isExact(e, sourceText(), paramOrdinal); } @Override @@ -43,4 +44,4 @@ protected Pipe makePipe() { public static Integer compare(Object left, Object right) { return Comparisons.compare(left, right); } -} \ No newline at end of file +} diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/operator/comparison/In.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/operator/comparison/In.java index f76523eaf0cd9..e687c9ac1baee 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/operator/comparison/In.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/operator/comparison/In.java @@ -5,12 +5,11 @@ */ package org.elasticsearch.xpack.sql.expression.predicate.operator.comparison; -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.Foldables; import org.elasticsearch.xpack.sql.expression.Nullability; +import org.elasticsearch.xpack.sql.expression.TypeResolutions; import org.elasticsearch.xpack.sql.expression.function.scalar.ScalarFunction; import org.elasticsearch.xpack.sql.expression.gen.pipeline.Pipe; import org.elasticsearch.xpack.sql.expression.gen.script.ScriptTemplate; @@ -109,13 +108,9 @@ protected Pipe makePipe() { @Override protected TypeResolution resolveType() { - if (value instanceof FieldAttribute) { - try { - ((FieldAttribute) value).exactAttribute(); - } catch (MappingException e) { - return new TypeResolution(format(null, "[{}] cannot operate on field of data type [{}]: {}", - functionName(), value().dataType().esType, e.getMessage())); - } + TypeResolution resolution = TypeResolutions.isExact(value, functionName(), Expressions.ParamOrdinal.DEFAULT); + if (resolution != TypeResolution.TYPE_RESOLVED) { + return resolution; } for (Expression ex : list) { diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/regex/RegexMatch.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/regex/RegexMatch.java index ed65b1fcaf9cb..b3c09c67fc6c7 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/regex/RegexMatch.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/regex/RegexMatch.java @@ -7,6 +7,7 @@ package org.elasticsearch.xpack.sql.expression.predicate.regex; import org.elasticsearch.xpack.sql.expression.Expression; +import org.elasticsearch.xpack.sql.expression.Expressions; import org.elasticsearch.xpack.sql.expression.Nullability; import org.elasticsearch.xpack.sql.expression.function.scalar.UnaryScalarFunction; import org.elasticsearch.xpack.sql.expression.gen.processor.Processor; @@ -14,6 +15,8 @@ import org.elasticsearch.xpack.sql.tree.Source; import org.elasticsearch.xpack.sql.type.DataType; +import static org.elasticsearch.xpack.sql.expression.TypeResolutions.isStringAndExact; + public abstract class RegexMatch extends UnaryScalarFunction { private final String pattern; @@ -36,6 +39,11 @@ public Nullability nullable() { return field().nullable(); } + @Override + protected TypeResolution resolveType() { + return isStringAndExact(field(), sourceText(), Expressions.ParamOrdinal.DEFAULT); + } + @Override public boolean foldable() { // right() is not directly foldable in any context but Like can fold it. diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/QueryTranslator.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/QueryTranslator.java index 57d1c016c608c..42becd95c6eae 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/QueryTranslator.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/QueryTranslator.java @@ -257,10 +257,7 @@ static GroupingContext groupBy(List groupings) { // change analyzed to non non-analyzed attributes if (exp instanceof FieldAttribute) { - FieldAttribute fa = (FieldAttribute) exp; - if (fa.isInexact()) { - ne = fa.exactAttribute(); - } + ne = ((FieldAttribute) exp).exactAttribute(); } // handle functions differently @@ -448,7 +445,7 @@ static String field(AggregateFunction af) { // COUNT(DISTINCT) uses cardinality aggregation which works on exact values (not changed by analyzers or normalizers) if (af instanceof Count && ((Count) af).distinct()) { // use the `keyword` version of the field, if there is one - return field.isInexact() ? field.exactAttribute().name() : field.name(); + return field.exactAttribute().name(); } return field.name(); } @@ -481,9 +478,7 @@ protected QueryTranslation asQuery(RegexMatch e, boolean onAggs) { String target = null; if (e.field() instanceof FieldAttribute) { - FieldAttribute fa = (FieldAttribute) e.field(); - inexact = fa.isInexact(); - target = nameOf(inexact ? fa.exactAttribute() : fa); + target = nameOf(((FieldAttribute) e.field()).exactAttribute()); } else { throw new SqlIllegalArgumentException("Scalar function ({}) not allowed (yet) as arguments for LIKE", Expressions.name(e.field())); @@ -683,12 +678,9 @@ private static Query translateQuery(BinaryComparison bc) { } if (bc instanceof Equals || bc instanceof NullEquals || bc instanceof NotEquals) { if (bc.left() instanceof FieldAttribute) { - FieldAttribute fa = (FieldAttribute) bc.left(); // equality should always be against an exact match // (which is important for strings) - if (fa.isInexact()) { - name = fa.exactAttribute().name(); - } + name = ((FieldAttribute) bc.left()).exactAttribute().name(); } Query query = new TermQuery(source, name, value); if (bc instanceof NotEquals) { @@ -726,7 +718,7 @@ protected QueryTranslation asQuery(In in, boolean onAggs) { if (in.value() instanceof FieldAttribute) { FieldAttribute fa = (FieldAttribute) in.value(); // equality should always be against an exact match (which is important for strings) - q = new TermsQuery(in.source(), fa.isInexact() ? fa.exactAttribute().name() : fa.name(), in.list()); + q = new TermsQuery(in.source(), fa.exactAttribute().name(), in.list()); } else { q = new ScriptQuery(in.source(), in.asScript()); } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/EsField.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/EsField.java index 47a2904adb7a7..52f531ba6e41e 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/EsField.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/EsField.java @@ -14,6 +14,7 @@ * SQL-related information about an index field */ public class EsField { + private final DataType esDataType; private final boolean aggregatable; private final Map properties; @@ -58,7 +59,9 @@ public Map getProperties() { /** * 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 int getPrecision() { } /** - * 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 boolean equals(Object o) { 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 59bb94c78c86e..79f8eb1c20c1f 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 boolean equals(Object obj) { @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 d40fa7b19af92..3b77608fc8bed 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 int getPrecision() { } @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 int hashCode() { 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 f1c596a301c54..4944a472e2104 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 TextEsField(String name, Map properties, boolean hasDocV @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 c88d676c223b6..2909c5f199053 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 String getOriginalType() { @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 607810efc666a..bc7b85b5392e9 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 void testExactKeyword() { 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 void testAmbiguousExactKeyword() { 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 void testNormalizedKeyword() { 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 void testFieldAmbiguity() { 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 eec483ca219b8..dfeb44dfe2165 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 void testGroupByNegativeOrdinal() { } 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 void testStarOnNested() { 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 void testUnsupportedTypeInFilter() { 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 void testUnsupportedTypeInOrder() { 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 void testInOnFieldTextWithNoKeyword() { } 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 void testScalarOfHistogramNotInGrouping() { } 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 void testAggregateAliasInFilter() { 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 b26034f24c382..d3f58cf91feaa 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.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 void testTermEqualityAnalyzer() { 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 void testTopHitsAggregationWithOneArg() { 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 void testTopHitsAggregationWithOneArg() { 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 void testTopHitsAggregationWithTwoArgs() { 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 void testTopHitsAggregationWithTwoArgs() { 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\"}]," +