[ESQL] Test that we don't return NaNs or Infinites (#99489)

Relates to #98698 Add asserts that we aren't returning NaN or Infinite values from scalar functions. I also fixed the functions that already had test cases returning NaN/Infinite values. Note that this doesn't fix all our scalar functions, as some may just not have test cases that hit these outputs regularly, which is why this PR doesn't close the above issue.
elastic · Sep 12, 2023 · 7f92a3d · 7f92a3d
1 parent daddca7
commit 7f92a3d
Show file tree

Hide file tree

Showing 21 changed files with 387 additions and 100 deletions.
diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/floats.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/floats.csv-spec
@@ -269,13 +269,15 @@ a:double | acos:double
 // end::acos-result[]
 ;
 
-acosNan
+acosNull
 ROW a=12.0
 | EVAL acos=ACOS(a)
 ;
+warning:Line 2:13: evaluation of [ACOS(a)] failed, treating result as null. Only first 20 failures recorded.
+warning:java.lang.ArithmeticException: Acos input out of range
 
 a:double | acos:double
-      12 | NaN
+      12 | null
 ;
 
 sin
@@ -317,13 +319,15 @@ a:double | asin:double
 // end::asin-result[]
 ;
 
-asinNan
+asinNull
 ROW a=12.0
 | EVAL asin=ASIN(a)
 ;
+warning:Line 2:13: evaluation of [ASIN(a)] failed, treating result as null. Only first 20 failures recorded.
+warning:java.lang.ArithmeticException: Asin input out of range
 
 a:double | asin:double
-      12 | NaN
+      12 | null
 ;
 
 tan

diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/math.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/math.csv-spec
@@ -1019,10 +1019,12 @@ l:ul                  | s:double
 ;
 
 sqrtOfNegative
-row d = -1.0 | eval s = is_nan(sqrt(d));
+row d = -1.0 | eval s = sqrt(d);
+warning:Line 1:25: evaluation of [sqrt(d)] failed, treating result as null. Only first 20 failures recorded.
+warning:java.lang.ArithmeticException: Square root of negative
 
-d:double | s:boolean
--1.0     | true
+d:double | s:double
+-1.0     | null
 ;
 
 sqrtOfNan

diff --git a/...generated/org/elasticsearch/xpack/esql/expression/function/scalar/math/AcosEvaluator.java b/...generated/org/elasticsearch/xpack/esql/expression/function/scalar/math/AcosEvaluator.java
@@ -4,22 +4,28 @@
 // 2.0.
 package org.elasticsearch.xpack.esql.expression.function.scalar.math;
 
+import java.lang.ArithmeticException;
 import java.lang.Override;
 import java.lang.String;
 import org.elasticsearch.compute.data.Block;
 import org.elasticsearch.compute.data.DoubleBlock;
 import org.elasticsearch.compute.data.DoubleVector;
 import org.elasticsearch.compute.data.Page;
 import org.elasticsearch.compute.operator.EvalOperator;
+import org.elasticsearch.xpack.esql.expression.function.Warnings;
+import org.elasticsearch.xpack.ql.tree.Source;
 
 /**
  * {@link EvalOperator.ExpressionEvaluator} implementation for {@link Acos}.
  * This class is generated. Do not edit it.
  */
 public final class AcosEvaluator implements EvalOperator.ExpressionEvaluator {
+  private final Warnings warnings;
+
   private final EvalOperator.ExpressionEvaluator val;
 
-  public AcosEvaluator(EvalOperator.ExpressionEvaluator val) {
+  public AcosEvaluator(Source source, EvalOperator.ExpressionEvaluator val) {
+    this.warnings = new Warnings(source);
     this.val = val;
   }
 
@@ -34,7 +40,7 @@ public Block eval(Page page) {
     if (valVector == null) {
       return eval(page.getPositionCount(), valBlock);
     }
-    return eval(page.getPositionCount(), valVector).asBlock();
+    return eval(page.getPositionCount(), valVector);
   }
 
   public DoubleBlock eval(int positionCount, DoubleBlock valBlock) {
@@ -44,15 +50,25 @@ public DoubleBlock eval(int positionCount, DoubleBlock valBlock) {
         result.appendNull();
         continue position;
       }
-      result.appendDouble(Acos.process(valBlock.getDouble(valBlock.getFirstValueIndex(p))));
+      try {
+        result.appendDouble(Acos.process(valBlock.getDouble(valBlock.getFirstValueIndex(p))));
+      } catch (ArithmeticException e) {
+        warnings.registerException(e);
+        result.appendNull();
+      }
     }
     return result.build();
   }
 
-  public DoubleVector eval(int positionCount, DoubleVector valVector) {
-    DoubleVector.Builder result = DoubleVector.newVectorBuilder(positionCount);
+  public DoubleBlock eval(int positionCount, DoubleVector valVector) {
+    DoubleBlock.Builder result = DoubleBlock.newBlockBuilder(positionCount);
     position: for (int p = 0; p < positionCount; p++) {
-      result.appendDouble(Acos.process(valVector.getDouble(p)));
+      try {
+        result.appendDouble(Acos.process(valVector.getDouble(p)));
+      } catch (ArithmeticException e) {
+        warnings.registerException(e);
+        result.appendNull();
+      }
     }
     return result.build();
   }

diff --git a/...generated/org/elasticsearch/xpack/esql/expression/function/scalar/math/AsinEvaluator.java b/...generated/org/elasticsearch/xpack/esql/expression/function/scalar/math/AsinEvaluator.java
@@ -4,22 +4,28 @@
 // 2.0.
 package org.elasticsearch.xpack.esql.expression.function.scalar.math;
 
+import java.lang.ArithmeticException;
 import java.lang.Override;
 import java.lang.String;
 import org.elasticsearch.compute.data.Block;
 import org.elasticsearch.compute.data.DoubleBlock;
 import org.elasticsearch.compute.data.DoubleVector;
 import org.elasticsearch.compute.data.Page;
 import org.elasticsearch.compute.operator.EvalOperator;
+import org.elasticsearch.xpack.esql.expression.function.Warnings;
+import org.elasticsearch.xpack.ql.tree.Source;
 
 /**
  * {@link EvalOperator.ExpressionEvaluator} implementation for {@link Asin}.
  * This class is generated. Do not edit it.
  */
 public final class AsinEvaluator implements EvalOperator.ExpressionEvaluator {
+  private final Warnings warnings;
+
   private final EvalOperator.ExpressionEvaluator val;
 
-  public AsinEvaluator(EvalOperator.ExpressionEvaluator val) {
+  public AsinEvaluator(Source source, EvalOperator.ExpressionEvaluator val) {
+    this.warnings = new Warnings(source);
     this.val = val;
   }
 
@@ -34,7 +40,7 @@ public Block eval(Page page) {
     if (valVector == null) {
       return eval(page.getPositionCount(), valBlock);
     }
-    return eval(page.getPositionCount(), valVector).asBlock();
+    return eval(page.getPositionCount(), valVector);
   }
 
   public DoubleBlock eval(int positionCount, DoubleBlock valBlock) {
@@ -44,15 +50,25 @@ public DoubleBlock eval(int positionCount, DoubleBlock valBlock) {
         result.appendNull();
         continue position;
       }
-      result.appendDouble(Asin.process(valBlock.getDouble(valBlock.getFirstValueIndex(p))));
+      try {
+        result.appendDouble(Asin.process(valBlock.getDouble(valBlock.getFirstValueIndex(p))));
+      } catch (ArithmeticException e) {
+        warnings.registerException(e);
+        result.appendNull();
+      }
     }
     return result.build();
   }
 
-  public DoubleVector eval(int positionCount, DoubleVector valVector) {
-    DoubleVector.Builder result = DoubleVector.newVectorBuilder(positionCount);
+  public DoubleBlock eval(int positionCount, DoubleVector valVector) {
+    DoubleBlock.Builder result = DoubleBlock.newBlockBuilder(positionCount);
     position: for (int p = 0; p < positionCount; p++) {
-      result.appendDouble(Asin.process(valVector.getDouble(p)));
+      try {
+        result.appendDouble(Asin.process(valVector.getDouble(p)));
+      } catch (ArithmeticException e) {
+        warnings.registerException(e);
+        result.appendNull();
+      }
     }
     return result.build();
   }

diff --git a/...generated/org/elasticsearch/xpack/esql/expression/function/scalar/math/CoshEvaluator.java b/...generated/org/elasticsearch/xpack/esql/expression/function/scalar/math/CoshEvaluator.java
@@ -4,22 +4,28 @@
 // 2.0.
 package org.elasticsearch.xpack.esql.expression.function.scalar.math;
 
+import java.lang.ArithmeticException;
 import java.lang.Override;
 import java.lang.String;
 import org.elasticsearch.compute.data.Block;
 import org.elasticsearch.compute.data.DoubleBlock;
 import org.elasticsearch.compute.data.DoubleVector;
 import org.elasticsearch.compute.data.Page;
 import org.elasticsearch.compute.operator.EvalOperator;
+import org.elasticsearch.xpack.esql.expression.function.Warnings;
+import org.elasticsearch.xpack.ql.tree.Source;
 
 /**
  * {@link EvalOperator.ExpressionEvaluator} implementation for {@link Cosh}.
  * This class is generated. Do not edit it.
  */
 public final class CoshEvaluator implements EvalOperator.ExpressionEvaluator {
+  private final Warnings warnings;
+
   private final EvalOperator.ExpressionEvaluator val;
 
-  public CoshEvaluator(EvalOperator.ExpressionEvaluator val) {
+  public CoshEvaluator(Source source, EvalOperator.ExpressionEvaluator val) {
+    this.warnings = new Warnings(source);
     this.val = val;
   }
 
@@ -34,7 +40,7 @@ public Block eval(Page page) {
     if (valVector == null) {
       return eval(page.getPositionCount(), valBlock);
     }
-    return eval(page.getPositionCount(), valVector).asBlock();
+    return eval(page.getPositionCount(), valVector);
   }
 
   public DoubleBlock eval(int positionCount, DoubleBlock valBlock) {
@@ -44,15 +50,25 @@ public DoubleBlock eval(int positionCount, DoubleBlock valBlock) {
         result.appendNull();
         continue position;
       }
-      result.appendDouble(Cosh.process(valBlock.getDouble(valBlock.getFirstValueIndex(p))));
+      try {
+        result.appendDouble(Cosh.process(valBlock.getDouble(valBlock.getFirstValueIndex(p))));
+      } catch (ArithmeticException e) {
+        warnings.registerException(e);
+        result.appendNull();
+      }
     }
     return result.build();
   }
 
-  public DoubleVector eval(int positionCount, DoubleVector valVector) {
-    DoubleVector.Builder result = DoubleVector.newVectorBuilder(positionCount);
+  public DoubleBlock eval(int positionCount, DoubleVector valVector) {
+    DoubleBlock.Builder result = DoubleBlock.newBlockBuilder(positionCount);
     position: for (int p = 0; p < positionCount; p++) {
-      result.appendDouble(Cosh.process(valVector.getDouble(p)));
+      try {
+        result.appendDouble(Cosh.process(valVector.getDouble(p)));
+      } catch (ArithmeticException e) {
+        warnings.registerException(e);
+        result.appendNull();
+      }
     }
     return result.build();
   }

diff --git a/...generated/org/elasticsearch/xpack/esql/expression/function/scalar/math/SinhEvaluator.java b/...generated/org/elasticsearch/xpack/esql/expression/function/scalar/math/SinhEvaluator.java
@@ -4,22 +4,28 @@
 // 2.0.
 package org.elasticsearch.xpack.esql.expression.function.scalar.math;
 
+import java.lang.ArithmeticException;
 import java.lang.Override;
 import java.lang.String;
 import org.elasticsearch.compute.data.Block;
 import org.elasticsearch.compute.data.DoubleBlock;
 import org.elasticsearch.compute.data.DoubleVector;
 import org.elasticsearch.compute.data.Page;
 import org.elasticsearch.compute.operator.EvalOperator;
+import org.elasticsearch.xpack.esql.expression.function.Warnings;
+import org.elasticsearch.xpack.ql.tree.Source;
 
 /**
  * {@link EvalOperator.ExpressionEvaluator} implementation for {@link Sinh}.
  * This class is generated. Do not edit it.
  */
 public final class SinhEvaluator implements EvalOperator.ExpressionEvaluator {
+  private final Warnings warnings;
+
   private final EvalOperator.ExpressionEvaluator val;
 
-  public SinhEvaluator(EvalOperator.ExpressionEvaluator val) {
+  public SinhEvaluator(Source source, EvalOperator.ExpressionEvaluator val) {
+    this.warnings = new Warnings(source);
     this.val = val;
   }
 
@@ -34,7 +40,7 @@ public Block eval(Page page) {
     if (valVector == null) {
       return eval(page.getPositionCount(), valBlock);
     }
-    return eval(page.getPositionCount(), valVector).asBlock();
+    return eval(page.getPositionCount(), valVector);
   }
 
   public DoubleBlock eval(int positionCount, DoubleBlock valBlock) {
@@ -44,15 +50,25 @@ public DoubleBlock eval(int positionCount, DoubleBlock valBlock) {
         result.appendNull();
         continue position;
       }
-      result.appendDouble(Sinh.process(valBlock.getDouble(valBlock.getFirstValueIndex(p))));
+      try {
+        result.appendDouble(Sinh.process(valBlock.getDouble(valBlock.getFirstValueIndex(p))));
+      } catch (ArithmeticException e) {
+        warnings.registerException(e);
+        result.appendNull();
+      }
     }
     return result.build();
   }
 
-  public DoubleVector eval(int positionCount, DoubleVector valVector) {
-    DoubleVector.Builder result = DoubleVector.newVectorBuilder(positionCount);
+  public DoubleBlock eval(int positionCount, DoubleVector valVector) {
+    DoubleBlock.Builder result = DoubleBlock.newBlockBuilder(positionCount);
     position: for (int p = 0; p < positionCount; p++) {
-      result.appendDouble(Sinh.process(valVector.getDouble(p)));
+      try {
+        result.appendDouble(Sinh.process(valVector.getDouble(p)));
+      } catch (ArithmeticException e) {
+        warnings.registerException(e);
+        result.appendNull();
+      }
     }
     return result.build();
   }

diff --git a/...ted/org/elasticsearch/xpack/esql/expression/function/scalar/math/SqrtDoubleEvaluator.java b/...ted/org/elasticsearch/xpack/esql/expression/function/scalar/math/SqrtDoubleEvaluator.java
@@ -4,22 +4,28 @@
 // 2.0.
 package org.elasticsearch.xpack.esql.expression.function.scalar.math;
 
+import java.lang.ArithmeticException;
 import java.lang.Override;
 import java.lang.String;
 import org.elasticsearch.compute.data.Block;
 import org.elasticsearch.compute.data.DoubleBlock;
 import org.elasticsearch.compute.data.DoubleVector;
 import org.elasticsearch.compute.data.Page;
 import org.elasticsearch.compute.operator.EvalOperator;
+import org.elasticsearch.xpack.esql.expression.function.Warnings;
+import org.elasticsearch.xpack.ql.tree.Source;
 
 /**
  * {@link EvalOperator.ExpressionEvaluator} implementation for {@link Sqrt}.
  * This class is generated. Do not edit it.
  */
 public final class SqrtDoubleEvaluator implements EvalOperator.ExpressionEvaluator {
+  private final Warnings warnings;
+
   private final EvalOperator.ExpressionEvaluator val;
 
-  public SqrtDoubleEvaluator(EvalOperator.ExpressionEvaluator val) {
+  public SqrtDoubleEvaluator(Source source, EvalOperator.ExpressionEvaluator val) {
+    this.warnings = new Warnings(source);
     this.val = val;
   }
 
@@ -34,7 +40,7 @@ public Block eval(Page page) {
     if (valVector == null) {
       return eval(page.getPositionCount(), valBlock);
     }
-    return eval(page.getPositionCount(), valVector).asBlock();
+    return eval(page.getPositionCount(), valVector);
   }
 
   public DoubleBlock eval(int positionCount, DoubleBlock valBlock) {
@@ -44,15 +50,25 @@ public DoubleBlock eval(int positionCount, DoubleBlock valBlock) {
         result.appendNull();
         continue position;
       }
-      result.appendDouble(Sqrt.process(valBlock.getDouble(valBlock.getFirstValueIndex(p))));
+      try {
+        result.appendDouble(Sqrt.process(valBlock.getDouble(valBlock.getFirstValueIndex(p))));
+      } catch (ArithmeticException e) {
+        warnings.registerException(e);
+        result.appendNull();
+      }
     }
     return result.build();
   }
 
-  public DoubleVector eval(int positionCount, DoubleVector valVector) {
-    DoubleVector.Builder result = DoubleVector.newVectorBuilder(positionCount);
+  public DoubleBlock eval(int positionCount, DoubleVector valVector) {
+    DoubleBlock.Builder result = DoubleBlock.newBlockBuilder(positionCount);
     position: for (int p = 0; p < positionCount; p++) {
-      result.appendDouble(Sqrt.process(valVector.getDouble(p)));
+      try {
+        result.appendDouble(Sqrt.process(valVector.getDouble(p)));
+      } catch (ArithmeticException e) {
+        warnings.registerException(e);
+        result.appendNull();
+      }
     }
     return result.build();
   }