Browse Source

ESQL: Account for an exception being thrown when building a BytesRefArrayBlock (#99726)

Account for an exception being thrown when building a BytesRefArrayBlock
by registering an empty BytesRef in the BytesRefArray, rather than doing nothing
Andrei Stefan 2 years ago
parent
commit
96679d28ac
20 changed files with 183 additions and 10 deletions
  1. 6 0
      docs/changelog/99726.yaml
  2. 4 0
      x-pack/plugin/esql/compute/gen/src/main/java/org/elasticsearch/compute/gen/ConvertEvaluatorImplementer.java
  3. 2 0
      x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/BytesRefBlock.java
  4. 1 3
      x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/BytesRefBlockBuilder.java
  5. 4 0
      x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/X-Block.java.st
  6. 1 3
      x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/X-BlockBuilder.java.st
  7. 55 0
      x-pack/plugin/esql/qa/server/single-node/src/yamlRestTest/resources/rest-api-spec/test/100_bug_fix.yml
  8. 1 0
      x-pack/plugin/esql/src/main/java/generated/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIPFromStringEvaluator.java
  9. 1 0
      x-pack/plugin/esql/src/main/java/generated/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToStringFromBooleanEvaluator.java
  10. 1 0
      x-pack/plugin/esql/src/main/java/generated/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToStringFromDatetimeEvaluator.java
  11. 1 0
      x-pack/plugin/esql/src/main/java/generated/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToStringFromDoubleEvaluator.java
  12. 1 0
      x-pack/plugin/esql/src/main/java/generated/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToStringFromIPEvaluator.java
  13. 1 0
      x-pack/plugin/esql/src/main/java/generated/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToStringFromIntEvaluator.java
  14. 1 0
      x-pack/plugin/esql/src/main/java/generated/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToStringFromLongEvaluator.java
  15. 1 0
      x-pack/plugin/esql/src/main/java/generated/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToStringFromUnsignedLongEvaluator.java
  16. 1 0
      x-pack/plugin/esql/src/main/java/generated/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToStringFromVersionEvaluator.java
  17. 1 0
      x-pack/plugin/esql/src/main/java/generated/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToVersionFromStringEvaluator.java
  18. 4 3
      x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/AbstractFunctionTestCase.java
  19. 1 1
      x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/TestCaseSupplier.java
  20. 95 0
      x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIPTests.java

+ 6 - 0
docs/changelog/99726.yaml

@@ -0,0 +1,6 @@
+pr: 99726
+summary: "ESQL: Account for an exception being thrown when building a `BytesRefArrayBlock`"
+area: ES|QL
+type: bug
+issues:
+ - 99472

+ 4 - 0
x-pack/plugin/esql/compute/gen/src/main/java/org/elasticsearch/compute/gen/ConvertEvaluatorImplementer.java

@@ -27,6 +27,7 @@ import static org.elasticsearch.compute.gen.Types.BIG_ARRAYS;
 import static org.elasticsearch.compute.gen.Types.BLOCK;
 import static org.elasticsearch.compute.gen.Types.BYTES_REF;
 import static org.elasticsearch.compute.gen.Types.BYTES_REF_ARRAY;
+import static org.elasticsearch.compute.gen.Types.BYTES_REF_BLOCK;
 import static org.elasticsearch.compute.gen.Types.EXPRESSION_EVALUATOR;
 import static org.elasticsearch.compute.gen.Types.SOURCE;
 import static org.elasticsearch.compute.gen.Types.VECTOR;
@@ -166,6 +167,9 @@ public class ConvertEvaluatorImplementer {
                 }
                 builder.endControlFlow();
                 builder.addStatement("nullsMask.set(p)");
+                if (resultType.equals(BYTES_REF)) {
+                    builder.addStatement("values.append($T.NULL_VALUE)", BYTES_REF_BLOCK);
+                }
             }
             builder.endControlFlow();
         }

+ 2 - 0
x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/BytesRefBlock.java

@@ -20,6 +20,8 @@ import java.io.IOException;
  */
 public sealed interface BytesRefBlock extends Block permits FilterBytesRefBlock, BytesRefArrayBlock, BytesRefVectorBlock {
 
+    BytesRef NULL_VALUE = new BytesRef();
+
     /**
      * Retrieves the BytesRef value stored at the given value index.
      *

+ 1 - 3
x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/BytesRefBlockBuilder.java

@@ -17,8 +17,6 @@ import org.elasticsearch.common.util.BytesRefArray;
  */
 final class BytesRefBlockBuilder extends AbstractBlockBuilder implements BytesRefBlock.Builder {
 
-    private static final BytesRef NULL_VALUE = new BytesRef();
-
     private BytesRefArray values;
 
     BytesRefBlockBuilder(int estimatedSize) {
@@ -69,7 +67,7 @@ final class BytesRefBlockBuilder extends AbstractBlockBuilder implements BytesRe
 
     @Override
     protected void writeNullValue() {
-        values.append(NULL_VALUE);
+        values.append(BytesRefBlock.NULL_VALUE);
     }
 
     /**

+ 4 - 0
x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/X-Block.java.st

@@ -23,6 +23,10 @@ import java.io.IOException;
  */
 public sealed interface $Type$Block extends Block permits Filter$Type$Block, $Type$ArrayBlock, $Type$VectorBlock {
 
+$if(BytesRef)$
+    BytesRef NULL_VALUE = new BytesRef();
+
+$endif$
     /**
      * Retrieves the $type$ value stored at the given value index.
      *

+ 1 - 3
x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/X-BlockBuilder.java.st

@@ -23,8 +23,6 @@ $endif$
 final class $Type$BlockBuilder extends AbstractBlockBuilder implements $Type$Block.Builder {
 
 $if(BytesRef)$
-    private static final BytesRef NULL_VALUE = new BytesRef();
-
     private BytesRefArray values;
 
     BytesRefBlockBuilder(int estimatedSize) {
@@ -96,7 +94,7 @@ $endif$
 $if(BytesRef)$
     @Override
     protected void writeNullValue() {
-        values.append(NULL_VALUE);
+        values.append(BytesRefBlock.NULL_VALUE);
     }
 $endif$
 

+ 55 - 0
x-pack/plugin/esql/qa/server/single-node/src/yamlRestTest/resources/rest-api-spec/test/100_bug_fix.yml

@@ -0,0 +1,55 @@
+---
+setup:
+  - do:
+      bulk:
+        index: test
+        refresh: true
+        body:
+          - { "index": { } }
+          - { "emp_no": 10, "ip1": "127.0", "ip2": "0.1" }
+          - { "index": { } }
+          - { "emp_no": 20 }
+
+---
+"Bug fix https://github.com/elastic/elasticsearch/issues/99472":
+  - skip:
+      features: warnings
+  - do:
+      warnings:
+        - "Line 1:37: evaluation of [to_ip(coalesce(ip1.keyword, \"255.255.255.255\"))] failed, treating result as null. Only first 20 failures recorded."
+        - "java.lang.IllegalArgumentException: '127.0' is not an IP string literal."
+      esql.query:
+        body:
+          query: 'FROM test | sort emp_no | eval ip = to_ip(coalesce(ip1.keyword, "255.255.255.255")) | keep emp_no, ip'
+
+  - match: { columns.0.name: "emp_no" }
+  - match: { columns.0.type: "long" }
+  - match: { columns.1.name: "ip" }
+  - match: { columns.1.type: "ip" }
+
+  - length: { values: 2 }
+  - match: { values.0: [ 10, null ] }
+  - match: { values.1: [ 20, "255.255.255.255"] }
+
+
+  - do:
+      warnings:
+        - "Line 1:98: evaluation of [to_ip(x2)] failed, treating result as null. Only first 20 failures recorded."
+        - "java.lang.IllegalArgumentException: '127.00.1' is not an IP string literal."
+      esql.query:
+        body:
+          query: 'FROM test | sort emp_no | eval x1 = concat(ip1, ip2), x2 = coalesce(x1, "255.255.255.255"), x3 = to_ip(x2) | keep emp_no, x*'
+
+  - match: { columns.0.name: "emp_no" }
+  - match: { columns.0.type: "long" }
+  - match: { columns.1.name: "x1" }
+  - match: { columns.1.type: "keyword" }
+  - match: { columns.2.name: "x2" }
+  - match: { columns.2.type: "keyword" }
+  - match: { columns.3.name: "x3" }
+  - match: { columns.3.type: "ip" }
+
+
+  - length: { values: 2 }
+  - match: { values.0: [ 10, "127.00.1", "127.00.1", null ] }
+  - match: { values.1: [ 20, null, "255.255.255.255", "255.255.255.255"] }

+ 1 - 0
x-pack/plugin/esql/src/main/java/generated/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIPFromStringEvaluator.java

@@ -58,6 +58,7 @@ public final class ToIPFromStringEvaluator extends AbstractConvertFunction.Abstr
           nullsMask = new BitSet(positionCount);
         }
         nullsMask.set(p);
+        values.append(BytesRefBlock.NULL_VALUE);
       }
     }
     return nullsMask == null

+ 1 - 0
x-pack/plugin/esql/src/main/java/generated/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToStringFromBooleanEvaluator.java

@@ -58,6 +58,7 @@ public final class ToStringFromBooleanEvaluator extends AbstractConvertFunction.
           nullsMask = new BitSet(positionCount);
         }
         nullsMask.set(p);
+        values.append(BytesRefBlock.NULL_VALUE);
       }
     }
     return nullsMask == null

+ 1 - 0
x-pack/plugin/esql/src/main/java/generated/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToStringFromDatetimeEvaluator.java

@@ -58,6 +58,7 @@ public final class ToStringFromDatetimeEvaluator extends AbstractConvertFunction
           nullsMask = new BitSet(positionCount);
         }
         nullsMask.set(p);
+        values.append(BytesRefBlock.NULL_VALUE);
       }
     }
     return nullsMask == null

+ 1 - 0
x-pack/plugin/esql/src/main/java/generated/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToStringFromDoubleEvaluator.java

@@ -58,6 +58,7 @@ public final class ToStringFromDoubleEvaluator extends AbstractConvertFunction.A
           nullsMask = new BitSet(positionCount);
         }
         nullsMask.set(p);
+        values.append(BytesRefBlock.NULL_VALUE);
       }
     }
     return nullsMask == null

+ 1 - 0
x-pack/plugin/esql/src/main/java/generated/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToStringFromIPEvaluator.java

@@ -58,6 +58,7 @@ public final class ToStringFromIPEvaluator extends AbstractConvertFunction.Abstr
           nullsMask = new BitSet(positionCount);
         }
         nullsMask.set(p);
+        values.append(BytesRefBlock.NULL_VALUE);
       }
     }
     return nullsMask == null

+ 1 - 0
x-pack/plugin/esql/src/main/java/generated/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToStringFromIntEvaluator.java

@@ -58,6 +58,7 @@ public final class ToStringFromIntEvaluator extends AbstractConvertFunction.Abst
           nullsMask = new BitSet(positionCount);
         }
         nullsMask.set(p);
+        values.append(BytesRefBlock.NULL_VALUE);
       }
     }
     return nullsMask == null

+ 1 - 0
x-pack/plugin/esql/src/main/java/generated/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToStringFromLongEvaluator.java

@@ -58,6 +58,7 @@ public final class ToStringFromLongEvaluator extends AbstractConvertFunction.Abs
           nullsMask = new BitSet(positionCount);
         }
         nullsMask.set(p);
+        values.append(BytesRefBlock.NULL_VALUE);
       }
     }
     return nullsMask == null

+ 1 - 0
x-pack/plugin/esql/src/main/java/generated/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToStringFromUnsignedLongEvaluator.java

@@ -58,6 +58,7 @@ public final class ToStringFromUnsignedLongEvaluator extends AbstractConvertFunc
           nullsMask = new BitSet(positionCount);
         }
         nullsMask.set(p);
+        values.append(BytesRefBlock.NULL_VALUE);
       }
     }
     return nullsMask == null

+ 1 - 0
x-pack/plugin/esql/src/main/java/generated/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToStringFromVersionEvaluator.java

@@ -58,6 +58,7 @@ public final class ToStringFromVersionEvaluator extends AbstractConvertFunction.
           nullsMask = new BitSet(positionCount);
         }
         nullsMask.set(p);
+        values.append(BytesRefBlock.NULL_VALUE);
       }
     }
     return nullsMask == null

+ 1 - 0
x-pack/plugin/esql/src/main/java/generated/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToVersionFromStringEvaluator.java

@@ -58,6 +58,7 @@ public final class ToVersionFromStringEvaluator extends AbstractConvertFunction.
           nullsMask = new BitSet(positionCount);
         }
         nullsMask.set(p);
+        values.append(BytesRefBlock.NULL_VALUE);
       }
     }
     return nullsMask == null

+ 4 - 3
x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/AbstractFunctionTestCase.java

@@ -327,7 +327,7 @@ public abstract class AbstractFunctionTestCase extends ESTestCase {
                         oc.evaluatorToString,
                         oc.expectedType,
                         nullValue(),
-                        oc.getExpectedWarnings(),
+                        null,
                         oc.getExpectedTypeError()
                     );
                 }));
@@ -352,7 +352,7 @@ public abstract class AbstractFunctionTestCase extends ESTestCase {
                                 "LiteralsEvaluator[block=null]",
                                 entirelyNullPreservesType == false && oc.getData().size() == 1 ? DataTypes.NULL : oc.expectedType,
                                 nullValue(),
-                                oc.getExpectedWarnings(),
+                                null,
                                 oc.getExpectedTypeError()
                             );
                         }));
@@ -475,7 +475,8 @@ public abstract class AbstractFunctionTestCase extends ESTestCase {
         Map.entry(Set.of(DataTypes.DOUBLE, DataTypes.NULL), "double"),
         Map.entry(Set.of(DataTypes.INTEGER, DataTypes.NULL), "integer"),
         Map.entry(Set.of(DataTypes.LONG, DataTypes.INTEGER, DataTypes.UNSIGNED_LONG, DataTypes.DOUBLE, DataTypes.NULL), "numeric"),
-        Map.entry(Set.of(DataTypes.KEYWORD, DataTypes.TEXT, DataTypes.VERSION, DataTypes.NULL), "keyword, text or version")
+        Map.entry(Set.of(DataTypes.KEYWORD, DataTypes.TEXT, DataTypes.VERSION, DataTypes.NULL), "keyword, text or version"),
+        Map.entry(Set.of(DataTypes.IP, DataTypes.KEYWORD, DataTypes.NULL), "ip or keyword")
     );
 
     private static String expectedType(Set<DataType> validTypes) {

+ 1 - 1
x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/TestCaseSupplier.java

@@ -633,7 +633,7 @@ public record TestCaseSupplier(String name, List<DataType> types, Supplier<TestC
         );
     }
 
-    private static List<TypedDataSupplier> stringCases(DataType type) {
+    public static List<TypedDataSupplier> stringCases(DataType type) {
         List<TypedDataSupplier> result = new ArrayList<>();
         result.add(new TypedDataSupplier("<empty " + type + ">", () -> new BytesRef(""), type));
         result.add(

+ 95 - 0
x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToIPTests.java

@@ -0,0 +1,95 @@
+/*
+ * 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.expression.function.AbstractFunctionTestCase;
+import org.elasticsearch.xpack.esql.expression.function.TestCaseSupplier;
+import org.elasticsearch.xpack.ql.expression.Expression;
+import org.elasticsearch.xpack.ql.tree.Source;
+import org.elasticsearch.xpack.ql.type.DataType;
+import org.elasticsearch.xpack.ql.type.DataTypes;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.function.Supplier;
+
+import static org.elasticsearch.xpack.esql.expression.function.TestCaseSupplier.stringCases;
+import static org.elasticsearch.xpack.ql.util.StringUtils.parseIP;
+import static org.hamcrest.Matchers.equalTo;
+
+public class ToIPTests extends AbstractFunctionTestCase {
+    public ToIPTests(@Name("TestCase") Supplier<TestCaseSupplier.TestCase> testCaseSupplier) {
+        this.testCase = testCaseSupplier.get();
+    }
+
+    @ParametersFactory
+    public static Iterable<Object[]> parameters() {
+        String read = "Attribute[channel=0]";
+        String stringEvaluator = "ToIPFromStringEvaluator[field=" + read + "]";
+        List<TestCaseSupplier> suppliers = new ArrayList<>();
+
+        // convert from IP to IP
+        TestCaseSupplier.forUnaryIp(suppliers, read, DataTypes.IP, v -> v, List.of());
+
+        // convert any kind of string to IP, with warnings.
+        for (TestCaseSupplier.TypedDataSupplier supplier : stringCases(DataTypes.KEYWORD)) {
+            suppliers.add(new TestCaseSupplier(supplier.name(), List.of(supplier.type()), () -> {
+                BytesRef value = (BytesRef) supplier.supplier().get();
+                TestCaseSupplier.TypedData typed = new TestCaseSupplier.TypedData(value, supplier.type(), "value");
+                TestCaseSupplier.TestCase testCase = new TestCaseSupplier.TestCase(
+                    List.of(typed),
+                    stringEvaluator,
+                    DataTypes.IP,
+                    equalTo(null)
+                ).withWarning("Line -1:-1: evaluation of [] failed, treating result as null. Only first 20 failures recorded.")
+                    .withWarning("java.lang.IllegalArgumentException: '" + value.utf8ToString() + "' is not an IP string literal.");
+                return testCase;
+            }));
+        }
+
+        // convert valid IPs shaped as strings
+        DataType inputType = DataTypes.KEYWORD;
+        for (TestCaseSupplier.TypedDataSupplier ipGen : validIPsAsStrings()) {
+            suppliers.add(new TestCaseSupplier(ipGen.name(), List.of(inputType), () -> {
+                BytesRef ip = (BytesRef) ipGen.supplier().get();
+                TestCaseSupplier.TypedData typed = new TestCaseSupplier.TypedData(ip, inputType, "value");
+                return new TestCaseSupplier.TestCase(List.of(typed), stringEvaluator, DataTypes.IP, equalTo(parseIP(ip.utf8ToString())));
+            }));
+        }
+
+        // add null as parameter
+        return parameterSuppliersFromTypedData(errorsForCasesWithoutExamples(anyNullIsNull(true, suppliers)));
+    }
+
+    @Override
+    protected Expression build(Source source, List<Expression> args) {
+        return new ToIP(source, args.get(0));
+    }
+
+    private static List<TestCaseSupplier.TypedDataSupplier> validIPsAsStrings() {
+        return List.of(
+            new TestCaseSupplier.TypedDataSupplier("<127.0.0.1 ip>", () -> new BytesRef("127.0.0.1"), DataTypes.KEYWORD),
+            new TestCaseSupplier.TypedDataSupplier(
+                "<ipv4>",
+                () -> new BytesRef(NetworkAddress.format(ESTestCase.randomIp(true))),
+                DataTypes.KEYWORD
+            ),
+            new TestCaseSupplier.TypedDataSupplier(
+                "<ipv6>",
+                () -> new BytesRef(NetworkAddress.format(ESTestCase.randomIp(false))),
+                DataTypes.KEYWORD
+            )
+        );
+    }
+}