From e5ea51f7a8d2f4c097a18550cf4921e8961100d6 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Tue, 8 Apr 2025 12:38:27 -0400 Subject: [PATCH 1/7] ESQL: TO_IP can handle leading zeros Modifies TO_IP so it can handle leading `0`s in ipv4s. Here's how it works now: ``` ROW ip = TO_IP("192.168.0.1") // OK! ROW ip = TO_IP("192.168.010.1") // Fails ``` This adds ``` ROW ip = TO_IP("192.168.010.1", {"leading_zeros": "octal"}) ROW ip = TO_IP("192.168.010.1", {"leading_zeros": "decimal"}) ``` We do this because there isn't a consensus on how to parse leading zeros in ipv4s. The standard unix tools like `ping` and `ftp` interpret leading zeros as octal. Java's built in ip parsing interprets them as decimal. Because folks are using this for security rules we need to support all the choices. Closes #125460 --- .../functions/description/date_trunc.md | 3 +- .../_snippets/functions/examples/bucket.md | 3 +- .../_snippets/functions/examples/to_ip.md | 24 +- .../functions/functionNamedParams/to_ip.md | 7 + .../esql/_snippets/functions/layout/to_ip.md | 3 + .../_snippets/functions/parameters/to_ip.md | 3 + .../esql/_snippets/functions/types/to_ip.md | 10 +- .../esql/images/functions/to_ip.svg | 2 +- .../definition/functions/date_trunc.json | 2 +- .../kibana/definition/functions/to_ip.json | 4 +- .../esql/kibana/docs/functions/date_trunc.md | 4 +- .../src/main/resources/ip.csv-spec | 60 +++++ .../xpack/esql/action/EsqlCapabilities.java | 7 +- .../xpack/esql/analysis/Analyzer.java | 32 ++- .../esql/expression/ExpressionWritables.java | 8 +- .../function/EsqlFunctionRegistry.java | 18 +- .../convert/AbstractConvertFunction.java | 5 +- .../scalar/convert/ConvertFunction.java | 31 +++ .../function/scalar/convert/ParseIp.java | 8 + .../function/scalar/convert/ToIp.java | 255 ++++++++++++++++++ .../convert/ToIpLeadingZerosDecimal.java | 71 +++++ .../scalar/convert/ToIpLeadingZerosOctal.java | 71 +++++ ...oIP.java => ToIpLeadingZerosRejected.java} | 46 ++-- .../spatial/SpatialRelatesFunction.java | 5 +- .../esql/optimizer/LogicalPlanOptimizer.java | 8 +- .../logical/SubstituteSpatialSurrogates.java | 30 --- ...a => SubstituteSurrogateAggregations.java} | 6 +- .../SubstituteSurrogateExpressions.java | 33 +++ .../esql/type/EsqlDataTypeConverter.java | 4 +- .../xpack/esql/analysis/ParsingTests.java | 16 +- .../function/AbstractFunctionTestCase.java | 18 +- .../AbstractScalarFunctionTestCase.java | 7 + .../function/scalar/convert/ToIPTests.java | 87 ------ ...oIPErrorTests.java => ToIpErrorTests.java} | 7 +- ...LeadingZerosDecimalSerializationTests.java | 19 ++ ...pLeadingZerosOctalSerializationTests.java} | 6 +- ...eadingZerosRejectedSerializationTests.java | 19 ++ .../function/scalar/convert/ToIpTests.java | 197 ++++++++++++++ .../esql/type/MultiTypeEsFieldTests.java | 4 +- 39 files changed, 931 insertions(+), 212 deletions(-) create mode 100644 docs/reference/query-languages/esql/_snippets/functions/functionNamedParams/to_ip.md create mode 100644 x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ConvertFunction.java create mode 100644 x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIp.java create mode 100644 x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIpLeadingZerosDecimal.java create mode 100644 x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIpLeadingZerosOctal.java rename x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/{ToIP.java => ToIpLeadingZerosRejected.java} (52%) delete mode 100644 x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/SubstituteSpatialSurrogates.java rename x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/{SubstituteSurrogates.java => SubstituteSurrogateAggregations.java} (97%) create mode 100644 x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/SubstituteSurrogateExpressions.java delete mode 100644 x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIPTests.java rename x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/{ToIPErrorTests.java => ToIpErrorTests.java} (81%) create mode 100644 x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIpLeadingZerosDecimalSerializationTests.java rename x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/{ToIPSerializationTests.java => ToIpLeadingZerosOctalSerializationTests.java} (68%) create mode 100644 x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIpLeadingZerosRejectedSerializationTests.java create mode 100644 x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIpTests.java diff --git a/docs/reference/query-languages/esql/_snippets/functions/description/date_trunc.md b/docs/reference/query-languages/esql/_snippets/functions/description/date_trunc.md index c32140d50ecc9..e207cc6d94034 100644 --- a/docs/reference/query-languages/esql/_snippets/functions/description/date_trunc.md +++ b/docs/reference/query-languages/esql/_snippets/functions/description/date_trunc.md @@ -2,6 +2,5 @@ **Description** -Rounds down a date to the closest interval since epoch, which starts -at `0001-01-01T00:00:00Z`. +Rounds down a date to the closest interval since epoch, which starts at `0001-01-01T00:00:00Z`. diff --git a/docs/reference/query-languages/esql/_snippets/functions/examples/bucket.md b/docs/reference/query-languages/esql/_snippets/functions/examples/bucket.md index 2a598a4acf486..9973ecad71a40 100644 --- a/docs/reference/query-languages/esql/_snippets/functions/examples/bucket.md +++ b/docs/reference/query-languages/esql/_snippets/functions/examples/bucket.md @@ -100,8 +100,7 @@ FROM employees ::::{note} When providing the bucket size as the second parameter, it must be a time -duration or date period. Also the reference is epoch, which starts -at `0001-01-01T00:00:00Z`. +duration or date period. Also the reference is epoch, which starts at `0001-01-01T00:00:00Z`. :::: `BUCKET` can also operate on numeric fields. For example, to create a salary histogram: diff --git a/docs/reference/query-languages/esql/_snippets/functions/examples/to_ip.md b/docs/reference/query-languages/esql/_snippets/functions/examples/to_ip.md index 7f4256e24282e..f973ae8da4c24 100644 --- a/docs/reference/query-languages/esql/_snippets/functions/examples/to_ip.md +++ b/docs/reference/query-languages/esql/_snippets/functions/examples/to_ip.md @@ -1,6 +1,6 @@ % This is generated by ESQL's AbstractFunctionTestCase. Do no edit it. See ../README.md for how to regenerate it. -**Example** +**Examples** ```esql ROW str1 = "1.1.1.1", str2 = "foo" @@ -23,4 +23,26 @@ A following header will contain the failure reason and the offending value: `"java.lang.IllegalArgumentException: 'foo' is not an IP string literal."` +```esql +ROW s = "1.1.010.1" | EVAL ip = TO_IP(s, {"leading_zeros":"octal"}) +``` + +| s:keyword | ip:ip | +| --- | --- | +| 1.1.010.1 | 1.1.8.1 | + + +Parse v4 addresses with leading zeros as octal. Like `ping` or `ftp`. + +```esql +ROW s = "1.1.010.1" | EVAL ip = TO_IP(s, {"leading_zeros":"decimal"}) +``` + +| s:keyword | ip:ip | +| --- | --- | +| 1.1.010.1 | 1.1.10.1 | + + +Parse v4 addresses with leading zeros as decimal. Java's `InetAddress.getByName`. + diff --git a/docs/reference/query-languages/esql/_snippets/functions/functionNamedParams/to_ip.md b/docs/reference/query-languages/esql/_snippets/functions/functionNamedParams/to_ip.md new file mode 100644 index 0000000000000..63705daf29798 --- /dev/null +++ b/docs/reference/query-languages/esql/_snippets/functions/functionNamedParams/to_ip.md @@ -0,0 +1,7 @@ +% This is generated by ESQL's AbstractFunctionTestCase. Do no edit it. See ../README.md for how to regenerate it. + +**Supported function named parameters** + +`leading_zeros` +: (keyword) What to do with leading 0s in IPv4 addresses. + diff --git a/docs/reference/query-languages/esql/_snippets/functions/layout/to_ip.md b/docs/reference/query-languages/esql/_snippets/functions/layout/to_ip.md index 1f62f7a098eb6..2ad1c59e09868 100644 --- a/docs/reference/query-languages/esql/_snippets/functions/layout/to_ip.md +++ b/docs/reference/query-languages/esql/_snippets/functions/layout/to_ip.md @@ -19,5 +19,8 @@ :::{include} ../types/to_ip.md ::: +:::{include} ../functionNamedParams/to_ip.md +::: + :::{include} ../examples/to_ip.md ::: diff --git a/docs/reference/query-languages/esql/_snippets/functions/parameters/to_ip.md b/docs/reference/query-languages/esql/_snippets/functions/parameters/to_ip.md index 6957a5264b050..7371ac6b8bb03 100644 --- a/docs/reference/query-languages/esql/_snippets/functions/parameters/to_ip.md +++ b/docs/reference/query-languages/esql/_snippets/functions/parameters/to_ip.md @@ -5,3 +5,6 @@ `field` : Input value. The input can be a single- or multi-valued column or an expression. +`options` +: (Optional) Additional options. + diff --git a/docs/reference/query-languages/esql/_snippets/functions/types/to_ip.md b/docs/reference/query-languages/esql/_snippets/functions/types/to_ip.md index d8ffccaa6935a..e2d8481d8a370 100644 --- a/docs/reference/query-languages/esql/_snippets/functions/types/to_ip.md +++ b/docs/reference/query-languages/esql/_snippets/functions/types/to_ip.md @@ -2,9 +2,9 @@ **Supported types** -| field | result | -| --- | --- | -| ip | ip | -| keyword | ip | -| text | ip | +| field | options | result | +| --- | --- | --- | +| ip | | ip | +| keyword | | ip | +| text | | ip | diff --git a/docs/reference/query-languages/esql/images/functions/to_ip.svg b/docs/reference/query-languages/esql/images/functions/to_ip.svg index 47cd11f4f6ed2..a5127605ffc1b 100644 --- a/docs/reference/query-languages/esql/images/functions/to_ip.svg +++ b/docs/reference/query-languages/esql/images/functions/to_ip.svg @@ -1 +1 @@ -TO_IP(field) \ No newline at end of file +TO_IP(field,options) \ No newline at end of file diff --git a/docs/reference/query-languages/esql/kibana/definition/functions/date_trunc.json b/docs/reference/query-languages/esql/kibana/definition/functions/date_trunc.json index fe293c718a55f..c05449214e6a0 100644 --- a/docs/reference/query-languages/esql/kibana/definition/functions/date_trunc.json +++ b/docs/reference/query-languages/esql/kibana/definition/functions/date_trunc.json @@ -2,7 +2,7 @@ "comment" : "This is generated by ESQL’s AbstractFunctionTestCase. Do no edit it. See ../README.md for how to regenerate it.", "type" : "scalar", "name" : "date_trunc", - "description": "Rounds down a date to the closest interval since epoch, which starts at `0001-01-01T00:00:00Z`.", + "description" : "Rounds down a date to the closest interval since epoch, which starts at `0001-01-01T00:00:00Z`.", "signatures" : [ { "params" : [ diff --git a/docs/reference/query-languages/esql/kibana/definition/functions/to_ip.json b/docs/reference/query-languages/esql/kibana/definition/functions/to_ip.json index 50fbcd339e26f..7ed43181d3105 100644 --- a/docs/reference/query-languages/esql/kibana/definition/functions/to_ip.json +++ b/docs/reference/query-languages/esql/kibana/definition/functions/to_ip.json @@ -42,7 +42,9 @@ } ], "examples" : [ - "ROW str1 = \"1.1.1.1\", str2 = \"foo\"\n| EVAL ip1 = TO_IP(str1), ip2 = TO_IP(str2)\n| WHERE CIDR_MATCH(ip1, \"1.0.0.0/8\")" + "ROW str1 = \"1.1.1.1\", str2 = \"foo\"\n| EVAL ip1 = TO_IP(str1), ip2 = TO_IP(str2)\n| WHERE CIDR_MATCH(ip1, \"1.0.0.0/8\")", + "ROW s = \"1.1.010.1\" | EVAL ip = TO_IP(s, {\"leading_zeros\":\"octal\"})", + "ROW s = \"1.1.010.1\" | EVAL ip = TO_IP(s, {\"leading_zeros\":\"decimal\"})" ], "preview" : false, "snapshot_only" : false diff --git a/docs/reference/query-languages/esql/kibana/docs/functions/date_trunc.md b/docs/reference/query-languages/esql/kibana/docs/functions/date_trunc.md index 38d15ccd93e4a..36525c85f8519 100644 --- a/docs/reference/query-languages/esql/kibana/docs/functions/date_trunc.md +++ b/docs/reference/query-languages/esql/kibana/docs/functions/date_trunc.md @@ -3,9 +3,7 @@ This is generated by ESQL’s AbstractFunctionTestCase. Do no edit it. See ../RE --> ### DATE_TRUNC - -Rounds down a date to the closest interval since epoch, which starts -at `0001-01-01T00:00:00Z`. +Rounds down a date to the closest interval since epoch, which starts at `0001-01-01T00:00:00Z`. ```esql FROM employees 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 e8d59628c29fc..e3268bbf81b8e 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 @@ -271,6 +271,66 @@ str1:keyword |str2:keyword |ip1:ip |ip2:ip // end::to_ip-result[] ; +convertFromStringLeadingZerosDecimal +required_capability: to_ip_leading_zeros +// tag::to_ip_leading_zeros_decimal[] +ROW s = "1.1.010.1" | EVAL ip = TO_IP(s, {"leading_zeros":"decimal"}) +// end::to_ip_leading_zeros_decimal[] +; + +// tag::to_ip_leading_zeros_decimal-result[] +s:keyword | ip:ip +1.1.010.1 | 1.1.10.1 +// end::to_ip_leading_zeros_decimal-result[] +; + +convertFromStringLeadingZerosOctal +required_capability: to_ip_leading_zeros +// tag::to_ip_leading_zeros_octal[] +ROW s = "1.1.010.1" | EVAL ip = TO_IP(s, {"leading_zeros":"octal"}) +// end::to_ip_leading_zeros_octal[] +; + +// tag::to_ip_leading_zeros_octal-result[] +s:keyword | ip:ip +1.1.010.1 | 1.1.8.1 +// end::to_ip_leading_zeros_octal-result[] +; + +toIpInAgg +ROW s = "1.1.1.1" | STATS COUNT(*) BY ip = TO_IP(s) +; + +COUNT(*):long | ip:ip + 1 | 1.1.1.1 +; + +toIpInSort +ROW s = "1.1.1.1" | SORT TO_IP(s) +; + +s:keyword +1.1.1.1 +; + +toIpInAggOctal +required_capability: to_ip_leading_zeros +ROW s = "1.1.010.1" | STATS COUNT(*) BY ip = TO_IP(s, {"leading_zeros":"octal"}) +; + +COUNT(*):long | ip:ip + 1 | 1.1.8.1 +; + +toIpInSortOctal +required_capability: to_ip_leading_zeros +ROW s = "1.1.010.1" | SORT TO_IP(s, {"leading_zeros":"octal"}) +; + +s:keyword +1.1.010.1 +; + cdirMatchOrsIPs required_capability: combine_disjunctive_cidrmatches diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java index 3e4b0b50645ab..2e74b3200eab7 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java @@ -947,7 +947,12 @@ public enum Cap { /** * Supercedes {@link Cap#MAKE_NUMBER_OF_CHANNELS_CONSISTENT_WITH_LAYOUT}. */ - FIX_REPLACE_MISSING_FIELD_WITH_NULL_DUPLICATE_NAME_ID_IN_LAYOUT; + FIX_REPLACE_MISSING_FIELD_WITH_NULL_DUPLICATE_NAME_ID_IN_LAYOUT, + + /** + * Support for the {@code leading_zeros} named parameter. + */ + TO_IP_LEADING_ZEROS; private final boolean enabled; 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 7ab25d43dd8cb..192adef1ff64f 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 @@ -47,6 +47,7 @@ import org.elasticsearch.xpack.esql.core.util.Holder; import org.elasticsearch.xpack.esql.core.util.StringUtils; import org.elasticsearch.xpack.esql.expression.NamedExpressions; +import org.elasticsearch.xpack.esql.expression.SurrogateExpression; import org.elasticsearch.xpack.esql.expression.UnresolvedNamePattern; import org.elasticsearch.xpack.esql.expression.function.EsqlFunctionRegistry; import org.elasticsearch.xpack.esql.expression.function.FunctionDefinition; @@ -58,6 +59,7 @@ import org.elasticsearch.xpack.esql.expression.function.scalar.conditional.Greatest; import org.elasticsearch.xpack.esql.expression.function.scalar.conditional.Least; import org.elasticsearch.xpack.esql.expression.function.scalar.convert.AbstractConvertFunction; +import org.elasticsearch.xpack.esql.expression.function.scalar.convert.ConvertFunction; import org.elasticsearch.xpack.esql.expression.function.scalar.convert.FoldablesConvertFunction; import org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToDouble; import org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToInteger; @@ -1523,10 +1525,12 @@ private LogicalPlan doRule(LogicalPlan plan) { int alreadyAddedUnionFieldAttributes = unionFieldAttributes.size(); // See if the eval function has an unresolved MultiTypeEsField field // Replace the entire convert function with a new FieldAttribute (containing type conversion knowledge) - plan = plan.transformExpressionsOnly( - AbstractConvertFunction.class, - convert -> resolveConvertFunction(convert, unionFieldAttributes) - ); + plan = plan.transformExpressionsOnly(e -> { + if (e instanceof ConvertFunction convert) { + return resolveConvertFunction(convert, unionFieldAttributes); + } + return e; + }); // If no union fields were generated, return the plan as is if (unionFieldAttributes.size() == alreadyAddedUnionFieldAttributes) { return plan; @@ -1557,7 +1561,7 @@ private LogicalPlan doRule(LogicalPlan plan) { return plan; } - private Expression resolveConvertFunction(AbstractConvertFunction convert, List unionFieldAttributes) { + private Expression resolveConvertFunction(ConvertFunction convert, List unionFieldAttributes) { if (convert.field() instanceof FieldAttribute fa && fa.field() instanceof InvalidMappedField imf) { HashMap typeResolutions = new HashMap<>(); Set supportedTypes = convert.supportedTypes(); @@ -1586,7 +1590,7 @@ private Expression resolveConvertFunction(AbstractConvertFunction convert, List< } else if (convert.field() instanceof AbstractConvertFunction subConvert) { return convert.replaceChildren(Collections.singletonList(resolveConvertFunction(subConvert, unionFieldAttributes))); } - return convert; + return (Expression) convert; } private Expression createIfDoesNotAlreadyExist( @@ -1622,7 +1626,7 @@ private MultiTypeEsField resolvedMultiTypeEsField(FieldAttribute fa, HashMap unaryScalars() { entries.add(ToGeoShape.ENTRY); entries.add(ToCartesianShape.ENTRY); entries.add(ToGeoPoint.ENTRY); - entries.add(ToIP.ENTRY); + entries.add(ToIpLeadingZerosDecimal.ENTRY); + entries.add(ToIpLeadingZerosOctal.ENTRY); + entries.add(ToIpLeadingZerosRejected.ENTRY); entries.add(ToInteger.ENTRY); entries.add(ToLong.ENTRY); entries.add(ToRadians.ENTRY); diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/EsqlFunctionRegistry.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/EsqlFunctionRegistry.java index 41c2b64004b8f..5fa241b2c7e29 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/EsqlFunctionRegistry.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/EsqlFunctionRegistry.java @@ -16,6 +16,7 @@ import org.elasticsearch.xpack.esql.core.tree.Source; import org.elasticsearch.xpack.esql.core.type.DataType; import org.elasticsearch.xpack.esql.core.util.Check; +import org.elasticsearch.xpack.esql.expression.SurrogateExpression; import org.elasticsearch.xpack.esql.expression.function.aggregate.Avg; import org.elasticsearch.xpack.esql.expression.function.aggregate.Count; import org.elasticsearch.xpack.esql.expression.function.aggregate.CountDistinct; @@ -54,8 +55,11 @@ import org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToDouble; import org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToGeoPoint; import org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToGeoShape; -import org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToIP; import org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToInteger; +import org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToIp; +import org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToIpLeadingZerosDecimal; +import org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToIpLeadingZerosOctal; +import org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToIpLeadingZerosRejected; import org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToLong; import org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToRadians; import org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToString; @@ -228,6 +232,7 @@ public class EsqlFunctionRegistry { public EsqlFunctionRegistry() { register(functions()); buildDataTypesForStringLiteralConversion(functions()); + nameSurrogates(); } EsqlFunctionRegistry(FunctionDefinition... functions) { @@ -389,7 +394,7 @@ private static FunctionDefinition[][] functions() { def(ToDouble.class, ToDouble::new, "to_double", "to_dbl"), def(ToGeoPoint.class, ToGeoPoint::new, "to_geopoint"), def(ToGeoShape.class, ToGeoShape::new, "to_geoshape"), - def(ToIP.class, ToIP::new, "to_ip"), + def(ToIp.class, ToIp::new, "to_ip"), def(ToInteger.class, ToInteger::new, "to_integer", "to_int"), def(ToLong.class, ToLong::new, "to_long"), def(ToRadians.class, ToRadians::new, "to_radians"), @@ -791,6 +796,15 @@ protected void buildDataTypesForStringLiteralConversion(FunctionDefinition[]... } } + /** + * Add {@link #names} entries for functions that are not registered, but we rewrite to using {@link SurrogateExpression}. + */ + private void nameSurrogates() { + names.put(ToIpLeadingZerosRejected.class, "TO_IP"); + names.put(ToIpLeadingZerosDecimal.class, "TO_IP"); + names.put(ToIpLeadingZerosOctal.class, "TO_IP"); + } + protected interface FunctionBuilder { Function build(Source source, List children, Configuration cfg); } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/AbstractConvertFunction.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/AbstractConvertFunction.java index 5e10e8984f56e..3b869c0200cb9 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/AbstractConvertFunction.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/AbstractConvertFunction.java @@ -43,7 +43,7 @@ * {@link org.elasticsearch.xpack.esql.expression.function.scalar}. *

*/ -public abstract class AbstractConvertFunction extends UnaryScalarFunction { +public abstract class AbstractConvertFunction extends UnaryScalarFunction implements ConvertFunction { // the numeric types convert functions need to handle; the other numeric types are converted upstream to one of these private static final List NUMERIC_TYPES = List.of(DataType.INTEGER, DataType.LONG, DataType.UNSIGNED_LONG, DataType.DOUBLE); @@ -76,11 +76,12 @@ protected TypeResolution resolveType() { return isTypeOrUnionType(field(), factories()::containsKey, sourceText(), null, supportedTypesNames(supportedTypes())); } + @Override public Set supportedTypes() { return factories().keySet(); } - private static String supportedTypesNames(Set types) { + static String supportedTypesNames(Set types) { List supportedTypesNames = new ArrayList<>(types.size()); HashSet supportTypes = new HashSet<>(types); if (supportTypes.containsAll(NUMERIC_TYPES)) { diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ConvertFunction.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ConvertFunction.java new file mode 100644 index 0000000000000..75f946c13a9f8 --- /dev/null +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ConvertFunction.java @@ -0,0 +1,31 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.esql.expression.function.scalar.convert; + +import org.elasticsearch.xpack.esql.core.expression.Expression; +import org.elasticsearch.xpack.esql.core.type.DataType; + +import java.util.List; +import java.util.Set; + +/** + * A function that converts from one type to another. + */ +public interface ConvertFunction { + /** + * Expression containing the values to be converted. + */ + Expression field(); + + /** + * The types that {@link #field()} can have. + */ + Set supportedTypes(); + + Expression replaceChildren(List newChildren); +} diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ParseIp.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ParseIp.java index e83c85614f24a..cba986f7e4533 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ParseIp.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ParseIp.java @@ -42,6 +42,14 @@ public class ParseIp { return new ParseIpLeadingZerosRejectedEvaluator.Factory(source, field, driverContext -> buildScratch(driverContext.breaker())); }; + static final AbstractConvertFunction.BuildFactory FROM_KEYWORD_LEADING_ZEROS_DECIMAL = (source, field) -> { + return new ParseIpLeadingZerosAreDecimalEvaluator.Factory(source, field, driverContext -> buildScratch(driverContext.breaker())); + }; + + static final AbstractConvertFunction.BuildFactory FROM_KEYWORD_LEADING_ZEROS_OCTAL = (source, field) -> { + return new ParseIpLeadingZerosAreOctalEvaluator.Factory(source, field, driverContext -> buildScratch(driverContext.breaker())); + }; + public static BreakingBytesRefBuilder buildScratch(CircuitBreaker breaker) { BreakingBytesRefBuilder scratch = new BreakingBytesRefBuilder(breaker, "to_ip", 16); scratch.setLength(InetAddressPoint.BYTES); diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIp.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIp.java new file mode 100644 index 0000000000000..dd25015a6e6e3 --- /dev/null +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIp.java @@ -0,0 +1,255 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.esql.expression.function.scalar.convert; + +import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.compute.operator.EvalOperator; +import org.elasticsearch.xpack.esql.core.expression.EntryExpression; +import org.elasticsearch.xpack.esql.core.expression.Expression; +import org.elasticsearch.xpack.esql.core.expression.Literal; +import org.elasticsearch.xpack.esql.core.expression.MapExpression; +import org.elasticsearch.xpack.esql.core.tree.NodeInfo; +import org.elasticsearch.xpack.esql.core.tree.Source; +import org.elasticsearch.xpack.esql.core.type.DataType; +import org.elasticsearch.xpack.esql.expression.SurrogateExpression; +import org.elasticsearch.xpack.esql.expression.function.Example; +import org.elasticsearch.xpack.esql.expression.function.FunctionInfo; +import org.elasticsearch.xpack.esql.expression.function.MapParam; +import org.elasticsearch.xpack.esql.expression.function.OptionalArgument; +import org.elasticsearch.xpack.esql.expression.function.Param; +import org.elasticsearch.xpack.esql.expression.function.scalar.EsqlScalarFunction; + +import java.io.IOException; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.ParamOrdinal.SECOND; +import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isMapExpression; +import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isTypeOrUnionType; +import static org.elasticsearch.xpack.esql.core.type.DataType.IP; +import static org.elasticsearch.xpack.esql.core.type.DataType.KEYWORD; +import static org.elasticsearch.xpack.esql.expression.function.scalar.convert.AbstractConvertFunction.supportedTypesNames; + +/** + * Converts strings to IPs. + *

+ * IPv4 addresses have traditionally parsed quads with leading zeros in three + * mutually exclusive ways: + *

+ *
    + *
  • As octal numbers. So {@code 1.1.010.1} becomes {@code 1.1.8.1}.
    • + *
  • As decimal numbers. So {@code 1.1.010.1} becomes {@code 1.1.10.1}.
    • + *
  • Rejects them entirely. So {@code 1.1.010.1} becomes {@code null} with a warning.
    • + *
+ *

+ * These three ways of handling leading zeros are available with the optional + * {@code leading_zeros} named parameter. Set to {@code octal}, {@code decimal}, + * or {@code reject}. If not sent this defaults to {@code reject} which has + * been Elasticsearch's traditional way of handling leading zeros for years. + *

+ *

+ * This doesn't extend from {@link AbstractConvertFunction} so that it can + * support a named parameter for the leading zeros behavior. Instead, it rewrites + * itself into either {@link ToIpLeadingZerosOctal}, {@link ToIpLeadingZerosDecimal}, + * or {@link ToIpLeadingZerosRejected} which are all {@link AbstractConvertFunction} + * subclasses. This keeps the conversion code happy while still allowing us to + * expose a single method to users. + *

+ */ +public class ToIp extends EsqlScalarFunction implements SurrogateExpression, OptionalArgument, ConvertFunction { + private static final String LEADING_ZEROS = "leading_zeros"; + public static final Map ALLOWED_OPTIONS = Map.ofEntries(Map.entry(LEADING_ZEROS, KEYWORD)); + + private final Expression field; + private final Expression options; + + @FunctionInfo( + returnType = "ip", + description = "Converts an input string to an IP value.", + examples = { + @Example(file = "ip", tag = "to_ip", explanation = """ + Note that in this example, the last conversion of the string isn’t possible. + When this happens, the result is a `null` value. In this case a _Warning_ header is added to the response. + The header will provide information on the source of the failure: + + `"Line 1:68: evaluation of [TO_IP(str2)] failed, treating result as null. Only first 20 failures recorded."` + + A following header will contain the failure reason and the offending value: + + `"java.lang.IllegalArgumentException: 'foo' is not an IP string literal."`"""), + @Example(file = "ip", tag = "to_ip_leading_zeros_octal", explanation = """ + Parse v4 addresses with leading zeros as octal. Like `ping` or `ftp`. + """), + @Example(file = "ip", tag = "to_ip_leading_zeros_decimal", explanation = """ + Parse v4 addresses with leading zeros as decimal. Java's `InetAddress.getByName`. + """) } + ) + public ToIp( + Source source, + @Param( + name = "field", + type = { "ip", "keyword", "text" }, + description = "Input value. The input can be a single- or multi-valued column or an expression." + ) Expression field, + @MapParam( + name = "options", + params = { + @MapParam.MapParamEntry( + name = "leading_zeros", + type = "keyword", + valueHint = { "reject", "octal", "decimal" }, + description = "What to do with leading 0s in IPv4 addresses." + ) }, + description = "(Optional) Additional options.", + optional = true + ) Expression options + ) { + super(source, options == null ? List.of(field) : List.of(field, options)); + this.field = field; + this.options = options; + } + + @Override + public String getWriteableName() { + throw new UnsupportedOperationException("not serialized"); + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + throw new UnsupportedOperationException("not serialized"); + } + + @Override + public DataType dataType() { + return IP; + } + + @Override + public Expression replaceChildren(List newChildren) { + return new ToIp(source(), newChildren.get(0), newChildren.size() == 1 ? null : newChildren.get(1)); + } + + @Override + protected NodeInfo info() { + return NodeInfo.create(this, ToIp::new, field, options); + } + + @Override + public EvalOperator.ExpressionEvaluator.Factory toEvaluator(ToEvaluator toEvaluator) { + throw new UnsupportedOperationException("should be rewritten"); + } + + @Override + public Expression surrogate() { + return LeadingZeros.from((MapExpression) options).surrogate(source(), field); + } + + @Override + public Expression field() { + return field; + } + + @Override + public Set supportedTypes() { + // All ToIpLeadingZeros* functions support the same input set. So we just pick one. + return ToIpLeadingZerosRejected.EVALUATORS.keySet(); + } + + @Override + protected TypeResolution resolveType() { + if (childrenResolved() == false) { + return new TypeResolution("Unresolved children"); + } + TypeResolution resolution = isTypeOrUnionType( + field, + ToIpLeadingZerosRejected.EVALUATORS::containsKey, + sourceText(), + null, + supportedTypesNames(supportedTypes()) + ); + if (resolution.unresolved()) { + return resolution; + } + if (options == null) { + return resolution; + } + resolution = isMapExpression(options, sourceText(), SECOND); + if (resolution.unresolved()) { + return resolution; + } + for (EntryExpression e : ((MapExpression) options).entryExpressions()) { + String key; + if (e.key().dataType() != KEYWORD) { + return new TypeResolution("map keys must be strings"); + } + if (e.key() instanceof Literal keyl) { + key = (String) keyl.value(); + } else { + return new TypeResolution("map keys must be literals"); + } + DataType expected = ALLOWED_OPTIONS.get(key); + if (expected == null) { + return new TypeResolution("[" + key + "] is not a supported option"); + } + + if (e.value().dataType() != expected) { + return new TypeResolution("[" + key + "] expects [" + expected + "] but was [" + e.value().dataType() + "]"); + } + if (e.value() instanceof Literal == false) { + return new TypeResolution("map values must be literals"); + } + } + try { + LeadingZeros.from((MapExpression) options); + } catch (IllegalArgumentException e) { + return new TypeResolution(e.getMessage()); + } + return TypeResolution.TYPE_RESOLVED; + } + + public enum LeadingZeros { + REJECT { + @Override + public Expression surrogate(Source source, Expression field) { + return new ToIpLeadingZerosRejected(source, field); + } + }, + DECIMAL { + @Override + public Expression surrogate(Source source, Expression field) { + return new ToIpLeadingZerosDecimal(source, field); + } + }, + OCTAL { + @Override + public Expression surrogate(Source source, Expression field) { + return new ToIpLeadingZerosOctal(source, field); + } + }; + + public static LeadingZeros from(MapExpression exp) { + if (exp == null) { + return REJECT; + } + Expression e = exp.keyFoldedMap().get(LEADING_ZEROS); + return e == null ? REJECT : from((String) ((Literal) e).value()); + } + + public static LeadingZeros from(String str) { + return switch (str) { + case "reject" -> REJECT; + case "octal" -> OCTAL; + case "decimal" -> DECIMAL; + default -> throw new IllegalArgumentException("Illegal leading_zeros [" + str + "]"); + }; + } + + public abstract Expression surrogate(Source source, Expression field); + } +} diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIpLeadingZerosDecimal.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIpLeadingZerosDecimal.java new file mode 100644 index 0000000000000..2754dcf39ff73 --- /dev/null +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIpLeadingZerosDecimal.java @@ -0,0 +1,71 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.esql.expression.function.scalar.convert; + +import org.elasticsearch.common.io.stream.NamedWriteableRegistry; +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.xpack.esql.core.expression.Expression; +import org.elasticsearch.xpack.esql.core.tree.NodeInfo; +import org.elasticsearch.xpack.esql.core.tree.Source; +import org.elasticsearch.xpack.esql.core.type.DataType; + +import java.io.IOException; +import java.util.List; +import java.util.Map; + +import static org.elasticsearch.xpack.esql.core.type.DataType.IP; +import static org.elasticsearch.xpack.esql.core.type.DataType.KEYWORD; +import static org.elasticsearch.xpack.esql.core.type.DataType.TEXT; +import static org.elasticsearch.xpack.esql.expression.function.scalar.convert.ParseIp.FROM_KEYWORD_LEADING_ZEROS_DECIMAL; + +public class ToIpLeadingZerosDecimal extends AbstractConvertFunction { + public static final NamedWriteableRegistry.Entry ENTRY = new NamedWriteableRegistry.Entry( + Expression.class, + "ToIpLeadingZerosDecimal", + ToIpLeadingZerosDecimal::new + ); + + static final Map EVALUATORS = Map.ofEntries( + Map.entry(IP, (source, field) -> field), + Map.entry(KEYWORD, FROM_KEYWORD_LEADING_ZEROS_DECIMAL), + Map.entry(TEXT, FROM_KEYWORD_LEADING_ZEROS_DECIMAL) + ); + + public ToIpLeadingZerosDecimal(Source source, Expression field) { + super(source, field); + } + + private ToIpLeadingZerosDecimal(StreamInput in) throws IOException { + super(in); + } + + @Override + public String getWriteableName() { + return ENTRY.name; + } + + @Override + protected Map factories() { + return EVALUATORS; + } + + @Override + public DataType dataType() { + return IP; + } + + @Override + public Expression replaceChildren(List newChildren) { + return new ToIpLeadingZerosDecimal(source(), newChildren.get(0)); + } + + @Override + protected NodeInfo info() { + return NodeInfo.create(this, ToIpLeadingZerosDecimal::new, field()); + } +} diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIpLeadingZerosOctal.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIpLeadingZerosOctal.java new file mode 100644 index 0000000000000..404b6c27519c8 --- /dev/null +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIpLeadingZerosOctal.java @@ -0,0 +1,71 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.esql.expression.function.scalar.convert; + +import org.elasticsearch.common.io.stream.NamedWriteableRegistry; +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.xpack.esql.core.expression.Expression; +import org.elasticsearch.xpack.esql.core.tree.NodeInfo; +import org.elasticsearch.xpack.esql.core.tree.Source; +import org.elasticsearch.xpack.esql.core.type.DataType; + +import java.io.IOException; +import java.util.List; +import java.util.Map; + +import static org.elasticsearch.xpack.esql.core.type.DataType.IP; +import static org.elasticsearch.xpack.esql.core.type.DataType.KEYWORD; +import static org.elasticsearch.xpack.esql.core.type.DataType.TEXT; +import static org.elasticsearch.xpack.esql.expression.function.scalar.convert.ParseIp.FROM_KEYWORD_LEADING_ZEROS_OCTAL; + +public class ToIpLeadingZerosOctal extends AbstractConvertFunction { + public static final NamedWriteableRegistry.Entry ENTRY = new NamedWriteableRegistry.Entry( + Expression.class, + "ToIpLeadingZerosOctal", + ToIpLeadingZerosOctal::new + ); + + static final Map EVALUATORS = Map.ofEntries( + Map.entry(IP, (source, field) -> field), + Map.entry(KEYWORD, FROM_KEYWORD_LEADING_ZEROS_OCTAL), + Map.entry(TEXT, FROM_KEYWORD_LEADING_ZEROS_OCTAL) + ); + + public ToIpLeadingZerosOctal(Source source, Expression field) { + super(source, field); + } + + private ToIpLeadingZerosOctal(StreamInput in) throws IOException { + super(in); + } + + @Override + public String getWriteableName() { + return ENTRY.name; + } + + @Override + protected Map factories() { + return EVALUATORS; + } + + @Override + public DataType dataType() { + return IP; + } + + @Override + public Expression replaceChildren(List newChildren) { + return new ToIpLeadingZerosOctal(source(), newChildren.get(0)); + } + + @Override + protected NodeInfo info() { + return NodeInfo.create(this, ToIpLeadingZerosOctal::new, field()); + } +} diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIP.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIpLeadingZerosRejected.java similarity index 52% rename from x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIP.java rename to x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIpLeadingZerosRejected.java index 77f26269b1c79..a02f08b50c045 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIP.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIpLeadingZerosRejected.java @@ -13,9 +13,6 @@ import org.elasticsearch.xpack.esql.core.tree.NodeInfo; import org.elasticsearch.xpack.esql.core.tree.Source; import org.elasticsearch.xpack.esql.core.type.DataType; -import org.elasticsearch.xpack.esql.expression.function.Example; -import org.elasticsearch.xpack.esql.expression.function.FunctionInfo; -import org.elasticsearch.xpack.esql.expression.function.Param; import java.io.IOException; import java.util.List; @@ -26,41 +23,28 @@ import static org.elasticsearch.xpack.esql.core.type.DataType.TEXT; import static org.elasticsearch.xpack.esql.expression.function.scalar.convert.ParseIp.FROM_KEYWORD_LEADING_ZEROS_REJECTED; -public class ToIP extends AbstractConvertFunction { - public static final NamedWriteableRegistry.Entry ENTRY = new NamedWriteableRegistry.Entry(Expression.class, "ToIP", ToIP::new); +public class ToIpLeadingZerosRejected extends AbstractConvertFunction { + public static final NamedWriteableRegistry.Entry ENTRY = new NamedWriteableRegistry.Entry( + Expression.class, + /* + * This is the name a function with this behavior has had since the + * dawn of ESQL. The ToIp function that exists now is not serialized. + */ + "ToIP", + ToIpLeadingZerosRejected::new + ); - private static final Map EVALUATORS = Map.ofEntries( + static final Map EVALUATORS = Map.ofEntries( Map.entry(IP, (source, field) -> field), Map.entry(KEYWORD, FROM_KEYWORD_LEADING_ZEROS_REJECTED), Map.entry(TEXT, FROM_KEYWORD_LEADING_ZEROS_REJECTED) ); - @FunctionInfo( - returnType = "ip", - description = "Converts an input string to an IP value.", - examples = @Example(file = "ip", tag = "to_ip", explanation = """ - Note that in this example, the last conversion of the string isn’t possible. - When this happens, the result is a `null` value. In this case a _Warning_ header is added to the response. - The header will provide information on the source of the failure: - - `"Line 1:68: evaluation of [TO_IP(str2)] failed, treating result as null. Only first 20 failures recorded."` - - A following header will contain the failure reason and the offending value: - - `"java.lang.IllegalArgumentException: 'foo' is not an IP string literal."`""") - ) - public ToIP( - Source source, - @Param( - name = "field", - type = { "ip", "keyword", "text" }, - description = "Input value. The input can be a single- or multi-valued column or an expression." - ) Expression field - ) { + public ToIpLeadingZerosRejected(Source source, Expression field) { super(source, field); } - private ToIP(StreamInput in) throws IOException { + private ToIpLeadingZerosRejected(StreamInput in) throws IOException { super(in); } @@ -81,11 +65,11 @@ public DataType dataType() { @Override public Expression replaceChildren(List newChildren) { - return new ToIP(source(), newChildren.get(0)); + return new ToIpLeadingZerosRejected(source(), newChildren.get(0)); } @Override protected NodeInfo info() { - return NodeInfo.create(this, ToIP::new, field()); + return NodeInfo.create(this, ToIpLeadingZerosRejected::new, field()); } } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/SpatialRelatesFunction.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/SpatialRelatesFunction.java index 295116a5e99c2..0316f56b297da 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/SpatialRelatesFunction.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/SpatialRelatesFunction.java @@ -34,6 +34,7 @@ import org.elasticsearch.xpack.esql.core.util.Check; import org.elasticsearch.xpack.esql.core.util.SpatialCoordinateTypes; import org.elasticsearch.xpack.esql.evaluator.mapper.EvaluatorMapper; +import org.elasticsearch.xpack.esql.expression.SurrogateExpression; import org.elasticsearch.xpack.esql.optimizer.rules.physical.local.LucenePushdownPredicates; import org.elasticsearch.xpack.esql.planner.TranslatorHandler; import org.elasticsearch.xpack.esql.querydsl.query.SpatialRelatesQuery; @@ -48,7 +49,8 @@ public abstract class SpatialRelatesFunction extends BinarySpatialFunction implements EvaluatorMapper, SpatialEvaluatorFactory.SpatialSourceSupplier, - TranslationAware { + TranslationAware, + SurrogateExpression { protected SpatialRelatesFunction(Source source, Expression left, Expression right, boolean leftDocValues, boolean rightDocValues) { super(source, left, right, leftDocValues, rightDocValues, false); @@ -73,6 +75,7 @@ public DataType dataType() { /** * Some spatial functions can replace themselves with alternatives that are more efficient for certain cases. */ + @Override public SpatialRelatesFunction surrogate() { return this; } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizer.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizer.java index 6f98b1da758c5..64d47ea987eac 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizer.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizer.java @@ -58,9 +58,9 @@ import org.elasticsearch.xpack.esql.optimizer.rules.logical.SkipQueryOnLimitZero; import org.elasticsearch.xpack.esql.optimizer.rules.logical.SplitInWithFoldableValue; import org.elasticsearch.xpack.esql.optimizer.rules.logical.SubstituteFilteredExpression; -import org.elasticsearch.xpack.esql.optimizer.rules.logical.SubstituteSpatialSurrogates; +import org.elasticsearch.xpack.esql.optimizer.rules.logical.SubstituteSurrogateAggregations; +import org.elasticsearch.xpack.esql.optimizer.rules.logical.SubstituteSurrogateExpressions; import org.elasticsearch.xpack.esql.optimizer.rules.logical.SubstituteSurrogatePlans; -import org.elasticsearch.xpack.esql.optimizer.rules.logical.SubstituteSurrogates; import org.elasticsearch.xpack.esql.optimizer.rules.logical.TranslateTimeSeriesAggregate; import org.elasticsearch.xpack.esql.plan.logical.LogicalPlan; import org.elasticsearch.xpack.esql.rule.ParameterizedRuleExecutor; @@ -137,7 +137,7 @@ protected static Batch substitutions() { // then extract nested aggs top-level new ReplaceAggregateAggExpressionWithEval(), // lastly replace surrogate functions - new SubstituteSurrogates(), + new SubstituteSurrogateAggregations(), // translate metric aggregates after surrogate substitution and replace nested expressions with eval (again) new TranslateTimeSeriesAggregate(), new PruneUnusedIndexMode(), @@ -149,7 +149,7 @@ protected static Batch substitutions() { new ReplaceTrivialTypeConversions(), new ReplaceAliasingEvalWithProject(), new SkipQueryOnEmptyMappings(), - new SubstituteSpatialSurrogates(), + new SubstituteSurrogateExpressions(), new ReplaceOrderByExpressionWithEval() // new NormalizeAggregate(), - waits on https://github.com/elastic/elasticsearch/issues/100634 ); diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/SubstituteSpatialSurrogates.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/SubstituteSpatialSurrogates.java deleted file mode 100644 index 4b68ee941bc92..0000000000000 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/SubstituteSpatialSurrogates.java +++ /dev/null @@ -1,30 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0; you may not use this file except in compliance with the Elastic License - * 2.0. - */ - -package org.elasticsearch.xpack.esql.optimizer.rules.logical; - -import org.elasticsearch.xpack.esql.expression.function.scalar.spatial.SpatialRelatesFunction; -import org.elasticsearch.xpack.esql.optimizer.LogicalOptimizerContext; - -/** - * Currently this works similarly to SurrogateExpression, leaving the logic inside the expressions, - * so each can decide for itself whether or not to change to a surrogate expression. - * But what is actually being done is similar to LiteralsOnTheRight. We can consider in the future moving - * this in either direction, reducing the number of rules, but for now, - * it's a separate rule to reduce the risk of unintended interactions with other rules. - */ -public final class SubstituteSpatialSurrogates extends OptimizerRules.OptimizerExpressionRule { - - public SubstituteSpatialSurrogates() { - super(OptimizerRules.TransformDirection.UP); - } - - @Override - protected SpatialRelatesFunction rule(SpatialRelatesFunction function, LogicalOptimizerContext ctx) { - return function.surrogate(); - } -} diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/SubstituteSurrogates.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/SubstituteSurrogateAggregations.java similarity index 97% rename from x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/SubstituteSurrogates.java rename to x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/SubstituteSurrogateAggregations.java index 1831bfaf30f33..bcc72fc24ddb8 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/SubstituteSurrogates.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/SubstituteSurrogateAggregations.java @@ -32,10 +32,8 @@ import java.util.List; import java.util.Map; -public final class SubstituteSurrogates extends OptimizerRules.OptimizerRule { - // TODO: currently this rule only works for aggregate functions (AVG) - - public SubstituteSurrogates() { +public final class SubstituteSurrogateAggregations extends OptimizerRules.OptimizerRule { + public SubstituteSurrogateAggregations() { super(OptimizerRules.TransformDirection.UP); } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/SubstituteSurrogateExpressions.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/SubstituteSurrogateExpressions.java new file mode 100644 index 0000000000000..bccf119b423b3 --- /dev/null +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/SubstituteSurrogateExpressions.java @@ -0,0 +1,33 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.esql.optimizer.rules.logical; + +import org.elasticsearch.xpack.esql.core.expression.Expression; +import org.elasticsearch.xpack.esql.expression.SurrogateExpression; +import org.elasticsearch.xpack.esql.optimizer.LogicalOptimizerContext; + +/** + * Replace {@link SurrogateExpression}s with their {@link SurrogateExpression#surrogate surrogates}. + */ +public final class SubstituteSurrogateExpressions extends OptimizerRules.OptimizerExpressionRule { + + public SubstituteSurrogateExpressions() { + super(OptimizerRules.TransformDirection.UP); + } + + @Override + protected Expression rule(Expression e, LogicalOptimizerContext ctx) { + if (e instanceof SurrogateExpression s) { + Expression surrogate = s.surrogate(); + if (surrogate != null) { + return surrogate; + } + } + return e; + } +} 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 11a8c5c5f5b98..6cde1f28535dd 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 @@ -45,8 +45,8 @@ import org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToDouble; import org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToGeoPoint; import org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToGeoShape; -import org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToIP; import org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToInteger; +import org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToIpLeadingZerosRejected; import org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToLong; import org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToString; import org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToTimeDuration; @@ -122,7 +122,7 @@ public class EsqlDataTypeConverter { entry(GEO_POINT, ToGeoPoint::new), entry(GEO_SHAPE, ToGeoShape::new), entry(INTEGER, ToInteger::new), - entry(IP, ToIP::new), + entry(IP, ToIpLeadingZerosRejected::new), entry(LONG, ToLong::new), // ToRadians, typeless entry(KEYWORD, ToString::new), diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/ParsingTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/ParsingTests.java index 81022c5d69c35..e391b8ca1fa68 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/ParsingTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/ParsingTests.java @@ -13,10 +13,9 @@ import org.elasticsearch.xcontent.json.JsonXContent; import org.elasticsearch.xpack.esql.LoadMapping; import org.elasticsearch.xpack.esql.action.EsqlCapabilities; -import org.elasticsearch.xpack.esql.core.expression.Expression; +import org.elasticsearch.xpack.esql.core.expression.function.Function; import org.elasticsearch.xpack.esql.core.type.DataType; import org.elasticsearch.xpack.esql.expression.function.EsqlFunctionRegistry; -import org.elasticsearch.xpack.esql.expression.function.FunctionDefinition; import org.elasticsearch.xpack.esql.index.EsIndex; import org.elasticsearch.xpack.esql.index.IndexResolution; import org.elasticsearch.xpack.esql.parser.EsqlParser; @@ -100,9 +99,9 @@ public void testInlineCast() throws IOException { LogicalPlan plan = parser.createStatement("ROW a = 1::" + nameOrAlias); Row row = as(plan, Row.class); assertThat(row.fields(), hasSize(1)); - Expression functionCall = row.fields().get(0).child(); + Function functionCall = (Function) row.fields().get(0).child(); assertThat(functionCall.dataType(), equalTo(expectedType)); - report.field(nameOrAlias, functionName(registry, functionCall)); + report.field(nameOrAlias, registry.functionName(functionCall.getClass())); } report.endObject(); } @@ -157,15 +156,6 @@ public void testJoinTwiceOnTheSameField_TwoLookups() { ); } - private String functionName(EsqlFunctionRegistry registry, Expression functionCall) { - for (FunctionDefinition def : registry.listFunctions()) { - if (functionCall.getClass().equals(def.clazz())) { - return def.name(); - } - } - throw new IllegalArgumentException("can't find name for " + functionCall); - } - private String error(String query) { ParsingException e = expectThrows(ParsingException.class, () -> defaultAnalyzer.analyze(parser.createStatement(query))); String message = e.getMessage(); diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/AbstractFunctionTestCase.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/AbstractFunctionTestCase.java index d6f6b906a5d8f..e220eac5ab7a1 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/AbstractFunctionTestCase.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/AbstractFunctionTestCase.java @@ -42,6 +42,7 @@ import org.elasticsearch.xpack.esql.core.util.StringUtils; import org.elasticsearch.xpack.esql.evaluator.EvalMapper; import org.elasticsearch.xpack.esql.evaluator.mapper.EvaluatorMapper; +import org.elasticsearch.xpack.esql.expression.SurrogateExpression; import org.elasticsearch.xpack.esql.expression.function.scalar.conditional.Greatest; import org.elasticsearch.xpack.esql.expression.function.scalar.nulls.Coalesce; import org.elasticsearch.xpack.esql.io.stream.PlanStreamOutput; @@ -484,13 +485,21 @@ protected final Expression buildDeepCopyOfFieldExpression(TestCaseSupplier.TestC } private Expression randomSerializeDeserialize(Expression expression) { - if (randomBoolean()) { + if (canSerialize() == false || randomBoolean()) { return expression; } return serializeDeserializeExpression(expression); } + /** + * The expression being tested be serialized? The vast + * majority of expressions can be serialized. + */ + protected boolean canSerialize() { + return true; + } + /** * Returns the expression after being serialized and deserialized. *

@@ -545,6 +554,12 @@ public static ExpressionEvaluator.Factory evaluator(Expression e) { if (e.foldable()) { e = new Literal(e.source(), e.fold(FoldContext.small()), e.dataType()); } + if (e instanceof SurrogateExpression s) { + Expression surrogate = s.surrogate(); + if (surrogate != null) { + e = surrogate; + } + } Layout.Builder builder = new Layout.Builder(); buildLayout(builder, e); Expression.TypeResolution resolution = e.typeResolved(); @@ -705,6 +720,7 @@ private static BytesRef randomizeBytesRefOffset(BytesRef bytesRef) { } public void testSerializationOfSimple() { + assumeTrue("can't serialize function", canSerialize()); assertSerialization(buildFieldExpression(testCase), testCase.getConfiguration()); } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/AbstractScalarFunctionTestCase.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/AbstractScalarFunctionTestCase.java index 0cee8fe3f57cb..f056e0c61c8d1 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/AbstractScalarFunctionTestCase.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/AbstractScalarFunctionTestCase.java @@ -25,6 +25,7 @@ import org.elasticsearch.xpack.esql.core.expression.FoldContext; import org.elasticsearch.xpack.esql.core.type.DataType; import org.elasticsearch.xpack.esql.core.util.NumericUtils; +import org.elasticsearch.xpack.esql.expression.SurrogateExpression; import org.elasticsearch.xpack.esql.optimizer.rules.logical.FoldNull; import org.elasticsearch.xpack.esql.planner.PlannerUtils; import org.hamcrest.Matcher; @@ -366,6 +367,12 @@ public void testFold() { return; } assertFalse("expected resolved", expression.typeResolved().unresolved()); + if (expression instanceof SurrogateExpression s) { + Expression surrogate = s.surrogate(); + if (surrogate != null) { + expression = surrogate; + } + } Expression nullOptimized = new FoldNull().rule(expression, unboundLogicalOptimizerContext()); assertThat(nullOptimized.dataType(), equalTo(testCase.expectedType())); assertTrue(nullOptimized.foldable()); diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIPTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIPTests.java deleted file mode 100644 index e666d7c6defe9..0000000000000 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIPTests.java +++ /dev/null @@ -1,87 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0; you may not use this file except in compliance with the Elastic License - * 2.0. - */ - -package org.elasticsearch.xpack.esql.expression.function.scalar.convert; - -import com.carrotsearch.randomizedtesting.annotations.Name; -import com.carrotsearch.randomizedtesting.annotations.ParametersFactory; - -import org.apache.lucene.util.BytesRef; -import org.elasticsearch.common.network.NetworkAddress; -import org.elasticsearch.test.ESTestCase; -import org.elasticsearch.xpack.esql.core.expression.Expression; -import org.elasticsearch.xpack.esql.core.tree.Source; -import org.elasticsearch.xpack.esql.core.type.DataType; -import org.elasticsearch.xpack.esql.expression.function.AbstractScalarFunctionTestCase; -import org.elasticsearch.xpack.esql.expression.function.TestCaseSupplier; - -import java.util.ArrayList; -import java.util.List; -import java.util.function.Supplier; - -import static java.util.Collections.emptyList; -import static org.elasticsearch.xpack.esql.core.util.StringUtils.parseIP; - -public class ToIPTests extends AbstractScalarFunctionTestCase { - public ToIPTests(@Name("TestCase") Supplier testCaseSupplier) { - this.testCase = testCaseSupplier.get(); - } - - @ParametersFactory - public static Iterable parameters() { - String read = "Attribute[channel=0]"; - String stringEvaluator = "ParseIpLeadingZerosRejectedEvaluator[string=" + read + "]"; - List suppliers = new ArrayList<>(); - - // convert from IP to IP - TestCaseSupplier.forUnaryIp(suppliers, read, DataType.IP, v -> v, List.of()); - - // convert random string (i.e. not an IP representation) to IP `null`, with warnings. - TestCaseSupplier.forUnaryStrings( - suppliers, - stringEvaluator, - DataType.IP, - bytesRef -> null, - bytesRef -> List.of( - "Line 1:1: evaluation of [source] failed, treating result as null. Only first 20 failures recorded.", - "Line 1:1: java.lang.IllegalArgumentException: '" + bytesRef.utf8ToString() + "' is not an IP string literal." - ) - ); - - // convert valid IPs shaped as strings - TestCaseSupplier.unary( - suppliers, - stringEvaluator, - validIPsAsStrings(), - DataType.IP, - bytesRef -> parseIP(((BytesRef) bytesRef).utf8ToString()), - emptyList() - ); - return parameterSuppliersFromTypedDataWithDefaultChecksNoErrors(true, suppliers); - } - - @Override - protected Expression build(Source source, List args) { - return new ToIP(source, args.get(0)); - } - - private static List validIPsAsStrings() { - return List.of( - new TestCaseSupplier.TypedDataSupplier("<127.0.0.1 ip>", () -> new BytesRef("127.0.0.1"), DataType.KEYWORD), - new TestCaseSupplier.TypedDataSupplier( - "", - () -> new BytesRef(NetworkAddress.format(ESTestCase.randomIp(true))), - DataType.KEYWORD - ), - new TestCaseSupplier.TypedDataSupplier( - "", - () -> new BytesRef(NetworkAddress.format(ESTestCase.randomIp(false))), - DataType.TEXT - ) - ); - } -} diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIPErrorTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIpErrorTests.java similarity index 81% rename from x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIPErrorTests.java rename to x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIpErrorTests.java index 31dd36041a6fa..c57ce3073b211 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIPErrorTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIpErrorTests.java @@ -7,6 +7,7 @@ package org.elasticsearch.xpack.esql.expression.function.scalar.convert; +import org.elasticsearch.common.collect.Iterators; import org.elasticsearch.xpack.esql.core.expression.Expression; import org.elasticsearch.xpack.esql.core.tree.Source; import org.elasticsearch.xpack.esql.core.type.DataType; @@ -19,15 +20,15 @@ import static org.hamcrest.Matchers.equalTo; -public class ToIPErrorTests extends ErrorsForCasesWithoutExamplesTestCase { +public class ToIpErrorTests extends ErrorsForCasesWithoutExamplesTestCase { @Override protected List cases() { - return paramsToSuppliers(ToIPTests.parameters()); + return Iterators.toList(Iterators.map(ToIpTests.parameters().iterator(), p -> (TestCaseSupplier) p[0])); } @Override protected Expression build(Source source, List args) { - return new ToIP(source, args.get(0)); + return new ToIp(source, args.getFirst(), null); } @Override diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIpLeadingZerosDecimalSerializationTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIpLeadingZerosDecimalSerializationTests.java new file mode 100644 index 0000000000000..0d2c11d77cac6 --- /dev/null +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIpLeadingZerosDecimalSerializationTests.java @@ -0,0 +1,19 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.esql.expression.function.scalar.convert; + +import org.elasticsearch.xpack.esql.core.expression.Expression; +import org.elasticsearch.xpack.esql.core.tree.Source; +import org.elasticsearch.xpack.esql.expression.AbstractUnaryScalarSerializationTests; + +public class ToIpLeadingZerosDecimalSerializationTests extends AbstractUnaryScalarSerializationTests { + @Override + protected ToIpLeadingZerosDecimal create(Source source, Expression child) { + return new ToIpLeadingZerosDecimal(source, child); + } +} diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIPSerializationTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIpLeadingZerosOctalSerializationTests.java similarity index 68% rename from x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIPSerializationTests.java rename to x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIpLeadingZerosOctalSerializationTests.java index 76657639a5836..9c67f5d23142f 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIPSerializationTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIpLeadingZerosOctalSerializationTests.java @@ -11,9 +11,9 @@ import org.elasticsearch.xpack.esql.core.tree.Source; import org.elasticsearch.xpack.esql.expression.AbstractUnaryScalarSerializationTests; -public class ToIPSerializationTests extends AbstractUnaryScalarSerializationTests { +public class ToIpLeadingZerosOctalSerializationTests extends AbstractUnaryScalarSerializationTests { @Override - protected ToIP create(Source source, Expression child) { - return new ToIP(source, child); + protected ToIpLeadingZerosOctal create(Source source, Expression child) { + return new ToIpLeadingZerosOctal(source, child); } } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIpLeadingZerosRejectedSerializationTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIpLeadingZerosRejectedSerializationTests.java new file mode 100644 index 0000000000000..85281192ca1e0 --- /dev/null +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIpLeadingZerosRejectedSerializationTests.java @@ -0,0 +1,19 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.esql.expression.function.scalar.convert; + +import org.elasticsearch.xpack.esql.core.expression.Expression; +import org.elasticsearch.xpack.esql.core.tree.Source; +import org.elasticsearch.xpack.esql.expression.AbstractUnaryScalarSerializationTests; + +public class ToIpLeadingZerosRejectedSerializationTests extends AbstractUnaryScalarSerializationTests { + @Override + protected ToIpLeadingZerosRejected create(Source source, Expression child) { + return new ToIpLeadingZerosRejected(source, child); + } +} diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIpTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIpTests.java new file mode 100644 index 0000000000000..b41177f83c58d --- /dev/null +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIpTests.java @@ -0,0 +1,197 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.esql.expression.function.scalar.convert; + +import com.carrotsearch.randomizedtesting.annotations.Name; +import com.carrotsearch.randomizedtesting.annotations.ParametersFactory; + +import org.apache.lucene.util.BytesRef; +import org.elasticsearch.common.network.NetworkAddress; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.xpack.esql.core.expression.Expression; +import org.elasticsearch.xpack.esql.core.expression.Literal; +import org.elasticsearch.xpack.esql.core.expression.MapExpression; +import org.elasticsearch.xpack.esql.core.tree.Source; +import org.elasticsearch.xpack.esql.core.type.DataType; +import org.elasticsearch.xpack.esql.expression.function.AbstractScalarFunctionTestCase; +import org.elasticsearch.xpack.esql.expression.function.TestCaseSupplier; +import org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter; + +import java.util.ArrayList; +import java.util.List; +import java.util.Locale; +import java.util.function.Supplier; + +import static java.util.Collections.emptyList; +import static org.elasticsearch.xpack.esql.core.util.StringUtils.parseIP; +import static org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToIp.LeadingZeros.DECIMAL; +import static org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToIp.LeadingZeros.OCTAL; +import static org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToIp.LeadingZeros.REJECT; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.nullValue; + +public class ToIpTests extends AbstractScalarFunctionTestCase { + private final ToIp.LeadingZeros leadingZeros; + + public ToIpTests( + @Name("TestCase") Supplier testCaseSupplier, + @Name("leading_zeros") ToIp.LeadingZeros leadingZeros + ) { + this.testCase = testCaseSupplier.get(); + this.leadingZeros = leadingZeros; + } + + @ParametersFactory + public static Iterable parameters() { + List parameters = new ArrayList<>(); + for (ToIp.LeadingZeros leadingZeros : new ToIp.LeadingZeros[] { null, REJECT, OCTAL, DECIMAL }) { + List suppliers = new ArrayList<>(); + // convert from IP to IP + TestCaseSupplier.forUnaryIp(suppliers, readEvaluator(), DataType.IP, v -> v, List.of()); + + // convert random string (i.e. not an IP representation) to IP `null`, with warnings. + TestCaseSupplier.forUnaryStrings( + suppliers, + stringEvaluator(leadingZeros), + DataType.IP, + bytesRef -> null, + bytesRef -> List.of( + "Line 1:1: evaluation of [source] failed, treating result as null. Only first 20 failures recorded.", + "Line 1:1: java.lang.IllegalArgumentException: '" + bytesRef.utf8ToString() + "' is not an IP string literal." + ) + ); + + // convert valid IPs shaped as strings + TestCaseSupplier.unary( + suppliers, + stringEvaluator(leadingZeros), + validIPsAsStrings(), + DataType.IP, + bytesRef -> parseIP(((BytesRef) bytesRef).utf8ToString()), + emptyList() + ); + suppliers = anyNullIsNull(true, randomizeBytesRefsOffset(suppliers)); + for (TestCaseSupplier supplier : suppliers) { + parameters.add(new Object[] { supplier, leadingZeros }); + } + } + + parameters.add(new Object[] { exampleRejectingLeadingZeros(stringEvaluator(null)), null }); + parameters.add(new Object[] { exampleRejectingLeadingZeros(stringEvaluator(REJECT)), REJECT }); + parameters.add(new Object[] { exampleParsingLeadingZerosAsDecimal(stringEvaluator(DECIMAL)), DECIMAL }); + parameters.add(new Object[] { exampleParsingLeadingZerosAsOctal(stringEvaluator(OCTAL)), OCTAL }); + return parameters; + } + + private static TestCaseSupplier exampleRejectingLeadingZeros(String stringEvaluator) { + return new TestCaseSupplier(" with leading 0s", List.of(DataType.KEYWORD), () -> { + BytesRef withLeadingZeros = new BytesRef(randomIpWithLeadingZeros()); + return new TestCaseSupplier.TestCase( + List.of(new TestCaseSupplier.TypedData(withLeadingZeros, DataType.KEYWORD, "ip")), + stringEvaluator, + DataType.IP, + nullValue() + ).withWarning("Line 1:1: evaluation of [source] failed, treating result as null. Only first 20 failures recorded.") + .withWarning( + "Line 1:1: java.lang.IllegalArgumentException: '" + withLeadingZeros.utf8ToString() + "' is not an IP string literal." + ); + }); + } + + private static TestCaseSupplier exampleParsingLeadingZerosAsDecimal(String stringEvaluator) { + return new TestCaseSupplier(" with leading 0s", List.of(DataType.KEYWORD), () -> { + String ip = randomIpWithLeadingZeros(); + BytesRef withLeadingZeros = new BytesRef(ip); + String withoutLeadingZeros = ParseIpTests.leadingZerosAreDecimalToIp(ip); + return new TestCaseSupplier.TestCase( + List.of(new TestCaseSupplier.TypedData(withLeadingZeros, DataType.KEYWORD, "ip")), + stringEvaluator, + DataType.IP, + equalTo(EsqlDataTypeConverter.stringToIP(withoutLeadingZeros)) + ); + }); + } + + private static TestCaseSupplier exampleParsingLeadingZerosAsOctal(String stringEvaluator) { + return new TestCaseSupplier(" with leading 0s", List.of(DataType.KEYWORD), () -> { + String ip = randomIpWithLeadingZerosOctal(); + BytesRef withLeadingZeros = new BytesRef(ip); + String withoutLeadingZeros = ParseIpTests.leadingZerosAreOctalToIp(ip); + return new TestCaseSupplier.TestCase( + List.of(new TestCaseSupplier.TypedData(withLeadingZeros, DataType.KEYWORD, "ip")), + stringEvaluator, + DataType.IP, + equalTo(EsqlDataTypeConverter.stringToIP(withoutLeadingZeros)) + ); + }); + } + + @Override + protected Expression build(Source source, List args) { + return new ToIp(source, args.getFirst(), options()); + } + + private MapExpression options() { + if (leadingZeros == null) { + return null; + } + return new MapExpression( + Source.EMPTY, + List.of( + new Literal(Source.EMPTY, "leading_zeros", DataType.KEYWORD), + new Literal(Source.EMPTY, leadingZeros.toString().toLowerCase(Locale.ROOT), DataType.KEYWORD) + ) + ); + } + + private static List validIPsAsStrings() { + return List.of( + new TestCaseSupplier.TypedDataSupplier("<127.0.0.1 ip>", () -> new BytesRef("127.0.0.1"), DataType.KEYWORD), + new TestCaseSupplier.TypedDataSupplier( + "", + () -> new BytesRef(NetworkAddress.format(ESTestCase.randomIp(true))), + DataType.KEYWORD + ), + new TestCaseSupplier.TypedDataSupplier( + "", + () -> new BytesRef(NetworkAddress.format(ESTestCase.randomIp(false))), + DataType.TEXT + ) + ); + } + + private static String randomIpWithLeadingZeros() { + return randomValueOtherThanMany((String str) -> false == (str.startsWith("0") || str.contains(".0")), () -> { + byte[] addr = randomIp(true).getAddress(); + return String.format(Locale.ROOT, "%03d.%03d.%03d.%03d", addr[0] & 0xff, addr[1] & 0xff, addr[2] & 0xff, addr[3] & 0xff); + }); + } + + private static String randomIpWithLeadingZerosOctal() { + byte[] addr = randomIp(true).getAddress(); + return String.format(Locale.ROOT, "0%o.0%o.0%o.0%o", addr[0] & 0xff, addr[1] & 0xff, addr[2] & 0xff, addr[3] & 0xff); + } + + private static String readEvaluator() { + return "Attribute[channel=0]"; + } + + private static String stringEvaluator(ToIp.LeadingZeros leadingZeros) { + return switch (leadingZeros) { + case null -> "ParseIpLeadingZerosRejectedEvaluator"; + case REJECT -> "ParseIpLeadingZerosRejectedEvaluator"; + case DECIMAL -> "ParseIpLeadingZerosAreDecimalEvaluator"; + case OCTAL -> "ParseIpLeadingZerosAreOctalEvaluator"; + } + "[string=" + readEvaluator() + "]"; + } + + @Override + protected boolean canSerialize() { + return false; + } +} diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/type/MultiTypeEsFieldTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/type/MultiTypeEsFieldTests.java index 987ab103cf80b..154605a199d22 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/type/MultiTypeEsFieldTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/type/MultiTypeEsFieldTests.java @@ -24,8 +24,8 @@ import org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToDouble; import org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToGeoPoint; import org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToGeoShape; -import org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToIP; import org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToInteger; +import org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToIpLeadingZerosRejected; import org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToLong; import org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToString; import org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToVersion; @@ -156,7 +156,7 @@ private static Expression testConvertExpression(String name, DataType fromType, case DOUBLE, FLOAT -> new ToDouble(Source.EMPTY, fromField); case INTEGER -> new ToInteger(Source.EMPTY, fromField); case LONG -> new ToLong(Source.EMPTY, fromField); - case IP -> new ToIP(Source.EMPTY, fromField); + case IP -> new ToIpLeadingZerosRejected(Source.EMPTY, fromField); case KEYWORD -> new ToString(Source.EMPTY, fromField); case GEO_POINT -> new ToGeoPoint(Source.EMPTY, fromField); case GEO_SHAPE -> new ToGeoShape(Source.EMPTY, fromField); From 11c639121adaca6bd48cc4d23cdd647456324710 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Wed, 9 Apr 2025 09:40:32 -0400 Subject: [PATCH 2/7] Update docs/changelog/126532.yaml --- docs/changelog/126532.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 docs/changelog/126532.yaml diff --git a/docs/changelog/126532.yaml b/docs/changelog/126532.yaml new file mode 100644 index 0000000000000..dff3094c31ad8 --- /dev/null +++ b/docs/changelog/126532.yaml @@ -0,0 +1,6 @@ +pr: 126532 +summary: TO_IP can handle leading zeros +area: ES|QL +type: bug +issues: + - 125460 From 9f7af8e5a87de7219f37630d9a0cfecb7abe998a Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Wed, 9 Apr 2025 10:02:24 -0400 Subject: [PATCH 3/7] Format --- .../org/elasticsearch/xpack/esql/action/EsqlCapabilities.java | 1 - 1 file changed, 1 deletion(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java index 521cf8ee12bb1..e6a9fac9dc84f 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java @@ -949,7 +949,6 @@ public enum Cap { */ FIX_REPLACE_MISSING_FIELD_WITH_NULL_DUPLICATE_NAME_ID_IN_LAYOUT, - /** * When creating constant null blocks in {@link org.elasticsearch.compute.lucene.ValuesSourceReaderOperator}, we also handed off * the ownership of that block - but didn't account for the fact that the caller might close it, leading to double releases From 66536ef189e079bb15c2e3c907e68ab93654e208 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Thu, 10 Apr 2025 10:16:35 -0400 Subject: [PATCH 4/7] Test from field --- .../xpack/esql/CsvTestsDataLoader.java | 4 +++- .../src/main/resources/data/logs.csv | 5 +++++ .../src/main/resources/ip.csv-spec | 19 +++++++++++++++++++ .../src/main/resources/mapping-logs.json | 13 +++++++++++++ 4 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 x-pack/plugin/esql/qa/testFixtures/src/main/resources/data/logs.csv create mode 100644 x-pack/plugin/esql/qa/testFixtures/src/main/resources/mapping-logs.json diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/CsvTestsDataLoader.java b/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/CsvTestsDataLoader.java index 9575f8ee176ea..49dd99346b932 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/CsvTestsDataLoader.java +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/CsvTestsDataLoader.java @@ -133,6 +133,7 @@ public class CsvTestsDataLoader { private static final TestDataset ADDRESSES = new TestDataset("addresses"); private static final TestDataset BOOKS = new TestDataset("books").withSetting("books-settings.json"); private static final TestDataset SEMANTIC_TEXT = new TestDataset("semantic_text").withInferenceEndpoint(true); + private static final TestDataset LOGS = new TestDataset("logs"); public static final Map CSV_DATASET_MAP = Map.ofEntries( Map.entry(EMPLOYEES.indexName, EMPLOYEES), @@ -182,7 +183,8 @@ public class CsvTestsDataLoader { Map.entry(DISTANCES.indexName, DISTANCES), Map.entry(ADDRESSES.indexName, ADDRESSES), Map.entry(BOOKS.indexName, BOOKS), - Map.entry(SEMANTIC_TEXT.indexName, SEMANTIC_TEXT) + Map.entry(SEMANTIC_TEXT.indexName, SEMANTIC_TEXT), + Map.entry(LOGS.indexName, LOGS) ); private static final EnrichConfig LANGUAGES_ENRICH = new EnrichConfig("languages_policy", "enrich-policy-languages.json"); diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/data/logs.csv b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/data/logs.csv new file mode 100644 index 0000000000000..2fd232a3ed33d --- /dev/null +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/data/logs.csv @@ -0,0 +1,5 @@ +@timestamp:date ,system:keyword,message:keyword +2023-10-23T13:55:01.543Z, ping,Pinging 192.168.86.046 +2023-10-23T13:55:01.544Z, cron,Running cats +2023-10-23T13:55:01.545Z, java,Doing java stuff for 192.168.86.038 +2023-10-23T13:55:01.546Z, java,More java stuff 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 e3268bbf81b8e..8daca32cbb682 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 @@ -297,6 +297,25 @@ s:keyword | ip:ip // end::to_ip_leading_zeros_octal-result[] ; +convertFromStringFancy +FROM logs +| KEEP @timestamp, system, message +| EVAL client = CASE( + system == "ping", + TO_IP(REPLACE(message, "Pinging ", ""), {"leading_zeros": "octal"}), + system == "java" AND STARTS_WITH(message, "Doing java stuff for "), + TO_IP(REPLACE(message, "Doing java stuff for ", ""), {"leading_zeros": "decimal"})) +| SORT @timestamp +| LIMIT 4 +; + +@timestamp:date |system:keyword|message:keyword |client:ip +2023-10-23T13:55:01.543Z| ping|Pinging 192.168.86.046 |192.168.86.38 +2023-10-23T13:55:01.544Z| cron|Running cats |null +2023-10-23T13:55:01.545Z| java|Doing java stuff for 192.168.86.038|192.168.86.38 +2023-10-23T13:55:01.546Z| java|More java stuff |null +; + toIpInAgg ROW s = "1.1.1.1" | STATS COUNT(*) BY ip = TO_IP(s) ; diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/mapping-logs.json b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/mapping-logs.json new file mode 100644 index 0000000000000..8b1ddb0e299db --- /dev/null +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/mapping-logs.json @@ -0,0 +1,13 @@ +{ + "properties" : { + "@timestamp" : { + "type" : "date" + }, + "system" : { + "type" : "keyword" + }, + "message" : { + "type" : "keyword" + } + } +} From 67882f16c9870a4c7ae8efdc977ec9cccf41cb75 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Thu, 10 Apr 2025 10:22:04 -0400 Subject: [PATCH 5/7] Share better --- .../xpack/esql/analysis/Analyzer.java | 18 ++++++++---------- .../scalar/convert/ConvertFunction.java | 3 --- .../SubstituteSurrogateExpressions.java | 8 ++++++++ 3 files changed, 16 insertions(+), 13 deletions(-) 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 6cc746d10178a..0a16d11edbec2 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 @@ -72,6 +72,7 @@ import org.elasticsearch.xpack.esql.index.EsIndex; import org.elasticsearch.xpack.esql.index.IndexResolution; import org.elasticsearch.xpack.esql.inference.ResolvedInference; +import org.elasticsearch.xpack.esql.optimizer.rules.logical.SubstituteSurrogateExpressions; import org.elasticsearch.xpack.esql.parser.ParsingException; import org.elasticsearch.xpack.esql.plan.IndexPattern; import org.elasticsearch.xpack.esql.plan.logical.Aggregate; @@ -1617,6 +1618,7 @@ private LogicalPlan doRule(LogicalPlan plan) { } private Expression resolveConvertFunction(ConvertFunction convert, List unionFieldAttributes) { + Expression convertExpression = (Expression) convert; if (convert.field() instanceof FieldAttribute fa && fa.field() instanceof InvalidMappedField imf) { HashMap typeResolutions = new HashMap<>(); Set supportedTypes = convert.supportedTypes(); @@ -1643,9 +1645,11 @@ private Expression resolveConvertFunction(ConvertFunction convert, List supportedTypes(); - - Expression replaceChildren(List newChildren); } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/SubstituteSurrogateExpressions.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/SubstituteSurrogateExpressions.java index bccf119b423b3..38b68bfb1e7ef 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/SubstituteSurrogateExpressions.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/SubstituteSurrogateExpressions.java @@ -9,6 +9,7 @@ import org.elasticsearch.xpack.esql.core.expression.Expression; import org.elasticsearch.xpack.esql.expression.SurrogateExpression; +import org.elasticsearch.xpack.esql.expression.function.scalar.math.Exp; import org.elasticsearch.xpack.esql.optimizer.LogicalOptimizerContext; /** @@ -22,6 +23,13 @@ public SubstituteSurrogateExpressions() { @Override protected Expression rule(Expression e, LogicalOptimizerContext ctx) { + return rule(e); + } + + /** + * Perform the actual substitution. + */ + public static Expression rule(Expression e) { if (e instanceof SurrogateExpression s) { Expression surrogate = s.surrogate(); if (surrogate != null) { From 70b1df6e41af94d691019776dc82be767bfbc9c2 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Thu, 10 Apr 2025 10:58:04 -0400 Subject: [PATCH 6/7] Share --- .../java/org/elasticsearch/xpack/esql/analysis/Analyzer.java | 1 - .../optimizer/rules/logical/SubstituteSurrogateExpressions.java | 1 - 2 files changed, 2 deletions(-) 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 0a16d11edbec2..cde1a7ed3c616 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 @@ -47,7 +47,6 @@ import org.elasticsearch.xpack.esql.core.util.Holder; import org.elasticsearch.xpack.esql.core.util.StringUtils; import org.elasticsearch.xpack.esql.expression.NamedExpressions; -import org.elasticsearch.xpack.esql.expression.SurrogateExpression; import org.elasticsearch.xpack.esql.expression.UnresolvedNamePattern; import org.elasticsearch.xpack.esql.expression.function.EsqlFunctionRegistry; import org.elasticsearch.xpack.esql.expression.function.FunctionDefinition; diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/SubstituteSurrogateExpressions.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/SubstituteSurrogateExpressions.java index 38b68bfb1e7ef..307634f4e8983 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/SubstituteSurrogateExpressions.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/SubstituteSurrogateExpressions.java @@ -9,7 +9,6 @@ import org.elasticsearch.xpack.esql.core.expression.Expression; import org.elasticsearch.xpack.esql.expression.SurrogateExpression; -import org.elasticsearch.xpack.esql.expression.function.scalar.math.Exp; import org.elasticsearch.xpack.esql.optimizer.LogicalOptimizerContext; /** From 103a2ddf8cb10d3a328329c14f05133ee54623a2 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Fri, 11 Apr 2025 12:33:48 -0400 Subject: [PATCH 7/7] My fault --- .../plugin/esql/qa/testFixtures/src/main/resources/ip.csv-spec | 1 + 1 file changed, 1 insertion(+) 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 8daca32cbb682..50c46beeff327 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 @@ -298,6 +298,7 @@ s:keyword | ip:ip ; convertFromStringFancy +required_capability: to_ip_leading_zeros FROM logs | KEEP @timestamp, system, message | EVAL client = CASE(