[ES|QL] Refactor data type conversions between String/UnsignedLong an…

…d other data types (#106628) * refactor data type conversions related to string and unsignedlong in esql.
elastic · Mar 27, 2024 · 2f0a7b8 · 2f0a7b8
1 parent 9e6b893
commit 2f0a7b8
Show file tree

Hide file tree

Showing 42 changed files with 343 additions and 206 deletions.
diff --git a/...n/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java b/...n/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java
@@ -422,7 +422,7 @@ public void testWarningHeadersOnFailedConversions() throws IOException {
         for (int i = 1; i <= expectedWarnings; i++) {
             assertThat(
                 warnings.get(i),
-                containsString("java.lang.NumberFormatException: For input string: \\\"keyword" + (2 * i - 1) + "\\\"")
+                containsString("org.elasticsearch.xpack.ql.InvalidArgumentException: Cannot parse number [keyword" + (2 * i - 1) + "]")
             );
         }
     }

diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/floats.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/floats.csv-spec
@@ -55,14 +55,14 @@ emp_no:integer |hire_date:date          |hire_double:double
 10003          |1986-08-28T00:00:00.000Z|5.255712E11
 ;
 
-convertFromString
+convertFromString#[skip:-8.13.99, reason:warning changed in 8.14]
 // tag::to_double-str[]
 ROW str1 = "5.20128E11", str2 = "foo"
 | EVAL dbl = TO_DOUBLE("520128000000"), dbl1 = TO_DOUBLE(str1), dbl2 = TO_DOUBLE(str2)
 // end::to_double-str[]
 ;
 warning:Line 2:72: evaluation of [TO_DOUBLE(str2)] failed, treating result as null. Only first 20 failures recorded.
-warning:Line 2:72: java.lang.NumberFormatException: For input string: \"foo\"
+warning:Line 2:72: org.elasticsearch.xpack.ql.InvalidArgumentException: Cannot parse number [foo]
 
 // tag::to_double-str-result[]
 str1:keyword |str2:keyword |dbl:double |dbl1:double |dbl2:double

diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/ints.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/ints.csv-spec
@@ -153,14 +153,14 @@ tf:boolean     |t2l:long       |f2l:long       |tf2l:long
 [true, false]  |1              |0              |[1, 0]
 ;
 
-convertStringToLong
+convertStringToLong#[skip:-8.13.99, reason:warning changed in 8.14]
 // tag::to_long-str[]
 ROW str1 = "2147483648", str2 = "2147483648.2", str3 = "foo"
 | EVAL long1 = TO_LONG(str1), long2 = TO_LONG(str2), long3 = TO_LONG(str3)
 // end::to_long-str[]
 ;
 warning:Line 2:62: evaluation of [TO_LONG(str3)] failed, treating result as null. Only first 20 failures recorded.
-warning:Line 2:62: java.lang.NumberFormatException: For input string: \"foo\"
+warning:Line 2:62: org.elasticsearch.xpack.ql.InvalidArgumentException: Cannot parse number [foo]
 
 
 // tag::to_long-str-result[]
@@ -238,16 +238,16 @@ int_str:keyword  |int_dbl_str:keyword |is2i:integer|ids2i:integer
 2147483647       |2147483646.2        |2147483647  |2147483646
 ;
 
-convertStringToIntFail
+convertStringToIntFail#[skip:-8.13.99, reason:warning changed in 8.14]
 required_feature: esql.mv_warn
 
 row str1 = "2147483647.2", str2 = "2147483648", non = "no number" | eval i1 = to_integer(str1), i2 = to_integer(str2), noi = to_integer(non);
 warning:Line 1:79: evaluation of [to_integer(str1)] failed, treating result as null. Only first 20 failures recorded.
-warning:Line 1:79: java.lang.NumberFormatException: For input string: \"2147483647.2\"
+warning:Line 1:79: org.elasticsearch.xpack.ql.InvalidArgumentException: Cannot parse number [2147483647.2]
 warning:Line 1:102: evaluation of [to_integer(str2)] failed, treating result as null. Only first 20 failures recorded.
-warning:Line 1:102: java.lang.NumberFormatException: For input string: \"2147483648\"
+warning:Line 1:102: org.elasticsearch.xpack.ql.InvalidArgumentException: Cannot parse number [2147483648]
 warning:Line 1:126: evaluation of [to_integer(non)] failed, treating result as null. Only first 20 failures recorded.
-warning:Line 1:126: java.lang.NumberFormatException: For input string: \"no number\"
+warning:Line 1:126: org.elasticsearch.xpack.ql.InvalidArgumentException: Cannot parse number [no number]
 
 str1:keyword   |str2:keyword   |non:keyword    |i1:integer     |i2:integer     |noi:integer
 2147483647.2   |2147483648     |no number      |null           |null           |null

diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/row.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/row.csv-spec
@@ -289,30 +289,30 @@ a:integer |b:integer |c:integer
 // end::in-with-expressions-result[]
 ;
 
-convertMvToMvDifferentCardinality
+convertMvToMvDifferentCardinality#[skip:-8.13.99, reason:warning changed in 8.14]
 row strings = ["1", "2", "three"] | eval ints = to_int(strings);
 warning:Line 1:49: evaluation of [to_int(strings)] failed, treating result as null. Only first 20 failures recorded.
-warning:Line 1:49: java.lang.NumberFormatException: For input string: \"three\"
+warning:Line 1:49: org.elasticsearch.xpack.ql.InvalidArgumentException: Cannot parse number [three]
 
 strings:keyword |ints:integer
 [1, 2, three]   |[1, 2]
 ;
 
-convertMvToSv
+convertMvToSv#[skip:-8.13.99, reason:warning changed in 8.14]
 row strings = ["1", "two"] | eval ints = to_int(strings);
 
 warning:Line 1:42: evaluation of [to_int(strings)] failed, treating result as null. Only first 20 failures recorded.
-warning:Line 1:42: java.lang.NumberFormatException: For input string: \"two\"
+warning:Line 1:42: org.elasticsearch.xpack.ql.InvalidArgumentException: Cannot parse number [two]
 
 strings:keyword |ints:integer
 [1, two]        |1
 ;
 
-convertMvToNull
+convertMvToNull#[skip:-8.13.99, reason:warning changed in 8.14]
 row strings = ["one", "two"] | eval ints = to_int(strings);
 warning:Line 1:44: evaluation of [to_int(strings)] failed, treating result as null. Only first 20 failures recorded.
-warning:Line 1:44: java.lang.NumberFormatException: For input string: \"one\"
-warning:Line 1:44: java.lang.NumberFormatException: For input string: \"two\"
+warning:Line 1:44: org.elasticsearch.xpack.ql.InvalidArgumentException: Cannot parse number [one]
+warning:Line 1:44: org.elasticsearch.xpack.ql.InvalidArgumentException: Cannot parse number [two]
 
 strings:keyword |ints:integer
 [one, two]      |null

diff --git a/...sticsearch/xpack/esql/expression/function/scalar/convert/ToDoubleFromStringEvaluator.java b/...sticsearch/xpack/esql/expression/function/scalar/convert/ToDoubleFromStringEvaluator.java
@@ -4,7 +4,6 @@
 // 2.0.
 package org.elasticsearch.xpack.esql.expression.function.scalar.convert;
 
-import java.lang.NumberFormatException;
 import java.lang.Override;
 import java.lang.String;
 import org.apache.lucene.util.BytesRef;
@@ -15,6 +14,7 @@
 import org.elasticsearch.compute.data.Vector;
 import org.elasticsearch.compute.operator.DriverContext;
 import org.elasticsearch.compute.operator.EvalOperator;
+import org.elasticsearch.xpack.ql.InvalidArgumentException;
 import org.elasticsearch.xpack.ql.tree.Source;
 
 /**
@@ -40,7 +40,7 @@ public Block evalVector(Vector v) {
     if (vector.isConstant()) {
       try {
         return driverContext.blockFactory().newConstantDoubleBlockWith(evalValue(vector, 0, scratchPad), positionCount);
-      } catch (NumberFormatException  e) {
+      } catch (InvalidArgumentException  e) {
         registerException(e);
         return driverContext.blockFactory().newConstantNullBlock(positionCount);
       }
@@ -49,7 +49,7 @@ public Block evalVector(Vector v) {
       for (int p = 0; p < positionCount; p++) {
         try {
           builder.appendDouble(evalValue(vector, p, scratchPad));
-        } catch (NumberFormatException  e) {
+        } catch (InvalidArgumentException  e) {
           registerException(e);
           builder.appendNull();
         }
@@ -84,7 +84,7 @@ public Block evalBlock(Block b) {
             }
             builder.appendDouble(value);
             valuesAppended = true;
-          } catch (NumberFormatException  e) {
+          } catch (InvalidArgumentException  e) {
             registerException(e);
           }
         }

diff --git a/...ticsearch/xpack/esql/expression/function/scalar/convert/ToIntegerFromStringEvaluator.java b/...ticsearch/xpack/esql/expression/function/scalar/convert/ToIntegerFromStringEvaluator.java
@@ -4,7 +4,6 @@
 // 2.0.
 package org.elasticsearch.xpack.esql.expression.function.scalar.convert;
 
-import java.lang.NumberFormatException;
 import java.lang.Override;
 import java.lang.String;
 import org.apache.lucene.util.BytesRef;
@@ -41,7 +40,7 @@ public Block evalVector(Vector v) {
     if (vector.isConstant()) {
       try {
         return driverContext.blockFactory().newConstantIntBlockWith(evalValue(vector, 0, scratchPad), positionCount);
-      } catch (InvalidArgumentException | NumberFormatException  e) {
+      } catch (InvalidArgumentException  e) {
         registerException(e);
         return driverContext.blockFactory().newConstantNullBlock(positionCount);
       }
@@ -50,7 +49,7 @@ public Block evalVector(Vector v) {
       for (int p = 0; p < positionCount; p++) {
         try {
           builder.appendInt(evalValue(vector, p, scratchPad));
-        } catch (InvalidArgumentException | NumberFormatException  e) {
+        } catch (InvalidArgumentException  e) {
           registerException(e);
           builder.appendNull();
         }
@@ -85,7 +84,7 @@ public Block evalBlock(Block b) {
             }
             builder.appendInt(value);
             valuesAppended = true;
-          } catch (InvalidArgumentException | NumberFormatException  e) {
+          } catch (InvalidArgumentException  e) {
             registerException(e);
           }
         }

diff --git a/...lasticsearch/xpack/esql/expression/function/scalar/convert/ToLongFromStringEvaluator.java b/...lasticsearch/xpack/esql/expression/function/scalar/convert/ToLongFromStringEvaluator.java
@@ -4,7 +4,6 @@
 // 2.0.
 package org.elasticsearch.xpack.esql.expression.function.scalar.convert;
 
-import java.lang.NumberFormatException;
 import java.lang.Override;
 import java.lang.String;
 import org.apache.lucene.util.BytesRef;
@@ -15,6 +14,7 @@
 import org.elasticsearch.compute.data.Vector;
 import org.elasticsearch.compute.operator.DriverContext;
 import org.elasticsearch.compute.operator.EvalOperator;
+import org.elasticsearch.xpack.ql.InvalidArgumentException;
 import org.elasticsearch.xpack.ql.tree.Source;
 
 /**
@@ -40,7 +40,7 @@ public Block evalVector(Vector v) {
     if (vector.isConstant()) {
       try {
         return driverContext.blockFactory().newConstantLongBlockWith(evalValue(vector, 0, scratchPad), positionCount);
-      } catch (NumberFormatException  e) {
+      } catch (InvalidArgumentException  e) {
         registerException(e);
         return driverContext.blockFactory().newConstantNullBlock(positionCount);
       }
@@ -49,7 +49,7 @@ public Block evalVector(Vector v) {
       for (int p = 0; p < positionCount; p++) {
         try {
           builder.appendLong(evalValue(vector, p, scratchPad));
-        } catch (NumberFormatException  e) {
+        } catch (InvalidArgumentException  e) {
           registerException(e);
           builder.appendNull();
         }
@@ -84,7 +84,7 @@ public Block evalBlock(Block b) {
             }
             builder.appendLong(value);
             valuesAppended = true;
-          } catch (NumberFormatException  e) {
+          } catch (InvalidArgumentException  e) {
             registerException(e);
           }
         }

diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/PositionToXContent.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/PositionToXContent.java
@@ -17,21 +17,20 @@
 import org.elasticsearch.compute.data.IntBlock;
 import org.elasticsearch.compute.data.LongBlock;
 import org.elasticsearch.compute.lucene.UnsupportedValueSource;
-import org.elasticsearch.search.DocValueFormat;
 import org.elasticsearch.xcontent.ToXContent;
 import org.elasticsearch.xcontent.XContentBuilder;
 import org.elasticsearch.xcontent.XContentParser;
 import org.elasticsearch.xcontent.XContentParserConfiguration;
 import org.elasticsearch.xcontent.XContentType;
 import org.elasticsearch.xpack.core.esql.action.ColumnInfo;
-import org.elasticsearch.xpack.versionfield.Version;
 
 import java.io.IOException;
 
 import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.dateTimeToString;
+import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.ipToString;
+import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.spatialToString;
+import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.versionToString;
 import static org.elasticsearch.xpack.ql.util.NumericUtils.unsignedLongAsNumber;
-import static org.elasticsearch.xpack.ql.util.SpatialCoordinateTypes.CARTESIAN;
-import static org.elasticsearch.xpack.ql.util.SpatialCoordinateTypes.GEO;
 
 abstract class PositionToXContent {
     protected final Block block;
@@ -109,7 +108,7 @@ protected XContentBuilder valueToXContent(XContentBuilder builder, ToXContent.Pa
                 protected XContentBuilder valueToXContent(XContentBuilder builder, ToXContent.Params params, int valueIndex)
                     throws IOException {
                     BytesRef val = ((BytesRefBlock) block).getBytesRef(valueIndex, scratch);
-                    return builder.value(DocValueFormat.IP.format(val));
+                    return builder.value(ipToString(val));
                 }
             };
             case "date" -> new PositionToXContent(block) {
@@ -120,18 +119,11 @@ protected XContentBuilder valueToXContent(XContentBuilder builder, ToXContent.Pa
                     return builder.value(dateTimeToString(longVal));
                 }
             };
-            case "geo_point", "geo_shape" -> new PositionToXContent(block) {
+            case "geo_point", "geo_shape", "cartesian_point", "cartesian_shape" -> new PositionToXContent(block) {
                 @Override
                 protected XContentBuilder valueToXContent(XContentBuilder builder, ToXContent.Params params, int valueIndex)
                     throws IOException {
-                    return builder.value(GEO.wkbToWkt(((BytesRefBlock) block).getBytesRef(valueIndex, scratch)));
-                }
-            };
-            case "cartesian_point", "cartesian_shape" -> new PositionToXContent(block) {
-                @Override
-                protected XContentBuilder valueToXContent(XContentBuilder builder, ToXContent.Params params, int valueIndex)
-                    throws IOException {
-                    return builder.value(CARTESIAN.wkbToWkt(((BytesRefBlock) block).getBytesRef(valueIndex, scratch)));
+                    return builder.value(spatialToString(((BytesRefBlock) block).getBytesRef(valueIndex, scratch)));
                 }
             };
             case "boolean" -> new PositionToXContent(block) {
@@ -146,7 +138,7 @@ protected XContentBuilder valueToXContent(XContentBuilder builder, ToXContent.Pa
                 protected XContentBuilder valueToXContent(XContentBuilder builder, ToXContent.Params params, int valueIndex)
                     throws IOException {
                     BytesRef val = ((BytesRefBlock) block).getBytesRef(valueIndex, scratch);
-                    return builder.value(new Version(val).toString());
+                    return builder.value(versionToString(val));
                 }
             };
             case "null" -> new PositionToXContent(block) {