Skip to content

Commit 2a1176a

Browse files
authored
ESQL: percentile agg accepting doubles but ignoring decimals (#134165)
- Ensure the expression in the percentile field (the second parameter) results in a double instead of an int. - Fix the percentile unit tests by altering expected values. - Add percentile csv-tests for both FROM and ROW source commands - The precision field in the count_distinct function (also the second parameter), though using the same code, is left as is since it is already expected to be an int. Resolves: #132500
1 parent 10bc896 commit 2a1176a

File tree

5 files changed

+47
-6
lines changed

5 files changed

+47
-6
lines changed

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,3 +205,33 @@ row a=0
205205
constant_single:double
206206
5
207207
;
208+
209+
percentile precision tests with table read
210+
required_capability: fix_percentile_precision
211+
212+
FROM employees
213+
| SORT emp_no
214+
| LIMIT 10
215+
| KEEP emp_no
216+
| STATS p1 = percentile(emp_no, 50), p2 = percentile(emp_no, 50.55), p3 = percentile(emp_no, 51.0),
217+
p4 = percentile(emp_no, 97), p5 = percentile(emp_no, 97.30), p6 = percentile(emp_no, 97.6)
218+
;
219+
220+
p1:double | p2:double | p3:double | p4:double | p5:double | p6:double
221+
10005.5 | 10005.5495 | 10005.59 | 10009.73 | 10009.757 | 10009.784
222+
;
223+
224+
percentile precision tests with row
225+
required_capability: fix_percentile_precision
226+
227+
ROW x = [1.1, 2.71, 3.14, 4.0, 5.55, 6, 7, 8, 9, 10, 11.99]
228+
| STATS p1=percentile(x, 10.5), p2=percentile(x, 10.55), p3=percentile(x, 11.0),
229+
p4=percentile(x, 90.0), p5=percentile(x, 90.28), p6=percentile(x, 90.7)
230+
;
231+
232+
p1:double | p2:double | p3:double | p4:double | p5:double | p6:double
233+
2.7315 | 2.73365 | 2.753 | 10.0 | 10.05572 | 10.1393
234+
;
235+
236+
237+

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1482,7 +1482,9 @@ public enum Cap {
14821482
/**
14831483
* Multivalued query parameters
14841484
*/
1485-
QUERY_PARAMS_MULTI_VALUES();
1485+
QUERY_PARAMS_MULTI_VALUES(),
1486+
1487+
FIX_PERCENTILE_PRECISION();
14861488

14871489
private final boolean enabled;
14881490

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/Foldables.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,4 +189,13 @@ public static int intValueOf(Expression field, String sourceText, String fieldNa
189189
Strings.format(null, "[{}] value must be a constant number in [{}], found [{}]", fieldName, sourceText, field)
190190
);
191191
}
192+
193+
public static double doubleValueOf(Expression field, String sourceText, String fieldName) {
194+
if (field instanceof Literal literal && literal.value() instanceof Number n) {
195+
return n.doubleValue();
196+
}
197+
throw new EsqlIllegalArgumentException(
198+
Strings.format(null, "[{}] value must be a constant number in [{}], found [{}]", fieldName, sourceText, field)
199+
);
200+
}
192201
}

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Percentile.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.ParamOrdinal.SECOND;
3838
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isFoldable;
3939
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isType;
40-
import static org.elasticsearch.xpack.esql.expression.Foldables.intValueOf;
40+
import static org.elasticsearch.xpack.esql.expression.Foldables.doubleValueOf;
4141

4242
public class Percentile extends NumericAggregate implements SurrogateExpression {
4343
public static final NamedWriteableRegistry.Entry ENTRY = new NamedWriteableRegistry.Entry(
@@ -168,8 +168,8 @@ protected AggregatorFunctionSupplier doubleSupplier() {
168168
return new PercentileDoubleAggregatorFunctionSupplier(percentileValue());
169169
}
170170

171-
private int percentileValue() {
172-
return intValueOf(percentile(), source().text(), "Percentile");
171+
private double percentileValue() {
172+
return doubleValueOf(percentile(), source().text(), "Percentile");
173173
}
174174

175175
@Override

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/aggregate/PercentileTests.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,14 +69,14 @@ private static TestCaseSupplier makeSupplier(
6969
var fieldTypedData = fieldSupplier.get();
7070
var percentileTypedData = percentileSupplier.get().forceLiteral();
7171

72-
var percentile = ((Number) percentileTypedData.data()).intValue();
72+
var percentile = ((Number) percentileTypedData.data()).doubleValue();
7373

7474
try (var digest = TDigestState.create(newLimitedBreaker(ByteSizeValue.ofMb(100)), 1000)) {
7575
for (var value : fieldTypedData.multiRowData()) {
7676
digest.add(((Number) value).doubleValue());
7777
}
7878

79-
var expected = digest.size() == 0 ? null : digest.quantile((double) percentile / 100);
79+
var expected = digest.size() == 0 ? null : digest.quantile(percentile / 100);
8080

8181
return new TestCaseSupplier.TestCase(
8282
List.of(fieldTypedData, percentileTypedData),

0 commit comments

Comments
 (0)