handle error cases

idegtiarenko · Dec 6, 2024 · 923f453 · 923f453
1 parent 62f97c3
commit 923f453
Show file tree

Hide file tree

Showing 6 changed files with 40 additions and 6 deletions.
diff --git a/docs/reference/esql/functions/description/hash.asciidoc b/docs/reference/esql/functions/description/hash.asciidoc
diff --git a/docs/reference/esql/functions/kibana/definition/hash.json b/docs/reference/esql/functions/kibana/definition/hash.json
diff --git a/docs/reference/esql/functions/kibana/docs/hash.md b/docs/reference/esql/functions/kibana/docs/hash.md
diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/hash.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/hash.csv-spec
@@ -4,6 +4,7 @@ required_capability: hash_function
 FROM languages 
 | EVAL md5 = hash("md5", language_name), sha256 = hash("sha256", language_name) 
 | KEEP language_name, md5, sha256;
+ignoreOrder:true
 
 language_name:keyword | md5:keyword                      | sha256:keyword
 English               | 78463a384a5aa4fad5fa73e2f506ecfc | ba118bf7fc9c1aedc1edb28a0aa86e0b43b681f222af6616e13c43be87815b06

diff --git a/...esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/hash/Hash.java b/...esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/hash/Hash.java
@@ -15,6 +15,7 @@
 import org.elasticsearch.compute.ann.Fixed;
 import org.elasticsearch.compute.operator.BreakingBytesRefBuilder;
 import org.elasticsearch.compute.operator.EvalOperator;
+import org.elasticsearch.xpack.esql.core.InvalidArgumentException;
 import org.elasticsearch.xpack.esql.core.expression.Expression;
 import org.elasticsearch.xpack.esql.core.tree.NodeInfo;
 import org.elasticsearch.xpack.esql.core.tree.Source;
@@ -41,7 +42,7 @@ public class Hash extends EsqlScalarFunction {
     private final Expression alg;
     private final Expression input;
 
-    @FunctionInfo(returnType = "keyword", description = "Computes the hash of the input using the supplied algorithm.")
+    @FunctionInfo(returnType = "keyword", description = "Computes the hash of the input using java.security.MessageDigest.")
     public Hash(
         Source source,
         @Param(name = "alg", type = { "keyword", "text" }, description = "Hash algorithm to use.") Expression alg,
@@ -119,7 +120,7 @@ private static BytesRef hash(BreakingBytesRefBuilder scratch, MessageDigest alg,
 
     @Override
     public EvalOperator.ExpressionEvaluator.Factory toEvaluator(ToEvaluator toEvaluator) {
-        if (alg.foldable() && alg.dataType() == DataType.KEYWORD) {
+        if (alg.foldable()) {
             try {
                 var md = MessageDigest.getInstance(((BytesRef) alg.fold()).utf8ToString());
                 return new HashConstantEvaluator.Factory(
@@ -129,7 +130,7 @@ public EvalOperator.ExpressionEvaluator.Factory toEvaluator(ToEvaluator toEvalua
                     toEvaluator.apply(input)
                 );
             } catch (NoSuchAlgorithmException e) {
-                throw new IllegalArgumentException(e);
+                throw new InvalidArgumentException(e, "invalid alg for [{}]: {}", sourceText(), e.getMessage());
             }
         } else {
             return new HashEvaluator.Factory(

diff --git a/...src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/hash/HashTests.java b/...src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/hash/HashTests.java
@@ -11,7 +11,10 @@
 import com.carrotsearch.randomizedtesting.annotations.ParametersFactory;
 
 import org.apache.lucene.util.BytesRef;
+import org.elasticsearch.compute.operator.DriverContext;
+import org.elasticsearch.xpack.esql.core.InvalidArgumentException;
 import org.elasticsearch.xpack.esql.core.expression.Expression;
+import org.elasticsearch.xpack.esql.core.expression.Literal;
 import org.elasticsearch.xpack.esql.core.tree.Source;
 import org.elasticsearch.xpack.esql.core.type.DataType;
 import org.elasticsearch.xpack.esql.expression.function.AbstractScalarFunctionTestCase;
@@ -26,6 +29,9 @@
 import java.util.function.Supplier;
 
 import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.nullValue;
+import static org.hamcrest.Matchers.startsWith;
 
 public class HashTests extends AbstractScalarFunctionTestCase {
 
@@ -39,6 +45,20 @@ public static Iterable<Object[]> parameters() {
         for (String alg : List.of("MD5", "SHA", "SHA-224", "SHA-256", "SHA-384", "SHA-512")) {
             cases.addAll(createTestCases(alg));
         }
+        cases.add(new TestCaseSupplier("Invalid alg", List.of(DataType.KEYWORD, DataType.KEYWORD), () -> {
+            var input = randomAlphaOfLength(10);
+            return new TestCaseSupplier.TestCase(
+                List.of(
+                    new TestCaseSupplier.TypedData(new BytesRef("invalid"), DataType.KEYWORD, "alg"),
+                    new TestCaseSupplier.TypedData(new BytesRef(input), DataType.KEYWORD, "input")
+                ),
+                "HashEvaluator[alg=Attribute[channel=0], input=Attribute[channel=1]]",
+                DataType.KEYWORD,
+                is(nullValue())
+            ).withWarning("Line -1:-1: evaluation of [] failed, treating result as null. Only first 20 failures recorded.")
+                .withWarning("Line -1:-1: java.security.NoSuchAlgorithmException: invalid MessageDigest not available")
+                .withFoldingException(InvalidArgumentException.class, "invalid alg for []: invalid MessageDigest not available");
+        }));
         return parameterSuppliersFromTypedDataWithDefaultChecks(true, cases, (v, p) -> "string");
     }
 
@@ -78,4 +98,16 @@ private static String hash(String alg, String input) {
     protected Expression build(Source source, List<Expression> args) {
         return new Hash(source, args.get(0), args.get(1));
     }
+
+    public void testInvalidAlgLiteral() {
+        Source source = new Source(0, 0, "hast(\"invalid\", input)");
+        DriverContext driverContext = driverContext();
+        InvalidArgumentException e = expectThrows(
+            InvalidArgumentException.class,
+            () -> evaluator(
+                new Hash(source, new Literal(source, new BytesRef("invalid"), DataType.KEYWORD), field("str", DataType.KEYWORD))
+            ).get(driverContext)
+        );
+        assertThat(e.getMessage(), startsWith("invalid alg for [hast(\"invalid\", input)]: invalid MessageDigest not available"));
+    }
 }