Skip to content

Commit c5bdd94

Browse files
committed
address comments
1 parent e786889 commit c5bdd94

File tree

2 files changed

+10
-16
lines changed
  • x-pack/plugin/esql
    • qa/testFixtures/src/main/resources
    • src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/math

2 files changed

+10
-16
lines changed

x-pack/plugin/esql/qa/testFixtures/src/main/resources/math.csv-spec

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1849,11 +1849,10 @@ required_capability: copy_sign
18491849
ROW cs1=COPY_SIGN(10.0, -5.0), cs2=COPY_SIGN(10, 1), cs3=COPY_SIGN(1, -5.0);
18501850

18511851
cs1:double | cs2:integer | cs3:integer
1852-
-10.0 | 10 | -1
1852+
-10.0 | 10 | -1
18531853
;
18541854

18551855
copySignWithMixedNumericValuesWithMV
1856-
required_capability: mixed_numeric_types_in_case_greatest_least
18571856
required_capability: copy_sign
18581857

18591858
FROM employees

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

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -46,14 +46,10 @@ EvalOperator.ExpressionEvaluator.Factory create(
4646
}
4747

4848
private static final Map<DataType, CopySignFactoryProvider> FACTORY_PROVIDERS = Map.of(
49-
DataType.FLOAT,
50-
CopySignFloatEvaluator.Factory::new,
51-
DataType.DOUBLE,
52-
CopySignDoubleEvaluator.Factory::new,
53-
DataType.LONG,
54-
CopySignLongEvaluator.Factory::new,
55-
DataType.INTEGER,
56-
CopySignIntegerEvaluator.Factory::new
49+
Map.entry(DataType.FLOAT, CopySignFloatEvaluator.Factory::new),
50+
Map.entry(DataType.DOUBLE, CopySignDoubleEvaluator.Factory::new),
51+
Map.entry(DataType.LONG, CopySignLongEvaluator.Factory::new),
52+
Map.entry(DataType.INTEGER, CopySignIntegerEvaluator.Factory::new)
5753
);
5854

5955
private DataType dataType;
@@ -147,13 +143,12 @@ public EvalOperator.ExpressionEvaluator.Factory toEvaluator(ToEvaluator toEvalua
147143
}
148144
var sign = children().get(1);
149145
var magnitude = children().get(0);
146+
// The output of this function is the MAGNITUDE with the SIGN from `sign` applied to it.
147+
// For that reason, we cast the SIGN to DOUBLE, which is the most general numeric type,
148+
// and allows us to write a single check (<0 or >=0) for all possible types of `sign`.
149+
// However, the output type of this function is determined by the `magnitude` type.
150150
return FACTORY_PROVIDERS.get(dataType)
151-
.create(
152-
source(),
153-
toEvaluator.apply(magnitude),
154-
// We always cast the sign to double for processing, as the sign can be any numeric type.
155-
Cast.cast(source(), sign.dataType(), DataType.DOUBLE, toEvaluator.apply(sign))
156-
);
151+
.create(source(), toEvaluator.apply(magnitude), Cast.cast(source(), sign.dataType(), DataType.DOUBLE, toEvaluator.apply(sign)));
157152
}
158153

159154
@Evaluator(extraName = "Float")

0 commit comments

Comments
 (0)