Skip to content

Commit

Permalink
[ES|QL] pow function always returns double (elastic#102183)
Browse files Browse the repository at this point in the history
This corrects an earlier mistake in the ES|QL language design. Initially we had thought to have pow return the same type as its inputs, but in practice even for integer inputs this quickly grows out of the representable range, and we returned null much of the time. This also created a lot of edge cases around casting to/from doubles (which the underlying java function uses). The version in this PR follows the java spec, by always casting its inputs to doubles, and returning a double. Doing it this way also allows for a rather significant reduction in lines of code.

I removed many of the tests covering pow specific edge cases. This seems reasonable to me as I expect java.lang.math.pow to be well behaved and most of those edge cases were around type testing which no longer applies. At the same time, this simplification allows us to leverage the new scalar function testing framework, which means better null coverage, better type coverage, and much easier extensibility.

We do consider this a breaking change, but as the feature is still in tech preview and this is a relatively small surface area, we are not too concerned with disruptions.

Resolves elastic#99055
Relates to elastic#100558
---------

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
  • Loading branch information
not-napoleon and elasticmachine authored Nov 21, 2023
1 parent 9cd96df commit 7345e64
Show file tree
Hide file tree
Showing 11 changed files with 138 additions and 876 deletions.
13 changes: 13 additions & 0 deletions docs/changelog/102183.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
pr: 102183
summary: "[ES|QL] pow function always returns double"
area: ES|QL
type: "breaking"
issues:
- 99055
breaking:
title: "[ES|QL] pow function always returns double"
area: REST API
details: "In ES|QL, the pow function no longer returns the type of its inputs, instead\
\ always returning a double."
impact: low. Most queries should continue to function with the change.
notable: false
59 changes: 2 additions & 57 deletions docs/reference/esql/functions/pow.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
image::esql/functions/signature/pow.svg[Embedded,opts=inline]

Returns the value of a base (first argument) raised to the power of an exponent (second argument).
Both arguments must be numeric.
Both arguments must be numeric. The output is always a double. Note that it is still possible to overflow
a double result here; in that case, null will be returned.

[source.merge.styled,esql]
----
Expand All @@ -16,62 +17,6 @@ include::{esql-specs}/math.csv-spec[tag=powDI]
include::{esql-specs}/math.csv-spec[tag=powDI-result]
|===

[discrete]
==== Type rules

The type of the returned value is determined by the types of the base and exponent.
The following rules are applied to determine the result type:

* If either of the base or exponent are of a floating point type, the result will be a double
* Otherwise, if either the base or the exponent are 64-bit (long or unsigned long), the result will be a long
* Otherwise, the result will be a 32-bit integer (this covers all other numeric types, including int, short and byte)

For example, using simple integers as arguments will lead to an integer result:

[source.merge.styled,esql]
----
include::{esql-specs}/math.csv-spec[tag=powII]
----
[%header.monospaced.styled,format=dsv,separator=|]
|===
include::{esql-specs}/math.csv-spec[tag=powII-result]
|===

NOTE: The actual power function is performed using double precision values for all cases.
This means that for very large non-floating point values there is a small chance that the
operation can lead to slightly different answers than expected.
However, a more likely outcome of very large non-floating point values is numerical overflow.

[discrete]
==== Arithmetic errors

Arithmetic errors and numeric overflow do not result in an error. Instead, the result will be `null`
and a warning for the `ArithmeticException` added.
For example:

[source.merge.styled,esql]
----
include::{esql-specs}/math.csv-spec[tag=powULOverrun]
----
[%header.monospaced.styled,format=dsv,separator=|]
|===
include::{esql-specs}/math.csv-spec[tag=powULOverrun-warning]
|===
[%header.monospaced.styled,format=dsv,separator=|]
|===
include::{esql-specs}/math.csv-spec[tag=powULOverrun-result]
|===

If it is desired to protect against numerical overruns, use `TO_DOUBLE` on either of the arguments:

[source.merge.styled,esql]
----
include::{esql-specs}/math.csv-spec[tag=pow2d]
----
[%header.monospaced.styled,format=dsv,separator=|]
|===
include::{esql-specs}/math.csv-spec[tag=pow2d-result]
|===

[discrete]
==== Fractional exponents
Expand Down
4 changes: 2 additions & 2 deletions docs/reference/esql/functions/types/pow.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ base | exponent | result
double | double | double
double | integer | double
integer | double | double
integer | integer | integer
integer | integer | double
long | double | double
long | integer | long
long | integer | double
|===
70 changes: 28 additions & 42 deletions x-pack/plugin/esql/qa/testFixtures/src/main/resources/math.csv-spec
Original file line number Diff line number Diff line change
Expand Up @@ -200,10 +200,10 @@ height:double | s:double
1.53 | 0.34
;

powSalarySquared
powSalarySquared#[skip:-8.11.99,reason:return type changed in 8.12]
from employees | eval s = pow(to_long(salary) - 75000, 2) + 10000 | keep salary, s | sort salary desc | limit 4;

salary:integer | s:long
salary:integer | s:double
74999 | 10001
74970 | 10900
74572 | 193184
Expand Down Expand Up @@ -328,14 +328,14 @@ base:integer | exponent:double | s:double
// end::powID-sqrt-result[]
;

powSqrtNeg
powSqrtNeg#[skip:-8.11.99,reason:return type changed in 8.12]
// tag::powNeg-sqrt[]
ROW base = -4, exponent = 0.5
| EVAL s = POW(base, exponent)
// end::powNeg-sqrt[]
;
warning:Line 2:12: evaluation of [POW(base, exponent)] failed, treating result as null. Only first 20 failures recorded.
warning:Line 2:12: java.lang.ArithmeticException: invalid result: pow(-4.0, 0.5)
warning:Line 2:12: java.lang.ArithmeticException: invalid result when computing pow

// tag::powNeg-sqrt-result[]
base:integer | exponent:double | s:double
Expand All @@ -356,23 +356,19 @@ base:double | exponent:integer | result:double
// end::powDI-result[]
;

powIntInt
// tag::powII[]
powIntInt#[skip:-8.11.99,reason:return type changed in 8.12]
ROW base = 2, exponent = 2
| EVAL s = POW(base, exponent)
// end::powII[]
;

// tag::powII-result[]
base:integer | exponent:integer | s:integer
2 | 2 | 4
// end::powII-result[]
base:integer | exponent:integer | s:double
2 | 2 | 4.0
;

powIntIntPlusInt
powIntIntPlusInt#[skip:-8.11.99,reason:return type changed in 8.12]
row s = 1 + pow(2, 2);

s:integer
s:double
5
;

Expand All @@ -383,24 +379,24 @@ s:double
5
;

powIntUL
powIntUL#[skip:-8.11.99,reason:return type changed in 8.12]
row x = pow(1, 9223372036854775808);

x:long
x:double
1
;

powLongUL
powLongUL#[skip:-8.11.99,reason:return type changed in 8.12]
row x = to_long(1) | eval x = pow(x, 9223372036854775808);

x:long
x:double
1
;

powUnsignedLongUL
powUnsignedLongUL#[skip:-8.11.99,reason:return type changed in 8.12]
row x = to_ul(1) | eval x = pow(x, 9223372036854775808);

x:long
x:double
1
;

Expand All @@ -411,36 +407,28 @@ x:double
1.0
;

powIntULOverrun
powIntULOverrun#[skip:-8.11.99,reason:return type changed in 8.12]
row x = pow(2, 9223372036854775808);
warning:Line 1:9: evaluation of [pow(2, 9223372036854775808)] failed, treating result as null. Only first 20 failures recorded.
warning:Line 1:9: java.lang.ArithmeticException: long overflow
warning:Line 1:9: java.lang.ArithmeticException: invalid result when computing pow

x:long
x:double
null
;

powULInt
powULInt#[skip:-8.11.99,reason:return type changed in 8.12]
row x = pow(to_unsigned_long(9223372036854775807), 1);

x:long
x:double
9223372036854775807
;

powULIntOverrun
// tag::powULOverrun[]
powULIntOverrun#[skip:-8.11.99,reason:return type changed in 8.12]
ROW x = POW(9223372036854775808, 2)
// end::powULOverrun[]
;
// tag::powULOverrun-warning[]
warning:Line 1:9: evaluation of [POW(9223372036854775808, 2)] failed, treating result as null. Only first 20 failures recorded.
warning:Line 1:9: java.lang.ArithmeticException: long overflow
// end::powULOverrun-warning[]

// tag::powULOverrun-result[]
x:long
null
// end::powULOverrun-result[]
x:double
8.507059173023462E37
;

powULInt_2d
Expand All @@ -455,20 +443,18 @@ x:double
// end::pow2d-result[]
;

powULLong
powULLong#[skip:-8.11.99,reason:return type changed in 8.12]
row x = to_long(10) | eval x = pow(to_unsigned_long(10), x);

x:long
x:double
10000000000
;

powULLongOverrun
powULLongOverrun#[skip:-8.11.99,reason:return type changed in 8.12]
row x = to_long(100) | eval x = pow(to_unsigned_long(10), x);
warning:Line 1:33: evaluation of [pow(to_unsigned_long(10), x)] failed, treating result as null. Only first 20 failures recorded.
warning:Line 1:33: java.lang.ArithmeticException: long overflow

x:long
null
x:double
1.0E100
;

powULDouble
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ mv_sum |? mv_sum(arg1:?)
now |? now() | null |null | null |? | "" | null | false
percentile |? percentile(arg1:?, arg2:?) |[arg1, arg2] |[?, ?] |["", ""] |? | "" | [false, false] | false
pi |? pi() | null | null | null |? | "" | null | false
pow |"? pow(base:integer|long|double, exponent:integer|double)" |[base, exponent] |["integer|long|double", "integer|double"] |["", ""] |? | "" | [false, false] | false
pow |"? pow(base:integer|unsigned_long|long|double, exponent:integer|unsigned_long|long|double)" |[base, exponent] |["integer|unsigned_long|long|double", "integer|unsigned_long|long|double"] |["", ""] |? | "" | [false, false] | false
replace |"? replace(arg1:?, arg2:?, arg3:?)" | [arg1, arg2, arg3] | [?, ?, ?] |["", "", ""] |? | "" | [false, false, false]| false
right |"? right(string:keyword, length:integer)" |[string, length] |["keyword", "integer"] |["", ""] |? | "" | [false, false] | false
round |? round(arg1:?, arg2:?) |[arg1, arg2] |[?, ?] |["", ""] |? | "" | [false, false] | false
Expand Down Expand Up @@ -145,7 +145,7 @@ synopsis:keyword
? now()
? percentile(arg1:?, arg2:?)
? pi()
"? pow(base:integer|long|double, exponent:integer|double)"
"? pow(base:integer|unsigned_long|long|double, exponent:integer|unsigned_long|long|double)"
"? replace(arg1:?, arg2:?, arg3:?)"
"? right(string:keyword, length:integer)"
? round(arg1:?, arg2:?)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
* {@link EvalOperator.ExpressionEvaluator} implementation for {@link Pow}.
* This class is generated. Do not edit it.
*/
public final class PowDoubleEvaluator implements EvalOperator.ExpressionEvaluator {
public final class PowEvaluator implements EvalOperator.ExpressionEvaluator {
private final Warnings warnings;

private final EvalOperator.ExpressionEvaluator base;
Expand All @@ -30,7 +30,7 @@ public final class PowDoubleEvaluator implements EvalOperator.ExpressionEvaluato

private final DriverContext driverContext;

public PowDoubleEvaluator(Source source, EvalOperator.ExpressionEvaluator base,
public PowEvaluator(Source source, EvalOperator.ExpressionEvaluator base,
EvalOperator.ExpressionEvaluator exponent, DriverContext driverContext) {
this.warnings = new Warnings(source);
this.base = base;
Expand Down Expand Up @@ -95,7 +95,7 @@ public DoubleBlock eval(int positionCount, DoubleVector baseVector, DoubleVector

@Override
public String toString() {
return "PowDoubleEvaluator[" + "base=" + base + ", exponent=" + exponent + "]";
return "PowEvaluator[" + "base=" + base + ", exponent=" + exponent + "]";
}

@Override
Expand All @@ -118,13 +118,13 @@ public Factory(Source source, EvalOperator.ExpressionEvaluator.Factory base,
}

@Override
public PowDoubleEvaluator get(DriverContext context) {
return new PowDoubleEvaluator(source, base.get(context), exponent.get(context), context);
public PowEvaluator get(DriverContext context) {
return new PowEvaluator(source, base.get(context), exponent.get(context), context);
}

@Override
public String toString() {
return "PowDoubleEvaluator[" + "base=" + base + ", exponent=" + exponent + "]";
return "PowEvaluator[" + "base=" + base + ", exponent=" + exponent + "]";
}
}
}
Loading

0 comments on commit 7345e64

Please sign in to comment.