Skip to content

Commit c80bd11

Browse files
committed
enforcing floating point only for CopySign operation
1 parent 7e053c9 commit c80bd11

File tree

1 file changed

+14
-12
lines changed
  • x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/math

1 file changed

+14
-12
lines changed

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

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import org.elasticsearch.xpack.esql.expression.function.Param;
2424
import org.elasticsearch.xpack.esql.expression.function.scalar.EsqlScalarFunction;
2525
import org.elasticsearch.xpack.esql.io.stream.PlanStreamInput;
26+
import org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter;
2627

2728
import java.io.IOException;
2829
import java.util.Arrays;
@@ -47,8 +48,10 @@ EvalOperator.ExpressionEvaluator.Factory create(
4748
}
4849

4950
private static final Map<DataType, CopySignFactoryProvider> FACTORY_PROVIDERS = Map.of(
50-
DataType.FLOAT, CopySignFloatEvaluator.Factory::new,
51-
DataType.DOUBLE, CopySignDoubleEvaluator.Factory::new
51+
DataType.FLOAT,
52+
CopySignFloatEvaluator.Factory::new,
53+
DataType.DOUBLE,
54+
CopySignDoubleEvaluator.Factory::new
5255
);
5356

5457
private DataType dataType;
@@ -119,26 +122,24 @@ public TypeResolution resolveType() {
119122
}
120123
var magnitude = children().get(0);
121124
var sign = children().get(1);
122-
if (magnitude.dataType().isNumeric() == false) {
123-
return new TypeResolution("Magnitude must be a numeric type");
125+
if (magnitude.dataType().isNumeric() == false || magnitude.dataType().isRationalNumber() == false) {
126+
return new TypeResolution("Magnitude must be a float or double type");
124127
}
125-
if (sign.dataType().isNumeric() == false) {
126-
return new TypeResolution("Sign must be a numeric type");
128+
if (sign.dataType().isNumeric() == false || sign.dataType().isRationalNumber() == false) {
129+
return new TypeResolution("Sign must be a float or double type");
127130
}
128-
// TODO(pabloem): Looks like we don't currently support generalizing types
129-
// when two types are incompatible (e.g. copySign(int, long) -> long instead of
130-
// copySign(int, long) => ERROR).
131-
dataType = magnitude.dataType();
131+
var commonType = EsqlDataTypeConverter.commonType(magnitude.dataType(), sign.dataType());
132132
TypeResolution resolution = TypeResolutions.isType(
133133
magnitude,
134-
t -> t == dataType,
134+
t -> t == commonType,
135135
sourceText(),
136136
TypeResolutions.ParamOrdinal.fromIndex(1),
137137
magnitude.dataType().typeName()
138138
);
139139
if (resolution.unresolved()) {
140140
return resolution;
141141
}
142+
dataType = commonType;
142143
return TypeResolution.TYPE_RESOLVED;
143144
}
144145

@@ -151,7 +152,8 @@ public boolean foldable() {
151152
public EvalOperator.ExpressionEvaluator.Factory toEvaluator(ToEvaluator toEvaluator) {
152153
var dataType = dataType();
153154
if (FACTORY_PROVIDERS.containsKey(dataType)) {
154-
return FACTORY_PROVIDERS.get(dataType).create(source(), toEvaluator.apply(children().get(0)), toEvaluator.apply(children().get(1)));
155+
return FACTORY_PROVIDERS.get(dataType)
156+
.create(source(), toEvaluator.apply(children().get(0)), toEvaluator.apply(children().get(1)));
155157
} else {
156158
throw new EsqlIllegalArgumentException("Unsupported data type [{}] for function [{}]", dataType, NAME);
157159
}

0 commit comments

Comments
 (0)