Browse Source

ESQL: Small fixes for COPY_SIGN (#132459) (#132514)

Adds some small fixes to `COPY_SIGN` and some docs and tests. The fixes
are mostly supporting the literal `NULL` in parameters. `COPY_SIGN`
supports `float` arguments, but the tests don't cover it because we
don't build `float` blocks. Otherwise, the tests are fairly standard.
Nik Everett 2 months ago
parent
commit
9b49b5797a

+ 5 - 0
docs/changelog/132459.yaml

@@ -0,0 +1,5 @@
+pr: 132459
+summary: Small fixes for COPY_SIGN
+area: ES|QL
+type: bug
+issues: []

+ 6 - 0
docs/reference/query-languages/esql/_snippets/functions/description/copy_sign.md

@@ -0,0 +1,6 @@
+% This is generated by ESQL's AbstractFunctionTestCase. Do not edit it. See ../README.md for how to regenerate it.
+
+**Description**
+
+Returns a value with the magnitude of the first argument and the sign of the second argument. This function is similar to Java's Math.copySign(double magnitude, double sign) which is similar to `copysign` from [IEEE 754](https://en.wikipedia.org/wiki/IEEE_754).
+

+ 20 - 0
docs/reference/query-languages/esql/_snippets/functions/layout/copy_sign.md

@@ -0,0 +1,20 @@
+% This is generated by ESQL's AbstractFunctionTestCase. Do not edit it. See ../README.md for how to regenerate it.
+
+## `COPY_SIGN` [esql-copy_sign]
+
+**Syntax**
+
+:::{image} ../../../images/functions/copy_sign.svg
+:alt: Embedded
+:class: text-center
+:::
+
+
+:::{include} ../parameters/copy_sign.md
+:::
+
+:::{include} ../description/copy_sign.md
+:::
+
+:::{include} ../types/copy_sign.md
+:::

+ 10 - 0
docs/reference/query-languages/esql/_snippets/functions/parameters/copy_sign.md

@@ -0,0 +1,10 @@
+% This is generated by ESQL's AbstractFunctionTestCase. Do not edit it. See ../README.md for how to regenerate it.
+
+**Parameters**
+
+`magnitude`
+:   The expression providing the magnitude of the result. Must be a numeric type.
+
+`sign`
+:   The expression providing the sign of the result. Must be a numeric type.
+

+ 16 - 0
docs/reference/query-languages/esql/_snippets/functions/types/copy_sign.md

@@ -0,0 +1,16 @@
+% This is generated by ESQL's AbstractFunctionTestCase. Do not edit it. See ../README.md for how to regenerate it.
+
+**Supported types**
+
+| magnitude | sign | result |
+| --- | --- | --- |
+| double | double | double |
+| double | integer | double |
+| double | long | double |
+| integer | double | integer |
+| integer | integer | integer |
+| integer | long | integer |
+| long | double | long |
+| long | integer | long |
+| long | long | long |
+

+ 1 - 0
docs/reference/query-languages/esql/_snippets/lists/math-functions.md

@@ -5,6 +5,7 @@
 * [`ATAN2`](../../functions-operators/math-functions.md#esql-atan2)
 * [`CBRT`](../../functions-operators/math-functions.md#esql-cbrt)
 * [`CEIL`](../../functions-operators/math-functions.md#esql-ceil)
+* [`COPY_SIGN`](../../functions-operators/math-functions.md#esql-copy_sign)
 * [`COS`](../../functions-operators/math-functions.md#esql-cos)
 * [`COSH`](../../functions-operators/math-functions.md#esql-cosh)
 * [`E`](../../functions-operators/math-functions.md#esql-e)

+ 3 - 0
docs/reference/query-languages/esql/functions-operators/math-functions.md

@@ -33,6 +33,9 @@ mapped_pages:
 :::{include} ../_snippets/functions/layout/ceil.md
 :::
 
+:::{include} ../_snippets/functions/layout/copy_sign.md
+:::
+
 :::{include} ../_snippets/functions/layout/cos.md
 :::
 

+ 1 - 0
docs/reference/query-languages/esql/images/functions/copy_sign.svg

@@ -0,0 +1 @@
+<svg version="1.1" xmlns:xlink="http://www.w3.org/1999/xlink" xmlns="http://www.w3.org/2000/svg" width="480" height="46" viewbox="0 0 480 46"><defs><style type="text/css">.c{fill:none;stroke:#222222;}.k{fill:#000000;font-family: ui-monospace, SFMono-Regular, Menlo, Monaco, Consolas, "Liberation Mono", "Courier New", monospace;font-size:20px;}.s{fill:#e4f4ff;stroke:#222222;}.syn{fill:#8D8D8D;font-family: ui-monospace, SFMono-Regular, Menlo, Monaco, Consolas, "Liberation Mono", "Courier New", monospace;font-size:20px;}</style></defs><path class="c" d="M0 31h5m128 0h10m32 0h10m128 0h10m32 0h10m68 0h10m32 0h5"/><rect class="s" x="5" y="5" width="128" height="36"/><text class="k" x="15" y="31">COPY_SIGN</text><rect class="s" x="143" y="5" width="32" height="36" rx="7"/><text class="syn" x="153" y="31">(</text><rect class="s" x="185" y="5" width="128" height="36" rx="7"/><text class="k" x="195" y="31">magnitude</text><rect class="s" x="323" y="5" width="32" height="36" rx="7"/><text class="syn" x="333" y="31">,</text><rect class="s" x="365" y="5" width="68" height="36" rx="7"/><text class="k" x="375" y="31">sign</text><rect class="s" x="443" y="5" width="32" height="36" rx="7"/><text class="syn" x="453" y="31">)</text></svg>

+ 172 - 0
docs/reference/query-languages/esql/kibana/definition/functions/copy_sign.json

@@ -0,0 +1,172 @@
+{
+  "comment" : "This is generated by ESQL's AbstractFunctionTestCase. Do not edit it. See ../README.md for how to regenerate it.",
+  "type" : "scalar",
+  "name" : "copy_sign",
+  "description" : "Returns a value with the magnitude of the first argument and the sign of the second argument.\nThis function is similar to Java's Math.copySign(double magnitude, double sign) which is\nsimilar to `copysign` from IEEE 754.",
+  "signatures" : [
+    {
+      "params" : [
+        {
+          "name" : "magnitude",
+          "type" : "double",
+          "optional" : false,
+          "description" : "The expression providing the magnitude of the result. Must be a numeric type."
+        },
+        {
+          "name" : "sign",
+          "type" : "double",
+          "optional" : false,
+          "description" : "The expression providing the sign of the result. Must be a numeric type."
+        }
+      ],
+      "variadic" : false,
+      "returnType" : "double"
+    },
+    {
+      "params" : [
+        {
+          "name" : "magnitude",
+          "type" : "double",
+          "optional" : false,
+          "description" : "The expression providing the magnitude of the result. Must be a numeric type."
+        },
+        {
+          "name" : "sign",
+          "type" : "integer",
+          "optional" : false,
+          "description" : "The expression providing the sign of the result. Must be a numeric type."
+        }
+      ],
+      "variadic" : false,
+      "returnType" : "double"
+    },
+    {
+      "params" : [
+        {
+          "name" : "magnitude",
+          "type" : "double",
+          "optional" : false,
+          "description" : "The expression providing the magnitude of the result. Must be a numeric type."
+        },
+        {
+          "name" : "sign",
+          "type" : "long",
+          "optional" : false,
+          "description" : "The expression providing the sign of the result. Must be a numeric type."
+        }
+      ],
+      "variadic" : false,
+      "returnType" : "double"
+    },
+    {
+      "params" : [
+        {
+          "name" : "magnitude",
+          "type" : "integer",
+          "optional" : false,
+          "description" : "The expression providing the magnitude of the result. Must be a numeric type."
+        },
+        {
+          "name" : "sign",
+          "type" : "double",
+          "optional" : false,
+          "description" : "The expression providing the sign of the result. Must be a numeric type."
+        }
+      ],
+      "variadic" : false,
+      "returnType" : "integer"
+    },
+    {
+      "params" : [
+        {
+          "name" : "magnitude",
+          "type" : "integer",
+          "optional" : false,
+          "description" : "The expression providing the magnitude of the result. Must be a numeric type."
+        },
+        {
+          "name" : "sign",
+          "type" : "integer",
+          "optional" : false,
+          "description" : "The expression providing the sign of the result. Must be a numeric type."
+        }
+      ],
+      "variadic" : false,
+      "returnType" : "integer"
+    },
+    {
+      "params" : [
+        {
+          "name" : "magnitude",
+          "type" : "integer",
+          "optional" : false,
+          "description" : "The expression providing the magnitude of the result. Must be a numeric type."
+        },
+        {
+          "name" : "sign",
+          "type" : "long",
+          "optional" : false,
+          "description" : "The expression providing the sign of the result. Must be a numeric type."
+        }
+      ],
+      "variadic" : false,
+      "returnType" : "integer"
+    },
+    {
+      "params" : [
+        {
+          "name" : "magnitude",
+          "type" : "long",
+          "optional" : false,
+          "description" : "The expression providing the magnitude of the result. Must be a numeric type."
+        },
+        {
+          "name" : "sign",
+          "type" : "double",
+          "optional" : false,
+          "description" : "The expression providing the sign of the result. Must be a numeric type."
+        }
+      ],
+      "variadic" : false,
+      "returnType" : "long"
+    },
+    {
+      "params" : [
+        {
+          "name" : "magnitude",
+          "type" : "long",
+          "optional" : false,
+          "description" : "The expression providing the magnitude of the result. Must be a numeric type."
+        },
+        {
+          "name" : "sign",
+          "type" : "integer",
+          "optional" : false,
+          "description" : "The expression providing the sign of the result. Must be a numeric type."
+        }
+      ],
+      "variadic" : false,
+      "returnType" : "long"
+    },
+    {
+      "params" : [
+        {
+          "name" : "magnitude",
+          "type" : "long",
+          "optional" : false,
+          "description" : "The expression providing the magnitude of the result. Must be a numeric type."
+        },
+        {
+          "name" : "sign",
+          "type" : "long",
+          "optional" : false,
+          "description" : "The expression providing the sign of the result. Must be a numeric type."
+        }
+      ],
+      "variadic" : false,
+      "returnType" : "long"
+    }
+  ],
+  "preview" : false,
+  "snapshot_only" : false
+}

+ 6 - 0
docs/reference/query-languages/esql/kibana/docs/functions/copy_sign.md

@@ -0,0 +1,6 @@
+% This is generated by ESQL's AbstractFunctionTestCase. Do not edit it. See ../README.md for how to regenerate it.
+
+### COPY SIGN
+Returns a value with the magnitude of the first argument and the sign of the second argument.
+This function is similar to Java's Math.copySign(double magnitude, double sign) which is
+similar to `copysign` from [IEEE 754](https://en.wikipedia.org/wiki/IEEE_754).

+ 25 - 11
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/CopySign.java

@@ -15,6 +15,7 @@ import org.elasticsearch.compute.operator.EvalOperator;
 import org.elasticsearch.xpack.esql.EsqlIllegalArgumentException;
 import org.elasticsearch.xpack.esql.core.expression.Expression;
 import org.elasticsearch.xpack.esql.core.expression.Expressions;
+import org.elasticsearch.xpack.esql.core.expression.TypeResolutions;
 import org.elasticsearch.xpack.esql.core.tree.NodeInfo;
 import org.elasticsearch.xpack.esql.core.tree.Source;
 import org.elasticsearch.xpack.esql.core.type.DataType;
@@ -60,21 +61,20 @@ public class CopySign extends EsqlScalarFunction {
 
     private DataType dataType;
 
-    @FunctionInfo(
-        description = "Returns a value with the magnitude of the first argument and the sign of the second argument. "
-            + "This function is similar to Java's Math.copySign(double magnitude, double sign).",
-        returnType = { "double", "float" }
-    )
+    @FunctionInfo(description = """
+        Returns a value with the magnitude of the first argument and the sign of the second argument.
+        This function is similar to Java's Math.copySign(double magnitude, double sign) which is
+        similar to `copysign` from [IEEE 754](https://en.wikipedia.org/wiki/IEEE_754).""", returnType = { "double", "integer", "long" })
     public CopySign(
         Source source,
         @Param(
             name = "magnitude",
-            type = { "double", "float", "integer", "long" },
+            type = { "double", "integer", "long" },
             description = "The expression providing the magnitude of the result. Must be a numeric type."
         ) Expression magnitude,
         @Param(
             name = "sign",
-            type = { "double", "float", "integer", "long" },
+            type = { "double", "integer", "long" },
             description = "The expression providing the sign of the result. Must be a numeric type."
         ) Expression sign
     ) {
@@ -125,11 +125,25 @@ public class CopySign extends EsqlScalarFunction {
         }
         var magnitude = children().get(0);
         var sign = children().get(1);
-        if (magnitude.dataType().isNumeric() == false) {
-            return new TypeResolution("Magnitude must be a numeric type");
+        TypeResolution resolution = TypeResolutions.isType(
+            magnitude,
+            t -> t.isNumeric() && t != DataType.UNSIGNED_LONG,
+            sourceText(),
+            TypeResolutions.ParamOrdinal.FIRST,
+            "numeric"
+        );
+        if (resolution.unresolved()) {
+            return resolution;
         }
-        if (sign.dataType().isNumeric() == false) {
-            return new TypeResolution("Sign must be a numeric type");
+        resolution = TypeResolutions.isType(
+            sign,
+            t -> t.isNumeric() && t != DataType.UNSIGNED_LONG,
+            sourceText(),
+            TypeResolutions.ParamOrdinal.SECOND,
+            "numeric"
+        );
+        if (resolution.unresolved()) {
+            return resolution;
         }
         // The return type is the same as the magnitude type, so we can use it directly.
         dataType = magnitude.dataType();

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

@@ -47,6 +47,7 @@ import java.util.function.Supplier;
 import java.util.function.UnaryOperator;
 import java.util.stream.Collectors;
 
+import static org.elasticsearch.xpack.esql.core.util.NumericUtils.UNSIGNED_LONG_MAX;
 import static org.elasticsearch.xpack.esql.core.util.SpatialCoordinateTypes.CARTESIAN;
 import static org.elasticsearch.xpack.esql.core.util.SpatialCoordinateTypes.GEO;
 import static org.hamcrest.Matchers.equalTo;
@@ -357,6 +358,26 @@ public record TestCaseSupplier(String name, List<DataType> types, Supplier<TestC
         throw new IllegalArgumentException("bogus numeric type [" + type + "]");
     }
 
+    /**
+     * A {@link List} of the cases for the specified type without any limits.
+     * See {@link #getSuppliersForNumericType} for cases with limits on numbers.
+     */
+    public static List<TypedDataSupplier> unlimitedSuppliers(DataType type) {
+        if (type == DataType.INTEGER) {
+            return intCases(Integer.MIN_VALUE, Integer.MAX_VALUE, true);
+        }
+        if (type == DataType.LONG) {
+            return longCases(Long.MIN_VALUE, Long.MAX_VALUE, true);
+        }
+        if (type == DataType.UNSIGNED_LONG) {
+            return ulongCases(BigInteger.ZERO, UNSIGNED_LONG_MAX, true);
+        }
+        if (type == DataType.DOUBLE) {
+            return doubleCases(-Double.MAX_VALUE, Double.MAX_VALUE, true);
+        }
+        throw new IllegalArgumentException("bogus numeric type [" + type + "]");
+    }
+
     public static List<TestCaseSupplier> forBinaryComparisonWithWidening(
         NumericTypeTestConfigs<Boolean> typeStuff,
         String lhsName,

+ 37 - 0
x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/CopySignErrorTests.java

@@ -0,0 +1,37 @@
+/*
+ * 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.math;
+
+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.ErrorsForCasesWithoutExamplesTestCase;
+import org.elasticsearch.xpack.esql.expression.function.TestCaseSupplier;
+import org.hamcrest.Matcher;
+
+import java.util.List;
+import java.util.Set;
+
+import static org.hamcrest.Matchers.equalTo;
+
+public class CopySignErrorTests extends ErrorsForCasesWithoutExamplesTestCase {
+    @Override
+    protected List<TestCaseSupplier> cases() {
+        return paramsToSuppliers(CopySignTests.parameters());
+    }
+
+    @Override
+    protected Expression build(Source source, List<Expression> args) {
+        return new CopySign(source, args.get(0), args.get(1));
+    }
+
+    @Override
+    protected Matcher<String> expectedTypeErrorMatcher(List<Set<DataType>> validPerPosition, List<DataType> signature) {
+        return equalTo(typeErrorMessage(true, validPerPosition, signature, (v, i) -> "numeric"));
+    }
+}

+ 106 - 0
x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/CopySignTests.java

@@ -0,0 +1,106 @@
+/*
+ * 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.math;
+
+import com.carrotsearch.randomizedtesting.annotations.Name;
+import com.carrotsearch.randomizedtesting.annotations.ParametersFactory;
+
+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 org.hamcrest.Matcher;
+import org.hamcrest.Matchers;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.function.BiFunction;
+import java.util.function.BinaryOperator;
+import java.util.function.Supplier;
+
+import static org.elasticsearch.xpack.esql.expression.function.TestCaseSupplier.casesCrossProduct;
+import static org.elasticsearch.xpack.esql.expression.function.TestCaseSupplier.getCastEvaluator;
+import static org.elasticsearch.xpack.esql.expression.function.TestCaseSupplier.unlimitedSuppliers;
+import static org.hamcrest.Matchers.equalTo;
+
+public class CopySignTests extends AbstractScalarFunctionTestCase {
+    public CopySignTests(@Name("TestCase") Supplier<TestCaseSupplier.TestCase> testCaseSupplier) {
+        this.testCase = testCaseSupplier.get();
+    }
+
+    @ParametersFactory
+    public static Iterable<Object[]> parameters() {
+        List<TestCaseSupplier> suppliers = new ArrayList<>();
+        List<DataType> numericTypes = List.of(DataType.INTEGER, DataType.LONG, DataType.DOUBLE);
+
+        for (DataType lhsType : numericTypes) {
+            for (DataType rhsType : numericTypes) {
+                BinaryOperator<Object> expected = (lhs, rhs) -> {
+                    double sign = ((Number) rhs).doubleValue();
+                    return switch (lhs) {
+                        case Integer v -> {
+                            if (sign < 0) {
+                                yield v > 0 ? -v : v;
+                            }
+                            yield v > 0 ? v : -v;
+                        }
+                        case Long v -> {
+                            if (sign < 0) {
+                                yield v > 0 ? -v : v;
+                            }
+                            yield v > 0 ? v : -v;
+                        }
+                        case Double v -> Math.copySign(v, sign);
+                        case Float v -> Math.copySign(v, sign);
+                        default -> throw new IllegalArgumentException("unsupported [" + lhs.getClass() + "]");
+                    };
+                };
+                BiFunction<DataType, DataType, Matcher<String>> evaluatorToString = (lhs, rhs) -> {
+                    String name = "CopySign" + switch (lhs) {
+                        case INTEGER -> "Integer";
+                        case LONG -> "Long";
+                        case DOUBLE -> "Double";
+                        case FLOAT -> "Float";
+                        default -> throw new IllegalStateException("unsupported [" + lhs + "]");
+                    } + "Evaluator";
+                    return equalTo(
+                        name
+                            + "[magnitude=Attribute[channel=0], sign="
+                            + getCastEvaluator("Attribute[channel=1]", rhs, DataType.DOUBLE)
+                            + "]"
+                    );
+                };
+                casesCrossProduct(
+                    expected,
+                    unlimitedSuppliers(lhsType),
+                    unlimitedSuppliers(rhsType),
+                    evaluatorToString,
+                    (l, r) -> List.of(),
+                    suppliers,
+                    lhsType,
+                    false
+                );
+            }
+        }
+
+        return parameterSuppliersFromTypedData(
+            anyNullIsNull(randomizeBytesRefsOffset(suppliers), (nullPosition, nullValueDataType, original) -> {
+                if (nullPosition == 0 && nullValueDataType == DataType.NULL) {
+                    return DataType.NULL;
+                }
+                return original.expectedType();
+            }, (nullPosition, nullData, original) -> nullData.isForceLiteral() ? Matchers.equalTo("LiteralsEvaluator[lit=null]") : original)
+        );
+    }
+
+    @Override
+    protected Expression build(Source source, List<Expression> args) {
+        return new CopySign(source, args.get(0), args.get(1));
+    }
+}