Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions docs/changelog/132459.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 132459
summary: Small fixes for COPY_SIGN
area: ES|QL
type: bug
issues: []

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
:::

Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
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;
Expand Down Expand Up @@ -60,21 +61,20 @@ EvalOperator.ExpressionEvaluator.Factory create(

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
) {
Expand Down Expand Up @@ -125,11 +125,25 @@ public TypeResolution resolveType() {
}
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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
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;
Expand Down Expand Up @@ -357,6 +358,22 @@ public static List<TypedDataSupplier> getSuppliersForNumericType(DataType type,
throw new IllegalArgumentException("bogus numeric type [" + type + "]");
}

public static List<TypedDataSupplier> getSuppliersForNumericType(DataType type) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we just call this getSuppliersForType() and implement for everything? (Or leave it with an exception for unhandled cases until we need them)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me.

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,
Expand Down
Original file line number Diff line number Diff line change
@@ -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"));
}
}
Loading
Loading