From 62fee9e1fb9e7653a098eeb1c1997ee6c28efab3 Mon Sep 17 00:00:00 2001 From: Fang Xing Date: Wed, 24 Apr 2024 13:10:22 -0400 Subject: [PATCH 1/7] implicit cast from string to ip and version in binary comparison --- .../src/main/resources/ip.csv-spec | 10 ++++++++++ .../src/main/resources/version.csv-spec | 8 ++++++++ .../xpack/esql/analysis/Analyzer.java | 9 +++++++-- .../esql/type/EsqlDataTypeConverter.java | 4 ++-- .../elasticsearch/xpack/esql/CsvTests.java | 3 ++- .../esql/parser/StatementParserTests.java | 20 ++++++++++++++----- 6 files changed, 44 insertions(+), 10 deletions(-) diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/ip.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/ip.csv-spec index 8e0da1dd354ed..6c55380f55105 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/ip.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/ip.csv-spec @@ -432,3 +432,13 @@ required_feature: esql.agg_values [fe80::cae2:65ff:fece:feb9, fe80::cae2:65ff:fece:fec0, fe80::cae2:65ff:fece:fec1, fe81::cae2:65ff:fece:feb9, fe82::cae2:65ff:fece:fec0] | epsilon fe80::cae2:65ff:fece:feb9 | gamma ; + +implictCastingStringToIP +from hosts | where mv_first(ip0) == "127.0.0.1" | keep host, ip0; + +host:keyword | ip0:ip +alpha | 127.0.0.1 +beta | 127.0.0.1 +beta | 127.0.0.1 +beta | 127.0.0.1 +; diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/version.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/version.csv-spec index c5e42186d976f..14513d5df6a04 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/version.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/version.csv-spec @@ -370,3 +370,11 @@ version:version | name:keyword null | lllll 5.2.9 | mmmmm ; + +implictCastingStringToVersion +from apps | where version == "1.2.3.4" | sort name | keep name, version; + +name:keyword | version:version +aaaaa | 1.2.3.4 +hhhhh | 1.2.3.4 +; diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java index 02969ed56798f..777fe801012c0 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java @@ -105,6 +105,7 @@ import static org.elasticsearch.xpack.ql.type.DataTypes.LONG; import static org.elasticsearch.xpack.ql.type.DataTypes.NESTED; import static org.elasticsearch.xpack.ql.type.DataTypes.TEXT; +import static org.elasticsearch.xpack.ql.type.DataTypes.VERSION; public class Analyzer extends ParameterizedRuleExecutor { // marker list of attributes for plans that do not have any concrete fields to return, but have other computed columns to return @@ -846,14 +847,14 @@ private static Expression processBinaryOperator(BinaryOperator o) { if (left.dataType() == KEYWORD && left.foldable() - && (right.dataType().isNumeric() || right.dataType() == DATETIME) + && (isSupportedCastingForBinaryComparison(right.dataType())) && ((left instanceof EsqlScalarFunction) == false)) { targetDataType = right.dataType(); from = left; } if (right.dataType() == KEYWORD && right.foldable() - && (left.dataType().isNumeric() || left.dataType() == DATETIME) + && (isSupportedCastingForBinaryComparison(left.dataType())) && ((right instanceof EsqlScalarFunction) == false)) { targetDataType = left.dataType(); from = right; @@ -867,6 +868,10 @@ private static Expression processBinaryOperator(BinaryOperator o) { return childrenChanged ? o.replaceChildren(newChildren) : o; } + private static boolean isSupportedCastingForBinaryComparison(DataType type) { + return type.isNumeric() || type == DATETIME || type == IP || type == VERSION; + } + public static Expression castStringLiteral(Expression from, DataType target) { assert from.foldable(); try { diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/type/EsqlDataTypeConverter.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/type/EsqlDataTypeConverter.java index 7c0441443bf22..e4c7983d9a83a 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/type/EsqlDataTypeConverter.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/type/EsqlDataTypeConverter.java @@ -293,8 +293,8 @@ public static BytesRef stringToVersion(BytesRef field) { return new Version(field.utf8ToString()).toBytesRef(); } - public static Version stringToVersion(String field) { - return new Version(field); + public static BytesRef stringToVersion(String field) { + return new Version(field).toBytesRef(); } public static String versionToString(BytesRef field) { diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java index 573dbd20b39c5..b61de6c5bca08 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java @@ -37,6 +37,7 @@ import org.elasticsearch.tasks.CancellableTask; import org.elasticsearch.tasks.TaskId; import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.junit.annotations.TestLogging; import org.elasticsearch.threadpool.FixedExecutorBuilder; import org.elasticsearch.threadpool.TestThreadPool; import org.elasticsearch.threadpool.ThreadPool; @@ -139,7 +140,7 @@ * * To log the results logResults() should return "true". */ -// @TestLogging(value = "org.elasticsearch.xpack.esql:TRACE,org.elasticsearch.compute:TRACE", reason = "debug") +@TestLogging(value = "org.elasticsearch.xpack.esql:TRACE,org.elasticsearch.compute:TRACE", reason = "debug") public class CsvTests extends ESTestCase { private static final Logger LOGGER = LogManager.getLogger(CsvTests.class); diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java index 8901f94cd2cf6..1917bff25f0bb 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java @@ -7,6 +7,7 @@ package org.elasticsearch.xpack.esql.parser; +import org.apache.lucene.util.BytesRef; import org.elasticsearch.action.search.SearchRequest; import org.elasticsearch.common.Randomness; import org.elasticsearch.core.Tuple; @@ -49,6 +50,7 @@ import org.elasticsearch.xpack.ql.plan.logical.Project; import org.elasticsearch.xpack.ql.type.DataType; import org.elasticsearch.xpack.ql.type.DataTypes; +import org.elasticsearch.xpack.ql.util.StringUtils; import org.elasticsearch.xpack.versionfield.Version; import java.math.BigInteger; @@ -867,18 +869,19 @@ public void testUsageOfProject() { public void testInputParams() { LogicalPlan stm = statement( - "row x = ?, y = ?, a = ?, b = ?, c = ?", + "row x = ?, y = ?, a = ?, b = ?, c = ?, d = ?", List.of( new TypedParamValue("integer", 1), new TypedParamValue("keyword", "2"), new TypedParamValue("date_period", "2 days"), new TypedParamValue("time_duration", "4 hours"), - new TypedParamValue("version", "1.2.3") + new TypedParamValue("version", "1.2.3"), + new TypedParamValue("ip", "127.0.0.1") ) ); assertThat(stm, instanceOf(Row.class)); Row row = (Row) stm; - assertThat(row.fields().size(), is(5)); + assertThat(row.fields().size(), is(6)); NamedExpression field = row.fields().get(0); assertThat(field.name(), is("x")); @@ -908,8 +911,15 @@ public void testInputParams() { assertThat(field.name(), is("c")); assertThat(field, instanceOf(Alias.class)); alias = (Alias) field; - assertThat(alias.child().fold().getClass(), is(Version.class)); - assertThat(alias.child().fold().toString(), is("1.2.3")); + assertThat(alias.child().fold().getClass(), is(BytesRef.class)); + assertThat(alias.child().fold().toString(), is(new Version("1.2.3").toBytesRef().toString())); + + field = row.fields().get(5); + assertThat(field.name(), is("d")); + assertThat(field, instanceOf(Alias.class)); + alias = (Alias) field; + assertThat(alias.child().fold().getClass(), is(BytesRef.class)); + assertThat(alias.child().fold().toString(), is(StringUtils.parseIP("127.0.0.1").toString())); } public void testWrongIntervalParams() { From 9220a5040a1c6f6b13377a78c7a090cb17487770 Mon Sep 17 00:00:00 2001 From: Fang Xing Date: Wed, 24 Apr 2024 17:49:15 -0400 Subject: [PATCH 2/7] implicit casting for IN and binary comparison on ip, version, spatial, boolean --- .../src/main/resources/boolean.csv-spec | 16 +++++++ .../src/main/resources/date.csv-spec | 47 +++++++++++++++++++ .../src/main/resources/ip.csv-spec | 45 +++++++++++++++++- .../src/main/resources/math.csv-spec | 28 ----------- .../src/main/resources/spatial.csv-spec | 26 ++++++++++ .../src/main/resources/version.csv-spec | 40 +++++++++++++++- .../xpack/esql/analysis/Analyzer.java | 38 +++++++++++++-- .../xpack/esql/plugin/EsqlFeatures.java | 5 ++ .../elasticsearch/xpack/esql/CsvTests.java | 3 +- .../xpack/esql/analysis/AnalyzerTests.java | 16 ++++++- .../xpack/esql/analysis/VerifierTests.java | 2 +- 11 files changed, 227 insertions(+), 39 deletions(-) diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/boolean.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/boolean.csv-spec index 2713660cd47d8..c9e37ca70281e 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/boolean.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/boolean.csv-spec @@ -345,3 +345,19 @@ still_hired:boolean | job_positions:keyword [false, true] | Tech Lead [false, true] | null ; + +implicitCastingEqual +required_feature: esql.string_literal_auto_casting_8_15_0 +from employees | where still_hired == "true" | sort emp_no | keep emp_no | limit 1; + +emp_no:integer +10001 +; + +implicitCastingNotEqual +required_feature: esql.string_literal_auto_casting_8_15_0 +from employees | where still_hired != "true" | sort emp_no | keep emp_no | limit 1; + +emp_no:integer +10003 +; diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/date.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/date.csv-spec index 73b784663de8c..f978346818c13 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/date.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/date.csv-spec @@ -1041,3 +1041,50 @@ required_feature: esql.agg_values [1953-04-20T00:00:00Z, 1954-05-01T00:00:00Z] | Tech Lead [1955-01-21T00:00:00Z, 1957-05-23T00:00:00Z, 1959-12-03T00:00:00Z] | null ; + +implicitCastingEqual +required_feature: esql.string_literal_auto_casting +from employees | where birth_date == "1957-05-23T00:00:00Z" | keep emp_no, birth_date | sort emp_no; + +emp_no:integer | birth_date:datetime +10007 | 1957-05-23T00:00:00Z +; + +implicitCastingNotEqual +required_feature: esql.string_literal_auto_casting +from employees | where birth_date != "1957-05-23T00:00:00Z" | keep emp_no, birth_date | sort emp_no | limit 3; + +emp_no:integer | birth_date:datetime +10001 | 1953-09-02T00:00:00Z +10002 | 1964-06-02T00:00:00Z +10003 | 1959-12-03T00:00:00Z +; + +implicitCastingLessThanOrEqual +required_feature: esql.string_literal_auto_casting +from employees | where birth_date <= "1957-05-23T00:00:00Z" | keep emp_no, birth_date | sort emp_no | limit 3; + +emp_no:integer | birth_date:datetime +10001 | 1953-09-02T00:00:00Z +10004 | 1954-05-01T00:00:00Z +10005 | 1955-01-21T00:00:00Z +; + +implicitCastingGreaterThan +required_feature: esql.string_literal_auto_casting +from employees | where birth_date > "1957-05-23T00:00:00Z" | keep emp_no, birth_date | sort emp_no | limit 3; + +emp_no:integer | birth_date:datetime +10002 | 1964-06-02T00:00:00Z +10003 | 1959-12-03T00:00:00Z +10008 | 1958-02-19T00:00:00Z +; + +implicitCastingIn +required_feature: esql.string_literal_auto_casting_8_15_0 +from employees | where birth_date in ( "1957-05-23T00:00:00Z", "1960-02-20T00:00:00Z" ) | keep emp_no, birth_date | sort emp_no; + +emp_no:integer | birth_date:datetime +10007 | 1957-05-23T00:00:00Z +10021 | 1960-02-20T00:00:00Z +; diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/ip.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/ip.csv-spec index 6c55380f55105..225d74296b5f7 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/ip.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/ip.csv-spec @@ -433,7 +433,8 @@ required_feature: esql.agg_values fe80::cae2:65ff:fece:feb9 | gamma ; -implictCastingStringToIP +implictCastingEqual +required_feature: esql.string_literal_auto_casting_8_15_0 from hosts | where mv_first(ip0) == "127.0.0.1" | keep host, ip0; host:keyword | ip0:ip @@ -442,3 +443,45 @@ beta | 127.0.0.1 beta | 127.0.0.1 beta | 127.0.0.1 ; + +implictCastingNotEqual +required_feature: esql.string_literal_auto_casting_8_15_0 +from hosts | where mv_first(ip0) != "127.0.0.1" | keep host, ip0 | sort host, ip0 | limit 3; + +host:keyword | ip0:ip +alpha | ::1 +epsilon | [fe80::cae2:65ff:fece:feb9, fe80::cae2:65ff:fece:fec0, fe80::cae2:65ff:fece:fec1] +epsilon | [fe81::cae2:65ff:fece:feb9, fe82::cae2:65ff:fece:fec0] +; + +implictCastingGreaterThan +required_feature: esql.string_literal_auto_casting_8_15_0 +from hosts | where mv_first(ip0) > "127.0.0.1" | keep host, ip0 | sort host, ip0 | limit 3; + +host:keyword | ip0:ip +epsilon | [fe80::cae2:65ff:fece:feb9, fe80::cae2:65ff:fece:fec0, fe80::cae2:65ff:fece:fec1] +epsilon | [fe81::cae2:65ff:fece:feb9, fe82::cae2:65ff:fece:fec0] +gamma | fe80::cae2:65ff:fece:feb9 +; + +implictCastingLessThanOrEqual +required_feature: esql.string_literal_auto_casting_8_15_0 +from hosts | where mv_first(ip0) <= "127.0.0.1" | keep host, ip0 | sort host, ip0 | limit 3; + +host:keyword | ip0:ip +alpha | ::1 +alpha | 127.0.0.1 +beta | 127.0.0.1 +; + +implictCastingIn +required_feature: esql.string_literal_auto_casting_8_15_0 +from hosts | where mv_first(ip0) in ( "127.0.0.1", "::1") | keep host, ip0 | sort host, ip0; + +host:keyword | ip0:ip +alpha | ::1 +alpha | 127.0.0.1 +beta | 127.0.0.1 +beta | 127.0.0.1 +beta | 127.0.0.1 +; diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/math.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/math.csv-spec index 399e1b5dc791b..e0604acbcce1d 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/math.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/math.csv-spec @@ -1424,34 +1424,6 @@ number:double | abs_number:double -1.0 | 10.0 ; -arithmeticOperationWithString -required_feature: esql.string_literal_auto_casting - -from employees -| eval s1 = salary + "10000", s2 = height * "2", s3 = avg_worked_seconds / "2", s4 = languages - "1" -| sort emp_no -| keep emp_no, salary, s1, height, s2, avg_worked_seconds, s3, languages, s4 -| limit 2; - -emp_no:integer | salary:integer | s1:integer | height:double | s2:double | avg_worked_seconds:long | s3:long | languages:integer | s4:integer -10001 | 57305 | 67305 | 2.03 | 4.06 | 268728049 | 134364024 | 2 | 1 -10002 | 56371 | 66371 | 2.08 | 4.16 | 328922887 | 164461443 | 5 | 4 -; - -arithmeticOperationNestedWithString -required_feature: esql.string_literal_auto_casting - -from employees -| eval x = languages + "1", y = x * 2 -| sort emp_no -| keep emp_no, languages, x, y -| limit 2; - -emp_no: integer | languages:integer | x:integer | y:integer -10001 | 2 | 3 | 6 -10002 | 5 | 6 | 12 -; - functionUnderArithmeticOperationAggString required_feature: esql.string_literal_auto_casting diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/spatial.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/spatial.csv-spec index 26fcca423d28d..b925f93c8d123 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/spatial.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/spatial.csv-spec @@ -1733,3 +1733,29 @@ wkt:keyword |pt:cartesian_point "POINT(111)" |null // end::to_cartesianpoint-str-parse-error-result[] ; + +implicitCastingEqual +required_feature: esql.string_literal_auto_casting_8_15_0 +from airports | where location == "POINT(42.97109630194 14.7552534413725)" | sort name | keep name, location | limit 2; + +name:text | location:geo_point +Hodeidah Int'l | POINT(42.97109630194 14.7552534413725) +; + +implicitCastingNotEqual +required_feature: esql.string_literal_auto_casting_8_15_0 +from airports | where city_location != "POINT(78.178 26.2215)" | sort name | keep name, city_location | limit 2; + +name:text | city_location:geo_point +Aba Tenna D. Yilma Int'l | POINT(41.8667 9.6) +Abdul Rachman Saleh | POINT(112.62 -7.98) +; + +implicitCastingIn +required_feature: esql.string_literal_auto_casting_8_15_0 +from airports | where city_location in ( "POINT(78.178 26.2215)", "POINT(48.6692 31.3203)" ) | sort name | keep name, city_location | limit 2; + +name:text | city_location:geo_point +Ahwaz | POINT(48.6692 31.3203) +Gwalior | POINT(78.178 26.2215) +; diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/version.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/version.csv-spec index 14513d5df6a04..d3ab46f321590 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/version.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/version.csv-spec @@ -371,10 +371,48 @@ version:version | name:keyword 5.2.9 | mmmmm ; -implictCastingStringToVersion +implictCastingEqual +required_feature: esql.string_literal_auto_casting_8_15_0 from apps | where version == "1.2.3.4" | sort name | keep name, version; name:keyword | version:version aaaaa | 1.2.3.4 hhhhh | 1.2.3.4 ; + +implictCastingNotEqual +required_feature: esql.string_literal_auto_casting_8_15_0 +from apps | where version != "1.2.3.4" | sort name, version | keep name, version | limit 2; + +name:keyword | version:version +aaaaa | 1 +bbbbb | 2.1 +; + +implictCastingGreaterThan +required_feature: esql.string_literal_auto_casting_8_15_0 +from apps | where version > "1.2.3.4" | sort name, version | keep name, version | limit 2; + +name:keyword | version:version +bbbbb | 2.1 +ccccc | 2.3.4 +; + +implictCastingLessThanOrEqual +required_feature: esql.string_literal_auto_casting_8_15_0 +from apps | where version <= "1.2.3.4" | sort name, version | keep name, version | limit 2; + +name:keyword | version:version +aaaaa | 1 +aaaaa | 1.2.3.4 +; + +implictCastingIn +required_feature: esql.string_literal_auto_casting_8_15_0 +from apps | where version in ( "1.2.3.4", "bad" ) | sort name | keep name, version; + +name:keyword | version:version +aaaaa | 1.2.3.4 +hhhhh | 1.2.3.4 +iiiii | bad +; diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java index 777fe801012c0..cac9a073a6b5f 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java @@ -18,6 +18,7 @@ import org.elasticsearch.xpack.esql.expression.function.UnsupportedAttribute; import org.elasticsearch.xpack.esql.expression.function.scalar.EsqlScalarFunction; import org.elasticsearch.xpack.esql.expression.predicate.operator.arithmetic.EsqlArithmeticOperation; +import org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.In; import org.elasticsearch.xpack.esql.plan.logical.Drop; import org.elasticsearch.xpack.esql.plan.logical.Enrich; import org.elasticsearch.xpack.esql.plan.logical.EsqlAggregate; @@ -96,6 +97,8 @@ import static org.elasticsearch.xpack.esql.stats.FeatureMetric.LIMIT; import static org.elasticsearch.xpack.esql.type.EsqlDataTypes.GEO_POINT; import static org.elasticsearch.xpack.esql.type.EsqlDataTypes.GEO_SHAPE; +import static org.elasticsearch.xpack.esql.type.EsqlDataTypes.isSpatial; +import static org.elasticsearch.xpack.ql.type.DataTypes.BOOLEAN; import static org.elasticsearch.xpack.ql.type.DataTypes.DATETIME; import static org.elasticsearch.xpack.ql.type.DataTypes.DOUBLE; import static org.elasticsearch.xpack.ql.type.DataTypes.FLOAT; @@ -803,6 +806,9 @@ private static Expression cast(ScalarFunction f, EsqlFunctionRegistry registry) if (f instanceof EsqlArithmeticOperation || f instanceof BinaryComparison) { return processBinaryOperator((BinaryOperator) f); } + if (f instanceof In in) { + return processIn(in); + } return f; } @@ -847,14 +853,14 @@ private static Expression processBinaryOperator(BinaryOperator o) { if (left.dataType() == KEYWORD && left.foldable() - && (isSupportedCastingForBinaryComparison(right.dataType())) + && (canCastTo(right.dataType())) && ((left instanceof EsqlScalarFunction) == false)) { targetDataType = right.dataType(); from = left; } if (right.dataType() == KEYWORD && right.foldable() - && (isSupportedCastingForBinaryComparison(left.dataType())) + && (canCastTo(left.dataType())) && ((right instanceof EsqlScalarFunction) == false)) { targetDataType = left.dataType(); from = right; @@ -868,8 +874,32 @@ private static Expression processBinaryOperator(BinaryOperator o) { return childrenChanged ? o.replaceChildren(newChildren) : o; } - private static boolean isSupportedCastingForBinaryComparison(DataType type) { - return type.isNumeric() || type == DATETIME || type == IP || type == VERSION; + private static Expression processIn(In in) { + Expression left = in.value(); + List right = in.list(); + + if (left.resolved() == false || canCastTo(left.dataType()) == false) { + return in; + } + List newChildren = new ArrayList<>(right.size() + 1); + boolean childrenChanged = false; + + for (int i = 0; i < right.size(); i++) { + Expression value = right.get(i); + if (value.dataType() == KEYWORD) { + Expression e = castStringLiteral(value, left.dataType()); + newChildren.add(e); + childrenChanged = true; + } else { + newChildren.add(value); + } + } + newChildren.add(left); + return childrenChanged ? in.replaceChildren(newChildren) : in; + } + + private static boolean canCastTo(DataType type) { + return type == DATETIME || type == IP || type == VERSION || type == BOOLEAN || isSpatial(type); } public static Expression castStringLiteral(Expression from, DataType target) { diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/EsqlFeatures.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/EsqlFeatures.java index ff157c0fe3e0b..bbfb40351dfa8 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/EsqlFeatures.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/EsqlFeatures.java @@ -115,6 +115,11 @@ public class EsqlFeatures implements FeatureSpecification { */ public static final NodeFeature CASTING_OPERATOR = new NodeFeature("esql.casting_operator"); + /** + * Cast string literals to a desired data type for IN predicate and more types for BinaryComparison. + */ + public static final NodeFeature STRING_LITERAL_AUTO_CASTING_815 = new NodeFeature("esql.string_literal_auto_casting_8_15_0"); + @Override public Set getFeatures() { return Set.of( diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java index b61de6c5bca08..573dbd20b39c5 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java @@ -37,7 +37,6 @@ import org.elasticsearch.tasks.CancellableTask; import org.elasticsearch.tasks.TaskId; import org.elasticsearch.test.ESTestCase; -import org.elasticsearch.test.junit.annotations.TestLogging; import org.elasticsearch.threadpool.FixedExecutorBuilder; import org.elasticsearch.threadpool.TestThreadPool; import org.elasticsearch.threadpool.ThreadPool; @@ -140,7 +139,7 @@ * * To log the results logResults() should return "true". */ -@TestLogging(value = "org.elasticsearch.xpack.esql:TRACE,org.elasticsearch.compute:TRACE", reason = "debug") +// @TestLogging(value = "org.elasticsearch.xpack.esql:TRACE,org.elasticsearch.compute:TRACE", reason = "debug") public class CsvTests extends ESTestCase { private static final Logger LOGGER = LogManager.getLogger(CsvTests.class); diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java index 7a85ca1628048..bcfb7997eed65 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java @@ -1003,7 +1003,13 @@ public void testCompareIntToString() { from test | where emp_no COMPARISON "foo" """.replace("COMPARISON", comparison))); - assertThat(e.getMessage(), containsString("Cannot convert string [foo] to [INTEGER]".replace("COMPARISON", comparison))); + assertThat( + e.getMessage(), + containsString( + "first argument of [emp_no COMPARISON \"foo\"] is [numeric] so second argument must also be [numeric] but was [keyword]" + .replace("COMPARISON", comparison) + ) + ); } } @@ -1013,7 +1019,13 @@ public void testCompareStringToInt() { from test | where "foo" COMPARISON emp_no """.replace("COMPARISON", comparison))); - assertThat(e.getMessage(), containsString("Cannot convert string [foo] to [INTEGER]".replace("COMPARISON", comparison))); + assertThat( + e.getMessage(), + containsString( + "first argument of [\"foo\" COMPARISON emp_no] is [keyword] so second argument must also be [keyword] but was [integer]" + .replace("COMPARISON", comparison) + ) + ); } } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java index 8d9140cdda5f4..41aea9fe15642 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java @@ -385,7 +385,7 @@ public void testSumOnDate() { public void testWrongInputParam() { assertEquals( - "1:29: Cannot convert string [foo] to [INTEGER], error [Cannot parse number [foo]]", + "1:19: first argument of [emp_no == ?] is [numeric] so second argument must also be [numeric] but was [keyword]", error("from test | where emp_no == ?", "foo") ); From 4f2806225350e7a18659ee28d0b0f4fc010519bf Mon Sep 17 00:00:00 2001 From: Fang Xing Date: Wed, 24 Apr 2024 18:44:35 -0400 Subject: [PATCH 3/7] remove cast to spatial from binary comparison and in --- .../src/main/resources/date.csv-spec | 6 ++--- .../src/main/resources/spatial.csv-spec | 26 ------------------- .../xpack/esql/analysis/Analyzer.java | 4 +-- .../xpack/esql/plugin/EsqlFeatures.java | 1 + 4 files changed, 6 insertions(+), 31 deletions(-) diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/date.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/date.csv-spec index f978346818c13..c701b27bcd16c 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/date.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/date.csv-spec @@ -1062,7 +1062,7 @@ emp_no:integer | birth_date:datetime implicitCastingLessThanOrEqual required_feature: esql.string_literal_auto_casting -from employees | where birth_date <= "1957-05-23T00:00:00Z" | keep emp_no, birth_date | sort emp_no | limit 3; +from employees | where birth_date <= "1957-05-20T00:00:00Z" | keep emp_no, birth_date | sort emp_no | limit 3; emp_no:integer | birth_date:datetime 10001 | 1953-09-02T00:00:00Z @@ -1072,7 +1072,7 @@ emp_no:integer | birth_date:datetime implicitCastingGreaterThan required_feature: esql.string_literal_auto_casting -from employees | where birth_date > "1957-05-23T00:00:00Z" | keep emp_no, birth_date | sort emp_no | limit 3; +from employees | where birth_date > "1957-05-24T00:00:00Z" | keep emp_no, birth_date | sort emp_no | limit 3; emp_no:integer | birth_date:datetime 10002 | 1964-06-02T00:00:00Z @@ -1082,7 +1082,7 @@ emp_no:integer | birth_date:datetime implicitCastingIn required_feature: esql.string_literal_auto_casting_8_15_0 -from employees | where birth_date in ( "1957-05-23T00:00:00Z", "1960-02-20T00:00:00Z" ) | keep emp_no, birth_date | sort emp_no; +from employees | where birth_date in ( "1957-05-23T00:00:00.000Z", "1960-02-20T00:00:00.000Z" ) | keep emp_no, birth_date | sort emp_no; emp_no:integer | birth_date:datetime 10007 | 1957-05-23T00:00:00Z diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/spatial.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/spatial.csv-spec index b925f93c8d123..26fcca423d28d 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/spatial.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/spatial.csv-spec @@ -1733,29 +1733,3 @@ wkt:keyword |pt:cartesian_point "POINT(111)" |null // end::to_cartesianpoint-str-parse-error-result[] ; - -implicitCastingEqual -required_feature: esql.string_literal_auto_casting_8_15_0 -from airports | where location == "POINT(42.97109630194 14.7552534413725)" | sort name | keep name, location | limit 2; - -name:text | location:geo_point -Hodeidah Int'l | POINT(42.97109630194 14.7552534413725) -; - -implicitCastingNotEqual -required_feature: esql.string_literal_auto_casting_8_15_0 -from airports | where city_location != "POINT(78.178 26.2215)" | sort name | keep name, city_location | limit 2; - -name:text | city_location:geo_point -Aba Tenna D. Yilma Int'l | POINT(41.8667 9.6) -Abdul Rachman Saleh | POINT(112.62 -7.98) -; - -implicitCastingIn -required_feature: esql.string_literal_auto_casting_8_15_0 -from airports | where city_location in ( "POINT(78.178 26.2215)", "POINT(48.6692 31.3203)" ) | sort name | keep name, city_location | limit 2; - -name:text | city_location:geo_point -Ahwaz | POINT(48.6692 31.3203) -Gwalior | POINT(78.178 26.2215) -; diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java index cac9a073a6b5f..9f1ca6361753f 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java @@ -97,7 +97,6 @@ import static org.elasticsearch.xpack.esql.stats.FeatureMetric.LIMIT; import static org.elasticsearch.xpack.esql.type.EsqlDataTypes.GEO_POINT; import static org.elasticsearch.xpack.esql.type.EsqlDataTypes.GEO_SHAPE; -import static org.elasticsearch.xpack.esql.type.EsqlDataTypes.isSpatial; import static org.elasticsearch.xpack.ql.type.DataTypes.BOOLEAN; import static org.elasticsearch.xpack.ql.type.DataTypes.DATETIME; import static org.elasticsearch.xpack.ql.type.DataTypes.DOUBLE; @@ -870,6 +869,7 @@ private static Expression processBinaryOperator(BinaryOperator o) { newChildren.add(from == left ? e : left); newChildren.add(from == right ? e : right); childrenChanged = true; + System.err.println("cast string to " + targetDataType.toString() + " : from : " + from.fold() + " : to : " + e.fold()); } return childrenChanged ? o.replaceChildren(newChildren) : o; } @@ -899,7 +899,7 @@ private static Expression processIn(In in) { } private static boolean canCastTo(DataType type) { - return type == DATETIME || type == IP || type == VERSION || type == BOOLEAN || isSpatial(type); + return type == DATETIME || type == IP || type == VERSION || type == BOOLEAN; } public static Expression castStringLiteral(Expression from, DataType target) { diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/EsqlFeatures.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/EsqlFeatures.java index bbfb40351dfa8..969ef228c6bae 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/EsqlFeatures.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/EsqlFeatures.java @@ -137,6 +137,7 @@ public Set getFeatures() { ST_CONTAINS_WITHIN, ST_DISJOINT, STRING_LITERAL_AUTO_CASTING, + STRING_LITERAL_AUTO_CASTING_815, CASTING_OPERATOR ); } From 229da5ecea30bd52d166df2da83bed766a619802 Mon Sep 17 00:00:00 2001 From: Fang Xing Date: Wed, 24 Apr 2024 21:07:30 -0400 Subject: [PATCH 4/7] remove unstable tests --- .../src/main/resources/date.csv-spec | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/date.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/date.csv-spec index 4b643f9afc64e..721cff076aeaa 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/date.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/date.csv-spec @@ -1042,14 +1042,6 @@ required_feature: esql.agg_values [1955-01-21T00:00:00Z, 1957-05-23T00:00:00Z, 1959-12-03T00:00:00Z] | null ; -implicitCastingEqual -required_feature: esql.string_literal_auto_casting -from employees | where birth_date == "1957-05-23T00:00:00Z" | keep emp_no, birth_date | sort emp_no; - -emp_no:integer | birth_date:datetime -10007 | 1957-05-23T00:00:00Z -; - implicitCastingNotEqual required_feature: esql.string_literal_auto_casting from employees | where birth_date != "1957-05-23T00:00:00Z" | keep emp_no, birth_date | sort emp_no | limit 3; @@ -1079,12 +1071,3 @@ emp_no:integer | birth_date:datetime 10003 | 1959-12-03T00:00:00Z 10008 | 1958-02-19T00:00:00Z ; - -implicitCastingIn -required_feature: esql.string_literal_auto_casting_8_15_0 -from employees | where birth_date in ( "1957-05-23T00:00:00Z", "1960-02-20T00:00:00Z" ) | keep emp_no, birth_date | sort emp_no; - -emp_no:integer | birth_date:datetime -10007 | 1957-05-23T00:00:00Z -10021 | 1960-02-20T00:00:00Z -; From ae3e516d88fea373fb44e71fb81ded333e1bed94 Mon Sep 17 00:00:00 2001 From: Fang Xing Date: Wed, 1 May 2024 09:40:57 -0400 Subject: [PATCH 5/7] spotless --- .../java/org/elasticsearch/xpack/esql/plugin/EsqlFeatures.java | 1 - 1 file changed, 1 deletion(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/EsqlFeatures.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/EsqlFeatures.java index bd3808d867007..323931fb16d3c 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/EsqlFeatures.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/EsqlFeatures.java @@ -131,7 +131,6 @@ public class EsqlFeatures implements FeatureSpecification { */ public static final NodeFeature STRING_LITERAL_AUTO_CASTING_8_15 = new NodeFeature("esql.string_literal_auto_casting_8_15_0"); - @Override public Set getFeatures() { return Set.of( From aed74ac693e2280970fc4e27711851d96b176902 Mon Sep 17 00:00:00 2001 From: Fang Xing Date: Thu, 2 May 2024 10:28:40 -0400 Subject: [PATCH 6/7] remove version from feature name --- .../testFixtures/src/main/resources/boolean.csv-spec | 4 ++-- .../qa/testFixtures/src/main/resources/ip.csv-spec | 10 +++++----- .../testFixtures/src/main/resources/version.csv-spec | 10 +++++----- .../elasticsearch/xpack/esql/plugin/EsqlFeatures.java | 4 ++-- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/boolean.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/boolean.csv-spec index c9e37ca70281e..3e88ed787f28f 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/boolean.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/boolean.csv-spec @@ -347,7 +347,7 @@ still_hired:boolean | job_positions:keyword ; implicitCastingEqual -required_feature: esql.string_literal_auto_casting_8_15_0 +required_feature: esql.string_literal_auto_casting_to_ip from employees | where still_hired == "true" | sort emp_no | keep emp_no | limit 1; emp_no:integer @@ -355,7 +355,7 @@ emp_no:integer ; implicitCastingNotEqual -required_feature: esql.string_literal_auto_casting_8_15_0 +required_feature: esql.string_literal_auto_casting_to_ip from employees | where still_hired != "true" | sort emp_no | keep emp_no | limit 1; emp_no:integer diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/ip.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/ip.csv-spec index 225d74296b5f7..2902d1ffc1cb5 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/ip.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/ip.csv-spec @@ -434,7 +434,7 @@ fe80::cae2:65ff:fece:feb9 | gamma ; implictCastingEqual -required_feature: esql.string_literal_auto_casting_8_15_0 +required_feature: esql.string_literal_auto_casting_to_ip from hosts | where mv_first(ip0) == "127.0.0.1" | keep host, ip0; host:keyword | ip0:ip @@ -445,7 +445,7 @@ beta | 127.0.0.1 ; implictCastingNotEqual -required_feature: esql.string_literal_auto_casting_8_15_0 +required_feature: esql.string_literal_auto_casting_to_ip from hosts | where mv_first(ip0) != "127.0.0.1" | keep host, ip0 | sort host, ip0 | limit 3; host:keyword | ip0:ip @@ -455,7 +455,7 @@ epsilon | [fe81::cae2:65ff:fece:feb9, fe82::cae2:65ff:fece:fec0] ; implictCastingGreaterThan -required_feature: esql.string_literal_auto_casting_8_15_0 +required_feature: esql.string_literal_auto_casting_to_ip from hosts | where mv_first(ip0) > "127.0.0.1" | keep host, ip0 | sort host, ip0 | limit 3; host:keyword | ip0:ip @@ -465,7 +465,7 @@ gamma | fe80::cae2:65ff:fece:feb9 ; implictCastingLessThanOrEqual -required_feature: esql.string_literal_auto_casting_8_15_0 +required_feature: esql.string_literal_auto_casting_to_ip from hosts | where mv_first(ip0) <= "127.0.0.1" | keep host, ip0 | sort host, ip0 | limit 3; host:keyword | ip0:ip @@ -475,7 +475,7 @@ beta | 127.0.0.1 ; implictCastingIn -required_feature: esql.string_literal_auto_casting_8_15_0 +required_feature: esql.string_literal_auto_casting_to_ip from hosts | where mv_first(ip0) in ( "127.0.0.1", "::1") | keep host, ip0 | sort host, ip0; host:keyword | ip0:ip diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/version.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/version.csv-spec index d3ab46f321590..9f886981bc560 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/version.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/version.csv-spec @@ -372,7 +372,7 @@ version:version | name:keyword ; implictCastingEqual -required_feature: esql.string_literal_auto_casting_8_15_0 +required_feature: esql.string_literal_auto_casting_to_ip from apps | where version == "1.2.3.4" | sort name | keep name, version; name:keyword | version:version @@ -381,7 +381,7 @@ hhhhh | 1.2.3.4 ; implictCastingNotEqual -required_feature: esql.string_literal_auto_casting_8_15_0 +required_feature: esql.string_literal_auto_casting_to_ip from apps | where version != "1.2.3.4" | sort name, version | keep name, version | limit 2; name:keyword | version:version @@ -390,7 +390,7 @@ bbbbb | 2.1 ; implictCastingGreaterThan -required_feature: esql.string_literal_auto_casting_8_15_0 +required_feature: esql.string_literal_auto_casting_to_ip from apps | where version > "1.2.3.4" | sort name, version | keep name, version | limit 2; name:keyword | version:version @@ -399,7 +399,7 @@ ccccc | 2.3.4 ; implictCastingLessThanOrEqual -required_feature: esql.string_literal_auto_casting_8_15_0 +required_feature: esql.string_literal_auto_casting_to_ip from apps | where version <= "1.2.3.4" | sort name, version | keep name, version | limit 2; name:keyword | version:version @@ -408,7 +408,7 @@ aaaaa | 1.2.3.4 ; implictCastingIn -required_feature: esql.string_literal_auto_casting_8_15_0 +required_feature: esql.string_literal_auto_casting_to_ip from apps | where version in ( "1.2.3.4", "bad" ) | sort name | keep name, version; name:keyword | version:version diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/EsqlFeatures.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/EsqlFeatures.java index 323931fb16d3c..06ef2ebb3b4fe 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/EsqlFeatures.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/EsqlFeatures.java @@ -129,7 +129,7 @@ public class EsqlFeatures implements FeatureSpecification { /** * Cast string literals to a desired data type for IN predicate and more types for BinaryComparison. */ - public static final NodeFeature STRING_LITERAL_AUTO_CASTING_8_15 = new NodeFeature("esql.string_literal_auto_casting_8_15_0"); + public static final NodeFeature STRING_LITERAL_AUTO_CASTING_TO_IP = new NodeFeature("esql.string_literal_auto_casting_to_ip"); @Override public Set getFeatures() { @@ -151,7 +151,7 @@ public Set getFeatures() { CASTING_OPERATOR, MV_ORDERING_SORTED_ASCENDING, METRICS_COUNTER_FIELDS, - STRING_LITERAL_AUTO_CASTING_8_15 + STRING_LITERAL_AUTO_CASTING_TO_IP ); } From 46c132714933a7b589d7837310a035485f03144b Mon Sep 17 00:00:00 2001 From: Fang Xing Date: Fri, 3 May 2024 13:47:25 -0400 Subject: [PATCH 7/7] address review comments --- .../src/main/resources/boolean.csv-spec | 20 +++++++++++++++++-- .../src/main/resources/ip.csv-spec | 10 +++++----- .../src/main/resources/version.csv-spec | 10 +++++----- .../xpack/esql/analysis/Analyzer.java | 13 ++++++------ .../xpack/esql/plugin/EsqlFeatures.java | 4 ++-- 5 files changed, 36 insertions(+), 21 deletions(-) diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/boolean.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/boolean.csv-spec index 3e88ed787f28f..809f4e9ba2c74 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/boolean.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/boolean.csv-spec @@ -347,7 +347,7 @@ still_hired:boolean | job_positions:keyword ; implicitCastingEqual -required_feature: esql.string_literal_auto_casting_to_ip +required_feature: esql.string_literal_auto_casting_extended from employees | where still_hired == "true" | sort emp_no | keep emp_no | limit 1; emp_no:integer @@ -355,9 +355,25 @@ emp_no:integer ; implicitCastingNotEqual -required_feature: esql.string_literal_auto_casting_to_ip +required_feature: esql.string_literal_auto_casting_extended from employees | where still_hired != "true" | sort emp_no | keep emp_no | limit 1; emp_no:integer 10003 ; + +implicitCastingIn +required_feature: esql.string_literal_auto_casting_extended +from employees | where still_hired in ("true", "false") | sort emp_no | keep emp_no | limit 1; + +emp_no:integer +10001 +; + +implicitCastingInField +required_feature: esql.string_literal_auto_casting_extended +from employees | where false in ("true", still_hired) | sort emp_no | keep emp_no | limit 1; + +emp_no:integer +10003 +; diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/ip.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/ip.csv-spec index 2902d1ffc1cb5..f987b27e4737a 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/ip.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/ip.csv-spec @@ -434,7 +434,7 @@ fe80::cae2:65ff:fece:feb9 | gamma ; implictCastingEqual -required_feature: esql.string_literal_auto_casting_to_ip +required_feature: esql.string_literal_auto_casting_extended from hosts | where mv_first(ip0) == "127.0.0.1" | keep host, ip0; host:keyword | ip0:ip @@ -445,7 +445,7 @@ beta | 127.0.0.1 ; implictCastingNotEqual -required_feature: esql.string_literal_auto_casting_to_ip +required_feature: esql.string_literal_auto_casting_extended from hosts | where mv_first(ip0) != "127.0.0.1" | keep host, ip0 | sort host, ip0 | limit 3; host:keyword | ip0:ip @@ -455,7 +455,7 @@ epsilon | [fe81::cae2:65ff:fece:feb9, fe82::cae2:65ff:fece:fec0] ; implictCastingGreaterThan -required_feature: esql.string_literal_auto_casting_to_ip +required_feature: esql.string_literal_auto_casting_extended from hosts | where mv_first(ip0) > "127.0.0.1" | keep host, ip0 | sort host, ip0 | limit 3; host:keyword | ip0:ip @@ -465,7 +465,7 @@ gamma | fe80::cae2:65ff:fece:feb9 ; implictCastingLessThanOrEqual -required_feature: esql.string_literal_auto_casting_to_ip +required_feature: esql.string_literal_auto_casting_extended from hosts | where mv_first(ip0) <= "127.0.0.1" | keep host, ip0 | sort host, ip0 | limit 3; host:keyword | ip0:ip @@ -475,7 +475,7 @@ beta | 127.0.0.1 ; implictCastingIn -required_feature: esql.string_literal_auto_casting_to_ip +required_feature: esql.string_literal_auto_casting_extended from hosts | where mv_first(ip0) in ( "127.0.0.1", "::1") | keep host, ip0 | sort host, ip0; host:keyword | ip0:ip diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/version.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/version.csv-spec index 9f886981bc560..513189cc0fe86 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/version.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/version.csv-spec @@ -372,7 +372,7 @@ version:version | name:keyword ; implictCastingEqual -required_feature: esql.string_literal_auto_casting_to_ip +required_feature: esql.string_literal_auto_casting_extended from apps | where version == "1.2.3.4" | sort name | keep name, version; name:keyword | version:version @@ -381,7 +381,7 @@ hhhhh | 1.2.3.4 ; implictCastingNotEqual -required_feature: esql.string_literal_auto_casting_to_ip +required_feature: esql.string_literal_auto_casting_extended from apps | where version != "1.2.3.4" | sort name, version | keep name, version | limit 2; name:keyword | version:version @@ -390,7 +390,7 @@ bbbbb | 2.1 ; implictCastingGreaterThan -required_feature: esql.string_literal_auto_casting_to_ip +required_feature: esql.string_literal_auto_casting_extended from apps | where version > "1.2.3.4" | sort name, version | keep name, version | limit 2; name:keyword | version:version @@ -399,7 +399,7 @@ ccccc | 2.3.4 ; implictCastingLessThanOrEqual -required_feature: esql.string_literal_auto_casting_to_ip +required_feature: esql.string_literal_auto_casting_extended from apps | where version <= "1.2.3.4" | sort name, version | keep name, version | limit 2; name:keyword | version:version @@ -408,7 +408,7 @@ aaaaa | 1.2.3.4 ; implictCastingIn -required_feature: esql.string_literal_auto_casting_to_ip +required_feature: esql.string_literal_auto_casting_extended from apps | where version in ( "1.2.3.4", "bad" ) | sort name | keep name, version; name:keyword | version:version diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java index 67875236250ee..511aafd52021a 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java @@ -852,14 +852,14 @@ private static Expression processBinaryOperator(BinaryOperator o) { if (left.dataType() == KEYWORD && left.foldable() - && (canCastTo(right.dataType())) + && (supportsImplicitCasting(right.dataType())) && ((left instanceof EsqlScalarFunction) == false)) { targetDataType = right.dataType(); from = left; } if (right.dataType() == KEYWORD && right.foldable() - && (canCastTo(left.dataType())) + && (supportsImplicitCasting(left.dataType())) && ((right instanceof EsqlScalarFunction) == false)) { targetDataType = left.dataType(); from = right; @@ -877,15 +877,14 @@ private static Expression processIn(In in) { Expression left = in.value(); List right = in.list(); - if (left.resolved() == false || canCastTo(left.dataType()) == false) { + if (left.resolved() == false || supportsImplicitCasting(left.dataType()) == false) { return in; } List newChildren = new ArrayList<>(right.size() + 1); boolean childrenChanged = false; - for (int i = 0; i < right.size(); i++) { - Expression value = right.get(i); - if (value.dataType() == KEYWORD) { + for (Expression value : right) { + if (value.dataType() == KEYWORD && value.foldable()) { Expression e = castStringLiteral(value, left.dataType()); newChildren.add(e); childrenChanged = true; @@ -897,7 +896,7 @@ private static Expression processIn(In in) { return childrenChanged ? in.replaceChildren(newChildren) : in; } - private static boolean canCastTo(DataType type) { + private static boolean supportsImplicitCasting(DataType type) { return type == DATETIME || type == IP || type == VERSION || type == BOOLEAN; } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/EsqlFeatures.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/EsqlFeatures.java index 06ef2ebb3b4fe..3a8a34b54ee7a 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/EsqlFeatures.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/EsqlFeatures.java @@ -129,7 +129,7 @@ public class EsqlFeatures implements FeatureSpecification { /** * Cast string literals to a desired data type for IN predicate and more types for BinaryComparison. */ - public static final NodeFeature STRING_LITERAL_AUTO_CASTING_TO_IP = new NodeFeature("esql.string_literal_auto_casting_to_ip"); + public static final NodeFeature STRING_LITERAL_AUTO_CASTING_EXTENDED = new NodeFeature("esql.string_literal_auto_casting_extended"); @Override public Set getFeatures() { @@ -151,7 +151,7 @@ public Set getFeatures() { CASTING_OPERATOR, MV_ORDERING_SORTED_ASCENDING, METRICS_COUNTER_FIELDS, - STRING_LITERAL_AUTO_CASTING_TO_IP + STRING_LITERAL_AUTO_CASTING_EXTENDED ); }