Browse Source

[ES|QL] More implicit casting for binary comparison and IN list predicate (#107859)

* implicit casting for IN and binary comparison on ip, version, and boolean
Fang Xing 1 year ago
parent
commit
47a18d293c

+ 32 - 0
x-pack/plugin/esql/qa/testFixtures/src/main/resources/boolean.csv-spec

@@ -345,3 +345,35 @@ still_hired:boolean | job_positions:keyword
       [false, true] | Tech Lead
       [false, true] | null
 ;
+
+implicitCastingEqual
+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
+10001
+;
+
+implicitCastingNotEqual
+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
+;

+ 30 - 0
x-pack/plugin/esql/qa/testFixtures/src/main/resources/date.csv-spec

@@ -1041,3 +1041,33 @@ 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
 ;
+
+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-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
+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-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
+10003          | 1959-12-03T00:00:00Z
+10008          | 1958-02-19T00:00:00Z
+;

+ 53 - 0
x-pack/plugin/esql/qa/testFixtures/src/main/resources/ip.csv-spec

@@ -432,3 +432,56 @@ 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
 ;
+
+implictCastingEqual
+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
+alpha        | 127.0.0.1
+beta         | 127.0.0.1
+beta         | 127.0.0.1
+beta         | 127.0.0.1
+;
+
+implictCastingNotEqual
+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
+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_extended
+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_extended
+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_extended
+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
+;

+ 0 - 28
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
 

+ 46 - 0
x-pack/plugin/esql/qa/testFixtures/src/main/resources/version.csv-spec

@@ -370,3 +370,49 @@ version:version | name:keyword
            null | lllll
           5.2.9 | mmmmm
 ;
+
+implictCastingEqual
+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
+aaaaa        | 1.2.3.4
+hhhhh        | 1.2.3.4
+;
+
+implictCastingNotEqual
+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
+aaaaa        | 1
+bbbbb        | 2.1
+;
+
+implictCastingGreaterThan
+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
+bbbbb        | 2.1
+ccccc        | 2.3.4
+;
+
+implictCastingLessThanOrEqual
+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
+aaaaa        | 1
+aaaaa        | 1.2.3.4
+;
+
+implictCastingIn
+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
+aaaaa        | 1.2.3.4
+hhhhh        | 1.2.3.4
+iiiii        | bad
+;

+ 35 - 2
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.EsqlFunctionRegistry;
 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.Aggregate;
 import org.elasticsearch.xpack.esql.plan.logical.Drop;
 import org.elasticsearch.xpack.esql.plan.logical.Enrich;
@@ -96,6 +97,7 @@ import static org.elasticsearch.xpack.core.enrich.EnrichPolicy.GEO_MATCH_TYPE;
 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.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;
@@ -105,6 +107,7 @@ import static org.elasticsearch.xpack.ql.type.DataTypes.KEYWORD;
 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<LogicalPlan, AnalyzerContext> {
     // marker list of attributes for plans that do not have any concrete fields to return, but have other computed columns to return
@@ -802,6 +805,9 @@ public class Analyzer extends ParameterizedRuleExecutor<LogicalPlan, AnalyzerCon
             if (f instanceof EsqlArithmeticOperation || f instanceof BinaryComparison) {
                 return processBinaryOperator((BinaryOperator) f);
             }
+            if (f instanceof In in) {
+                return processIn(in);
+            }
             return f;
         }
 
@@ -846,14 +852,14 @@ public class Analyzer extends ParameterizedRuleExecutor<LogicalPlan, AnalyzerCon
 
             if (left.dataType() == KEYWORD
                 && left.foldable()
-                && (right.dataType().isNumeric() || right.dataType() == DATETIME)
+                && (supportsImplicitCasting(right.dataType()))
                 && ((left instanceof EsqlScalarFunction) == false)) {
                 targetDataType = right.dataType();
                 from = left;
             }
             if (right.dataType() == KEYWORD
                 && right.foldable()
-                && (left.dataType().isNumeric() || left.dataType() == DATETIME)
+                && (supportsImplicitCasting(left.dataType()))
                 && ((right instanceof EsqlScalarFunction) == false)) {
                 targetDataType = left.dataType();
                 from = right;
@@ -867,6 +873,33 @@ public class Analyzer extends ParameterizedRuleExecutor<LogicalPlan, AnalyzerCon
             return childrenChanged ? o.replaceChildren(newChildren) : o;
         }
 
+        private static Expression processIn(In in) {
+            Expression left = in.value();
+            List<Expression> right = in.list();
+
+            if (left.resolved() == false || supportsImplicitCasting(left.dataType()) == false) {
+                return in;
+            }
+            List<Expression> newChildren = new ArrayList<>(right.size() + 1);
+            boolean childrenChanged = false;
+
+            for (Expression value : right) {
+                if (value.dataType() == KEYWORD && value.foldable()) {
+                    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 supportsImplicitCasting(DataType type) {
+            return type == DATETIME || type == IP || type == VERSION || type == BOOLEAN;
+        }
+
         public static Expression castStringLiteral(Expression from, DataType target) {
             assert from.foldable();
             try {

+ 7 - 1
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/EsqlFeatures.java

@@ -126,6 +126,11 @@ public class EsqlFeatures implements FeatureSpecification {
      */
     public static final NodeFeature METRICS_COUNTER_FIELDS = new NodeFeature("esql.metrics_counter_fields");
 
+    /**
+     * Cast string literals to a desired data type for IN predicate and more types for BinaryComparison.
+     */
+    public static final NodeFeature STRING_LITERAL_AUTO_CASTING_EXTENDED = new NodeFeature("esql.string_literal_auto_casting_extended");
+
     @Override
     public Set<NodeFeature> getFeatures() {
         return Set.of(
@@ -145,7 +150,8 @@ public class EsqlFeatures implements FeatureSpecification {
             STRING_LITERAL_AUTO_CASTING,
             CASTING_OPERATOR,
             MV_ORDERING_SORTED_ASCENDING,
-            METRICS_COUNTER_FIELDS
+            METRICS_COUNTER_FIELDS,
+            STRING_LITERAL_AUTO_CASTING_EXTENDED
         );
     }
 

+ 2 - 2
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/type/EsqlDataTypeConverter.java

@@ -293,8 +293,8 @@ public class EsqlDataTypeConverter {
         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) {

+ 14 - 2
x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java

@@ -1007,7 +1007,13 @@ public class AnalyzerTests extends ESTestCase {
                 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)
+                )
+            );
         }
     }
 
@@ -1017,7 +1023,13 @@ public class AnalyzerTests extends ESTestCase {
                 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)
+                )
+            );
         }
     }
 

+ 1 - 1
x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java

@@ -390,7 +390,7 @@ public class VerifierTests extends ESTestCase {
 
     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")
         );
 

+ 15 - 5
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.OrderBy;
 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 class StatementParserTests extends ESTestCase {
 
     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 class StatementParserTests extends ESTestCase {
         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() {