From 4adbfee64fbf2353673e73a7966072772b7df955 Mon Sep 17 00:00:00 2001 From: Jan Kuipers Date: Tue, 9 Sep 2025 17:33:00 +0200 Subject: [PATCH 01/11] Propagate filter in TOP(field, 1, order) surrogate. --- .../src/main/resources/stats_top.csv-spec | 14 ++++++++++++++ .../xpack/esql/action/EsqlCapabilities.java | 7 ++++++- .../esql/expression/function/aggregate/Top.java | 4 ++-- 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/stats_top.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/stats_top.csv-spec index 2165ee42419c2..c199ac97b4c4e 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/stats_top.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/stats_top.csv-spec @@ -300,3 +300,17 @@ FROM books The Lord of the Rings Poster Collection: Six Paintings by Alan Lee (No. 1) | he Lo | [J. R. R. Tolkien, Alan Lee] | Alan Lee A Gentle Creature and Other Stories: White Nights, A Gentle Creature, and The Dream of a Ridiculous Man (The World's Classics) | Gent | [W. J. Leatherbarrow, Fyodor Dostoevsky, Alan Myers] | Alan Myers ; + +topWithConditions +required_capability: stats_top_1_with_condition_fixed + +FROM employees +| STATS min1 = TOP(emp_no, 1, "ASC") WHERE emp_no > 10010, + min2 = TOP(emp_no, 2, "ASC") WHERE emp_no > 10010, + max1 = TOP(emp_no, 1, "DESC") WHERE emp_no < 10080, + max2 = TOP(emp_no, 2, "DESC") WHERE emp_no < 10080 +; + +min1:integer | min2:integer | max1:integer | max2:integer +10011 | [10011, 10012] | 10079 | [10079, 10078] +; diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java index 3f921b33fd88a..38b462329cb2f 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java @@ -1462,7 +1462,12 @@ public enum Cap { /** * Support for the Present function */ - FN_PRESENT; + FN_PRESENT, + + /** + * Bugfix for STATS TOP(field, 1, order) WHERE condition. + */ + STATS_TOP_1_WITH_CONDITION_FIXED; private final boolean enabled; diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Top.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Top.java index b213f2120a7ac..bbb32bcbd0c53 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Top.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Top.java @@ -289,9 +289,9 @@ public Expression surrogate() { var s = source(); if (orderField() instanceof Literal && limitField() instanceof Literal && limitValue() == 1) { if (orderValue()) { - return new Min(s, field()); + return new Min(s, field(), filter()); } else { - return new Max(s, field()); + return new Max(s, field(), filter()); } } return null; From 7ebadef6955aa74b7e64f2ef33203018c823a215 Mon Sep 17 00:00:00 2001 From: Jan Kuipers <148754765+jan-elastic@users.noreply.github.com> Date: Tue, 9 Sep 2025 17:35:05 +0200 Subject: [PATCH 02/11] Update docs/changelog/134376.yaml --- docs/changelog/134376.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 docs/changelog/134376.yaml diff --git a/docs/changelog/134376.yaml b/docs/changelog/134376.yaml new file mode 100644 index 0000000000000..35ee17ca237cb --- /dev/null +++ b/docs/changelog/134376.yaml @@ -0,0 +1,6 @@ +pr: 134376 +summary: "Propagate filter in TOP(field, 1, order) surrogate" +area: ES|QL +type: bug +issues: + - 134293 From b0a771bb3694d13942b1b365878d747eccbfd85b Mon Sep 17 00:00:00 2001 From: Jan Kuipers <148754765+jan-elastic@users.noreply.github.com> Date: Tue, 9 Sep 2025 17:38:31 +0200 Subject: [PATCH 03/11] changelog description --- docs/changelog/134376.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changelog/134376.yaml b/docs/changelog/134376.yaml index 35ee17ca237cb..b17ab5953b3b0 100644 --- a/docs/changelog/134376.yaml +++ b/docs/changelog/134376.yaml @@ -1,5 +1,5 @@ pr: 134376 -summary: "Propagate filter in TOP(field, 1, order) surrogate" +summary: "Fix ES|QL TOP 1 aggregation with condition" area: ES|QL type: bug issues: From d4a3244befca0ae6cf3706985210374780e94fdb Mon Sep 17 00:00:00 2001 From: ncordon Date: Wed, 10 Sep 2025 17:15:13 +0200 Subject: [PATCH 04/11] Fixes filter part on aggregation functions' surrogates --- .../src/main/resources/stats.csv-spec | 159 ++++++++++++++++++ .../src/main/resources/stats_top.csv-spec | 14 -- .../xpack/esql/action/EsqlCapabilities.java | 6 +- .../expression/function/aggregate/Count.java | 4 +- .../expression/function/aggregate/Max.java | 2 +- .../expression/function/aggregate/Median.java | 2 +- .../expression/function/aggregate/Min.java | 2 +- .../expression/function/aggregate/Sum.java | 6 +- .../function/aggregate/WeightedAvg.java | 4 +- 9 files changed, 175 insertions(+), 24 deletions(-) diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/stats.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/stats.csv-spec index 8d8e0d8f74427..50dc51217df82 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/stats.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/stats.csv-spec @@ -3145,3 +3145,162 @@ FROM employees m:datetime | x:integer | d:boolean 1999-04-30T00:00:00.000Z | 2 | true ; + +sumWithConditions +required_capability: stats_with_filtered_surrogate_fixed + +FROM employees +| STATS sum1 = SUM(1), + sum2 = SUM(1) WHERE emp_no == 10080, + sum3 = SUM(1) WHERE emp_no < 10080, + sum4 = SUM(1) WHERE emp_no >= 10080 +; + +sum1:long | sum2:long | sum3:long | sum4:long +100 | 1 | 79 | 21 +; + +weightedAvgWithConditions +required_capability: stats_with_filtered_surrogate_fixed + +ROW x = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10] +| MV_EXPAND x +| STATS w_avg1 = WEIGHTED_AVG(x, 1) WHERE x == 5, + w_avg2 = WEIGHTED_AVG(x, x) WHERE x == 5, + w_avg3 = WEIGHTED_AVG(x, 2) WHERE x <= 5, + w_avg4 = WEIGHTED_AVG(x, x) WHERE x > 5 +; + +w_avg1:double | w_avg2:double | w_avg3:double | w_avg4:double +5.0 | 5.0 | 3.0 | 8.25 +; + +maxWithConditions +required_capability: stats_with_filtered_surrogate_fixed +required_capability: aggregate_metric_double_convert_to + +ROW x = [1, 2, 3, 4, 5] +| MV_EXPAND x +| EVAL agg_metric = TO_AGGREGATE_METRIC_DOUBLE(x) +| STATS max = MAX(agg_metric) WHERE x <= 3 +; + +max:double +3.0 +; + +minWithConditions +required_capability: stats_with_filtered_surrogate_fixed +required_capability: aggregate_metric_double_convert_to + +ROW x = [1, 2, 3, 4, 5] +| MV_EXPAND x +| EVAL agg_metric = TO_AGGREGATE_METRIC_DOUBLE(x) +| STATS min = MIN(agg_metric) WHERE x >= 3 +; + +min:double +3.0 +; + +countWithConditions +required_capability: stats_with_filtered_surrogate_fixed +required_capability: aggregate_metric_double_convert_to + +ROW x = [1, 2, 3, 4, 5] +| MV_EXPAND x +| EVAL agg_metric = TO_AGGREGATE_METRIC_DOUBLE(x) +| STATS count1 = COUNT(x) WHERE x >= 3, + count2 = COUNT(agg_metric) WHERE x >=3, + count3 = COUNT(4) WHERE x >= 3, + count4 = COUNT(*) WHERE x >= 3, + count5 = COUNT([1,2,3]) WHERE x >= 3 +; + +count1:long | count2:long | count3:long | count4:long | count5:long +3 | 3 | 3 | 3 | 9 +; + +countDistinctWithConditions +required_capability: stats_with_filtered_surrogate_fixed + +ROW x = [1, 2, 3, 4, 5] +| MV_EXPAND x +| EVAL agg_metric = TO_AGGREGATE_METRIC_DOUBLE(x) +| STATS count1 = COUNT_DISTINCT(x) WHERE x <= 3, + count2 = COUNT_DISTINCT(1) WHERE x <= 3 +; + +count1:long | count2:long +3 | 1 +; + +avgWithConditions +required_capability: stats_with_filtered_surrogate_fixed +required_capability: aggregate_metric_double_convert_to + +ROW x = [1, 2, 3, 4, 5] +| MV_EXPAND x +| EVAL agg_metric = TO_AGGREGATE_METRIC_DOUBLE(x) +| STATS avg1 = AVG(x) WHERE x <= 3, + avg2 = AVG(agg_metric) WHERE x <=3 +; + +avg1:double | avg2:double +2.0 | 2.0 +; + +percentileWithConditions +required_capability: stats_with_filtered_surrogate_fixed + +ROW x = [1, 2, 3, 4, 5] +| MV_EXPAND x +| STATS percentile1 = PERCENTILE(x, 100) WHERE x <= 3, + percentile2 = PERCENTILE(x, 100) +; + +percentile1:double | percentile2:double +3.0 | 5.0 +; + +medianWithConditions +required_capability: stats_with_filtered_surrogate_fixed + +ROW x = [1, 2, 3, 4, 5] +| MV_EXPAND x +| STATS median1 = MEDIAN(x) WHERE x <= 3, + median2 = MEDIAN(x), + median3 = MEDIAN([5,6,7,8,9]) WHERE x <= 3 +; + +median1:double | median2:double | median3:double +2.0 | 3.0 | 7.0 +; + +medianAbsoluteDeviationWithConditions +required_capability: stats_with_filtered_surrogate_fixed + +ROW x = [1, 2, 3, 4, 5] +| MV_EXPAND x +| STATS median_deviation1 = MEDIAN_ABSOLUTE_DEVIATION(x) WHERE x <= 3, + median_deviation2 = MEDIAN_ABSOLUTE_DEVIATION(x), + median_deviation3 = MEDIAN_ABSOLUTE_DEVIATION([5,6,7,8,9]) WHERE x <= 3 +; + +median_deviation1:double | median_deviation2:double | median_deviation3:double +1.0 | 1.0 | 1.0 +; + +topWithConditions +required_capability: stats_with_filtered_surrogate_fixed + +FROM employees +| STATS min1 = TOP(emp_no, 1, "ASC") WHERE emp_no > 10010, + min2 = TOP(emp_no, 2, "ASC") WHERE emp_no > 10010, + max1 = TOP(emp_no, 1, "DESC") WHERE emp_no < 10080, + max2 = TOP(emp_no, 2, "DESC") WHERE emp_no < 10080 +; + +min1:integer | min2:integer | max1:integer | max2:integer +10011 | [10011, 10012] | 10079 | [10079, 10078] +; diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/stats_top.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/stats_top.csv-spec index c199ac97b4c4e..2165ee42419c2 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/stats_top.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/stats_top.csv-spec @@ -300,17 +300,3 @@ FROM books The Lord of the Rings Poster Collection: Six Paintings by Alan Lee (No. 1) | he Lo | [J. R. R. Tolkien, Alan Lee] | Alan Lee A Gentle Creature and Other Stories: White Nights, A Gentle Creature, and The Dream of a Ridiculous Man (The World's Classics) | Gent | [W. J. Leatherbarrow, Fyodor Dostoevsky, Alan Myers] | Alan Myers ; - -topWithConditions -required_capability: stats_top_1_with_condition_fixed - -FROM employees -| STATS min1 = TOP(emp_no, 1, "ASC") WHERE emp_no > 10010, - min2 = TOP(emp_no, 2, "ASC") WHERE emp_no > 10010, - max1 = TOP(emp_no, 1, "DESC") WHERE emp_no < 10080, - max2 = TOP(emp_no, 2, "DESC") WHERE emp_no < 10080 -; - -min1:integer | min2:integer | max1:integer | max2:integer -10011 | [10011, 10012] | 10079 | [10079, 10078] -; diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java index 38b462329cb2f..4be5030b29dcb 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java @@ -1465,9 +1465,11 @@ public enum Cap { FN_PRESENT, /** - * Bugfix for STATS TOP(field, 1, order) WHERE condition. + * Bugfix for STATS {{expression}} WHERE {{condition}} when the expression + * is replaced by something else on planning + * e.g. STATS SUM(1) WHERE x==3 is replaced by MV_SUM(const)*COUNT(* WHERE x == 3). */ - STATS_TOP_1_WITH_CONDITION_FIXED; + STATS_WITH_FILTERED_SURROGATE_FIXED; private final boolean enabled; diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Count.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Count.java index cbf281f1bc86b..855de3090be9c 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Count.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Count.java @@ -152,7 +152,7 @@ public Expression surrogate() { var s = source(); var field = field(); if (field.dataType() == DataType.AGGREGATE_METRIC_DOUBLE) { - return new Sum(s, FromAggregateMetricDouble.withMetric(source(), field, AggregateMetricDoubleBlockBuilder.Metric.COUNT)); + return new Sum(s, FromAggregateMetricDouble.withMetric(source(), field, AggregateMetricDoubleBlockBuilder.Metric.COUNT), filter()); } if (field.foldable()) { @@ -169,7 +169,7 @@ public Expression surrogate() { return new Mul( s, new Coalesce(s, new MvCount(s, field), List.of(new Literal(s, 0, DataType.INTEGER))), - new Count(s, Literal.keyword(s, StringUtils.WILDCARD)) + new Count(s, Literal.keyword(s, StringUtils.WILDCARD), filter()) ); } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Max.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Max.java index ec8d54c965f62..649cfdb70457d 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Max.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Max.java @@ -160,7 +160,7 @@ public final AggregatorFunctionSupplier supplier() { @Override public Expression surrogate() { if (field().dataType() == DataType.AGGREGATE_METRIC_DOUBLE) { - return new Max(source(), FromAggregateMetricDouble.withMetric(source(), field(), AggregateMetricDoubleBlockBuilder.Metric.MAX)); + return new Max(source(), FromAggregateMetricDouble.withMetric(source(), field(), AggregateMetricDoubleBlockBuilder.Metric.MAX), filter()); } return field().foldable() ? new MvMax(source(), field()) : null; } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Median.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Median.java index 6ed4019cfeb97..cd8b244a3d81e 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Median.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Median.java @@ -117,6 +117,6 @@ public Expression surrogate() { return field.foldable() ? new MvMedian(s, new ToDouble(s, field)) - : new Percentile(source(), field(), new Literal(source(), (int) QuantileStates.MEDIAN, DataType.INTEGER)); + : new Percentile(source(), field(), filter(), new Literal(source(), (int) QuantileStates.MEDIAN, DataType.INTEGER)); } } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Min.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Min.java index 269911d391dc5..7c70baf0f4536 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Min.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Min.java @@ -160,7 +160,7 @@ public final AggregatorFunctionSupplier supplier() { @Override public Expression surrogate() { if (field().dataType() == DataType.AGGREGATE_METRIC_DOUBLE) { - return new Min(source(), FromAggregateMetricDouble.withMetric(source(), field(), AggregateMetricDoubleBlockBuilder.Metric.MIN)); + return new Min(source(), FromAggregateMetricDouble.withMetric(source(), field(), AggregateMetricDoubleBlockBuilder.Metric.MIN), filter()); } return field().foldable() ? new MvMin(source(), field()) : null; } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Sum.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Sum.java index 2877682f3749c..39541e5851166 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Sum.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Sum.java @@ -66,6 +66,10 @@ public Sum(Source source, @Param(name = "number", type = { "aggregate_metric_dou this(source, field, Literal.TRUE, SummationMode.COMPENSATED_LITERAL); } + public Sum(Source source, @Param(name = "number", type = { "aggregate_metric_double", "double", "integer", "long" }) Expression field, Expression filter) { + this(source, field, filter, SummationMode.COMPENSATED_LITERAL); + } + public Sum(Source source, Expression field, Expression filter, Expression summationMode) { super(source, field, filter, List.of(summationMode)); this.summationMode = summationMode; @@ -163,6 +167,6 @@ public Expression surrogate() { } // SUM(const) is equivalent to MV_SUM(const)*COUNT(*). - return field.foldable() ? new Mul(s, new MvSum(s, field), new Count(s, Literal.keyword(s, StringUtils.WILDCARD))) : null; + return field.foldable() ? new Mul(s, new MvSum(s, field), new Count(s, Literal.keyword(s, StringUtils.WILDCARD), filter())) : null; } } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/WeightedAvg.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/WeightedAvg.java index c58bc997527b0..9966e87dc5b5b 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/WeightedAvg.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/WeightedAvg.java @@ -160,9 +160,9 @@ public Expression surrogate() { return new MvAvg(s, field); } if (weight.foldable()) { - return new Div(s, new Sum(s, field), new Count(s, field), dataType()); + return new Div(s, new Sum(s, field, filter()), new Count(s, field, filter()), dataType()); } else { - return new Div(s, new Sum(s, new Mul(s, field, weight)), new Sum(s, weight), dataType()); + return new Div(s, new Sum(s, new Mul(s, field, weight), filter()), new Sum(s, weight, filter()), dataType()); } } From 55483f84b08ce37e14c1dd5777955b40f7cc6543 Mon Sep 17 00:00:00 2001 From: ncordon Date: Wed, 10 Sep 2025 17:17:16 +0200 Subject: [PATCH 05/11] Cleans up --- docs/changelog/134376.yaml | 6 ------ 1 file changed, 6 deletions(-) delete mode 100644 docs/changelog/134376.yaml diff --git a/docs/changelog/134376.yaml b/docs/changelog/134376.yaml deleted file mode 100644 index b17ab5953b3b0..0000000000000 --- a/docs/changelog/134376.yaml +++ /dev/null @@ -1,6 +0,0 @@ -pr: 134376 -summary: "Fix ES|QL TOP 1 aggregation with condition" -area: ES|QL -type: bug -issues: - - 134293 From 27dd3e0b3f56331e3a7e9f1bac14480aef97a14a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nacho=20Cord=C3=B3n?= Date: Wed, 10 Sep 2025 17:27:02 +0200 Subject: [PATCH 06/11] Update docs/changelog/134461.yaml --- docs/changelog/134461.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 docs/changelog/134461.yaml diff --git a/docs/changelog/134461.yaml b/docs/changelog/134461.yaml new file mode 100644 index 0000000000000..95814a6ee1aba --- /dev/null +++ b/docs/changelog/134461.yaml @@ -0,0 +1,6 @@ +pr: 134461 +summary: Propagates filter() to aggregation functions' surrogates +area: Compute Engine +type: bug +issues: + - 134380 From 92c07ddf64b59eb2d6dbb183d9c6045db8d9ec84 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Wed, 10 Sep 2025 15:40:11 +0000 Subject: [PATCH 07/11] [CI] Auto commit changes from spotless --- .../xpack/esql/expression/function/aggregate/Count.java | 6 +++++- .../xpack/esql/expression/function/aggregate/Max.java | 6 +++++- .../xpack/esql/expression/function/aggregate/Min.java | 6 +++++- .../xpack/esql/expression/function/aggregate/Sum.java | 6 +++++- 4 files changed, 20 insertions(+), 4 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Count.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Count.java index 855de3090be9c..15810e151e623 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Count.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Count.java @@ -152,7 +152,11 @@ public Expression surrogate() { var s = source(); var field = field(); if (field.dataType() == DataType.AGGREGATE_METRIC_DOUBLE) { - return new Sum(s, FromAggregateMetricDouble.withMetric(source(), field, AggregateMetricDoubleBlockBuilder.Metric.COUNT), filter()); + return new Sum( + s, + FromAggregateMetricDouble.withMetric(source(), field, AggregateMetricDoubleBlockBuilder.Metric.COUNT), + filter() + ); } if (field.foldable()) { diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Max.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Max.java index 649cfdb70457d..932ee08bdbed6 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Max.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Max.java @@ -160,7 +160,11 @@ public final AggregatorFunctionSupplier supplier() { @Override public Expression surrogate() { if (field().dataType() == DataType.AGGREGATE_METRIC_DOUBLE) { - return new Max(source(), FromAggregateMetricDouble.withMetric(source(), field(), AggregateMetricDoubleBlockBuilder.Metric.MAX), filter()); + return new Max( + source(), + FromAggregateMetricDouble.withMetric(source(), field(), AggregateMetricDoubleBlockBuilder.Metric.MAX), + filter() + ); } return field().foldable() ? new MvMax(source(), field()) : null; } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Min.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Min.java index 7c70baf0f4536..7ea6241f896c0 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Min.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Min.java @@ -160,7 +160,11 @@ public final AggregatorFunctionSupplier supplier() { @Override public Expression surrogate() { if (field().dataType() == DataType.AGGREGATE_METRIC_DOUBLE) { - return new Min(source(), FromAggregateMetricDouble.withMetric(source(), field(), AggregateMetricDoubleBlockBuilder.Metric.MIN), filter()); + return new Min( + source(), + FromAggregateMetricDouble.withMetric(source(), field(), AggregateMetricDoubleBlockBuilder.Metric.MIN), + filter() + ); } return field().foldable() ? new MvMin(source(), field()) : null; } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Sum.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Sum.java index 39541e5851166..bf78b0c5f8fdf 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Sum.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Sum.java @@ -66,7 +66,11 @@ public Sum(Source source, @Param(name = "number", type = { "aggregate_metric_dou this(source, field, Literal.TRUE, SummationMode.COMPENSATED_LITERAL); } - public Sum(Source source, @Param(name = "number", type = { "aggregate_metric_double", "double", "integer", "long" }) Expression field, Expression filter) { + public Sum( + Source source, + @Param(name = "number", type = { "aggregate_metric_double", "double", "integer", "long" }) Expression field, + Expression filter + ) { this(source, field, filter, SummationMode.COMPENSATED_LITERAL); } From d591ab75da811cb3a4c44286cc4bb20c5cb14fb0 Mon Sep 17 00:00:00 2001 From: ncordon Date: Wed, 10 Sep 2025 18:05:50 +0200 Subject: [PATCH 08/11] Cleans up --- docs/changelog/134461.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changelog/134461.yaml b/docs/changelog/134461.yaml index 95814a6ee1aba..0086e5e0a5050 100644 --- a/docs/changelog/134461.yaml +++ b/docs/changelog/134461.yaml @@ -1,6 +1,6 @@ pr: 134461 summary: Propagates filter() to aggregation functions' surrogates -area: Compute Engine +area: Aggregations type: bug issues: - 134380 From c7715facc9688f813eb570a10db737123b24f942 Mon Sep 17 00:00:00 2001 From: ncordon Date: Thu, 11 Sep 2025 09:43:48 +0200 Subject: [PATCH 09/11] Adds extra tests --- .../src/main/resources/stats.csv-spec | 91 ++++++++++++------- .../xpack/esql/action/EsqlCapabilities.java | 7 +- 2 files changed, 60 insertions(+), 38 deletions(-) diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/stats.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/stats.csv-spec index 50dc51217df82..a3616911f5f4e 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/stats.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/stats.csv-spec @@ -3148,16 +3148,20 @@ m:datetime | x:integer | d:boolean sumWithConditions required_capability: stats_with_filtered_surrogate_fixed +required_capability: aggregate_metric_double_convert_to FROM employees +| EVAL agg_metric = TO_AGGREGATE_METRIC_DOUBLE(1) | STATS sum1 = SUM(1), sum2 = SUM(1) WHERE emp_no == 10080, sum3 = SUM(1) WHERE emp_no < 10080, - sum4 = SUM(1) WHERE emp_no >= 10080 + sum4 = SUM(1) WHERE emp_no >= 10080, + sum5 = SUM(agg_metric), + sum6 = SUM(agg_metric) WHERE emp_no == 10080 ; -sum1:long | sum2:long | sum3:long | sum4:long -100 | 1 | 79 | 21 +sum1:long | sum2:long | sum3:long | sum4:long | sum5:double | sum6:double +100 | 1 | 79 | 21 | 100.0 | 1.0 ; weightedAvgWithConditions @@ -3168,11 +3172,13 @@ ROW x = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10] | STATS w_avg1 = WEIGHTED_AVG(x, 1) WHERE x == 5, w_avg2 = WEIGHTED_AVG(x, x) WHERE x == 5, w_avg3 = WEIGHTED_AVG(x, 2) WHERE x <= 5, - w_avg4 = WEIGHTED_AVG(x, x) WHERE x > 5 + w_avg4 = WEIGHTED_AVG(x, x) WHERE x > 5, + w_avg5 = WEIGHTED_AVG([1,2,3], 1), + w_avg6 = WEIGHTED_AVG([1,2,3], 1) WHERE x == 5 ; -w_avg1:double | w_avg2:double | w_avg3:double | w_avg4:double -5.0 | 5.0 | 3.0 | 8.25 +w_avg1:double | w_avg2:double | w_avg3:double | w_avg4:double | w_avg5:double | w_avg6:double +5.0 | 5.0 | 3.0 | 8.25 | 2.0 | 2.0 ; maxWithConditions @@ -3182,11 +3188,14 @@ required_capability: aggregate_metric_double_convert_to ROW x = [1, 2, 3, 4, 5] | MV_EXPAND x | EVAL agg_metric = TO_AGGREGATE_METRIC_DOUBLE(x) -| STATS max = MAX(agg_metric) WHERE x <= 3 +| STATS max1 = MAX(agg_metric) WHERE x <= 3, + max2 = MAX(agg_metric), + max3 = MAX(x), + max4 = MAX(x) WHERE x > 3 ; -max:double -3.0 +max1:double | max2:double | max3:integer | max4:integer +3.0 | 5.0 | 5 | 5 ; minWithConditions @@ -3196,11 +3205,14 @@ required_capability: aggregate_metric_double_convert_to ROW x = [1, 2, 3, 4, 5] | MV_EXPAND x | EVAL agg_metric = TO_AGGREGATE_METRIC_DOUBLE(x) -| STATS min = MIN(agg_metric) WHERE x >= 3 +| STATS min1 = MIN(agg_metric) WHERE x <= 3, + min2 = MIN(agg_metric), + min3 = MIN(x), + min4 = MIN(x) WHERE x > 3 ; -min:double -3.0 +min1:double | min2:double | min3:integer | min4:integer +1.0 | 1.0 | 1 | 4 ; countWithConditions @@ -3211,14 +3223,17 @@ ROW x = [1, 2, 3, 4, 5] | MV_EXPAND x | EVAL agg_metric = TO_AGGREGATE_METRIC_DOUBLE(x) | STATS count1 = COUNT(x) WHERE x >= 3, - count2 = COUNT(agg_metric) WHERE x >=3, - count3 = COUNT(4) WHERE x >= 3, - count4 = COUNT(*) WHERE x >= 3, - count5 = COUNT([1,2,3]) WHERE x >= 3 + count2 = COUNT(x), + count3 = COUNT(agg_metric), + count4 = COUNT(agg_metric) WHERE x >=3, + count5 = COUNT(4) WHERE x >= 3, + count6 = COUNT(*) WHERE x >= 3, + count7 = COUNT([1,2,3]) WHERE x >= 3, + count8 = COUNT([1,2,3]) ; -count1:long | count2:long | count3:long | count4:long | count5:long -3 | 3 | 3 | 3 | 9 +count1:long | count2:long | count3:long | count4:long | count5:long | count6:long | count7:long | count8:long +3 | 5 | 5 | 3 | 3 | 3 | 9 | 15 ; countDistinctWithConditions @@ -3228,11 +3243,13 @@ ROW x = [1, 2, 3, 4, 5] | MV_EXPAND x | EVAL agg_metric = TO_AGGREGATE_METRIC_DOUBLE(x) | STATS count1 = COUNT_DISTINCT(x) WHERE x <= 3, - count2 = COUNT_DISTINCT(1) WHERE x <= 3 + count2 = COUNT_DISTINCT(x), + count3 = COUNT_DISTINCT(1) WHERE x <= 3, + count4 = COUNT_DISTINCT(1) ; -count1:long | count2:long -3 | 1 +count1:long | count2:long | count3:long | count4:long +3 | 5 | 1 | 1 ; avgWithConditions @@ -3243,11 +3260,13 @@ ROW x = [1, 2, 3, 4, 5] | MV_EXPAND x | EVAL agg_metric = TO_AGGREGATE_METRIC_DOUBLE(x) | STATS avg1 = AVG(x) WHERE x <= 3, - avg2 = AVG(agg_metric) WHERE x <=3 + avg2 = AVG(x), + avg3 = AVG(agg_metric) WHERE x <=3, + avg4 = AVG(agg_metric) ; -avg1:double | avg2:double -2.0 | 2.0 +avg1:double | avg2:double | avg3:double | avg4:double +2.0 | 3.0 | 2.0 | 3.0 ; percentileWithConditions @@ -3255,12 +3274,12 @@ required_capability: stats_with_filtered_surrogate_fixed ROW x = [1, 2, 3, 4, 5] | MV_EXPAND x -| STATS percentile1 = PERCENTILE(x, 100) WHERE x <= 3, - percentile2 = PERCENTILE(x, 100) +| STATS percentile1 = PERCENTILE(x, 50) WHERE x <= 3, + percentile2 = PERCENTILE(x, 50) ; percentile1:double | percentile2:double -3.0 | 5.0 +2.0 | 3.0 ; medianWithConditions @@ -3270,11 +3289,12 @@ ROW x = [1, 2, 3, 4, 5] | MV_EXPAND x | STATS median1 = MEDIAN(x) WHERE x <= 3, median2 = MEDIAN(x), - median3 = MEDIAN([5,6,7,8,9]) WHERE x <= 3 + median3 = MEDIAN([5,6,7,8,9]) WHERE x <= 3, + median4 = MEDIAN([5,6,7,8,9]) ; -median1:double | median2:double | median3:double -2.0 | 3.0 | 7.0 +median1:double | median2:double | median3:double | median4:double +2.0 | 3.0 | 7.0 | 7.0 ; medianAbsoluteDeviationWithConditions @@ -3282,13 +3302,14 @@ required_capability: stats_with_filtered_surrogate_fixed ROW x = [1, 2, 3, 4, 5] | MV_EXPAND x -| STATS median_deviation1 = MEDIAN_ABSOLUTE_DEVIATION(x) WHERE x <= 3, - median_deviation2 = MEDIAN_ABSOLUTE_DEVIATION(x), - median_deviation3 = MEDIAN_ABSOLUTE_DEVIATION([5,6,7,8,9]) WHERE x <= 3 +| STATS median_dev1 = MEDIAN_ABSOLUTE_DEVIATION(x) WHERE x <= 3, + median_dev2 = MEDIAN_ABSOLUTE_DEVIATION(x), + median_dev3 = MEDIAN_ABSOLUTE_DEVIATION([5,6,7,8,9]) WHERE x <= 3, + median_dev4 = MEDIAN_ABSOLUTE_DEVIATION([5,6,7,8,9]) ; -median_deviation1:double | median_deviation2:double | median_deviation3:double -1.0 | 1.0 | 1.0 +median_dev1:double | median_dev2:double | median_dev3:double | median_dev4:double +1.0 | 1.0 | 1.0 | 1.0 ; topWithConditions diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java index d5cff4df1e6f8..3b4c676878e2b 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java @@ -1470,9 +1470,10 @@ public enum Cap { FN_PRESENT, /** - * Bugfix for STATS {{expression}} WHERE {{condition}} when the expression - * is replaced by something else on planning - * e.g. STATS SUM(1) WHERE x==3 is replaced by MV_SUM(const)*COUNT(* WHERE x == 3). + * Bugfix for STATS {{expression}} WHERE {{condition}} when the + * expression is replaced by something else on planning + * e.g. STATS SUM(1) WHERE x==3 is replaced by + * MV_SUM(const)*COUNT(* WHERE x == 3). */ STATS_WITH_FILTERED_SURROGATE_FIXED, From 2ac7209588229d583ada4ba0c0b2ea30c3419e15 Mon Sep 17 00:00:00 2001 From: ncordon Date: Thu, 11 Sep 2025 14:36:39 +0200 Subject: [PATCH 10/11] Adds automatic test to check filter is not lost Thanks to @ivancea --- .../function/AbstractAggregationTestCase.java | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/AbstractAggregationTestCase.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/AbstractAggregationTestCase.java index 5bc8097c526b6..3196aae2dd5e7 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/AbstractAggregationTestCase.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/AbstractAggregationTestCase.java @@ -181,6 +181,31 @@ public void testFold() { }, this::evaluate); } + public void testSurrogateHasFilter() { + Expression expression = randomFrom( + buildLiteralExpression(testCase), + buildDeepCopyOfFieldExpression(testCase), + buildFieldExpression(testCase) + ); + + assumeTrue("expression should have no type errors", expression.typeResolved().resolved()); + + if (expression instanceof AggregateFunction && expression instanceof SurrogateExpression) { + var filter = ((AggregateFunction) expression).filter(); + + if (filter != null) { + var surrogate = ((SurrogateExpression) expression).surrogate(); + + if (surrogate != null) { + surrogate.forEachDown(AggregateFunction.class, child -> { + var surrogateFilter = child.filter(); + assertEquals(filter, surrogateFilter); + }); + } + } + } + } + private void aggregateSingleMode(Expression expression) { Object result; try (var aggregator = aggregator(expression, initialInputChannels(), AggregatorMode.SINGLE)) { From e9ec2db77bee45a6531b5ca3bd80e54881953b8c Mon Sep 17 00:00:00 2001 From: ncordon Date: Thu, 11 Sep 2025 16:45:46 +0200 Subject: [PATCH 11/11] Addresses pr feedback --- .../src/main/resources/stats.csv-spec | 8 ++++---- .../xpack/esql/action/EsqlCapabilities.java | 2 +- .../function/AbstractAggregationTestCase.java | 16 +++++++--------- 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/stats.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/stats.csv-spec index a3616911f5f4e..a8005a42f73bd 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/stats.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/stats.csv-spec @@ -3300,16 +3300,16 @@ median1:double | median2:double | median3:double | median4:double medianAbsoluteDeviationWithConditions required_capability: stats_with_filtered_surrogate_fixed -ROW x = [1, 2, 3, 4, 5] +ROW x = [1, 3, 4, 7, 11, 18] | MV_EXPAND x | STATS median_dev1 = MEDIAN_ABSOLUTE_DEVIATION(x) WHERE x <= 3, median_dev2 = MEDIAN_ABSOLUTE_DEVIATION(x), - median_dev3 = MEDIAN_ABSOLUTE_DEVIATION([5,6,7,8,9]) WHERE x <= 3, - median_dev4 = MEDIAN_ABSOLUTE_DEVIATION([5,6,7,8,9]) + median_dev3 = MEDIAN_ABSOLUTE_DEVIATION([3, 11, 14, 25]) WHERE x <= 3, + median_dev4 = MEDIAN_ABSOLUTE_DEVIATION([3, 11, 14, 25]) ; median_dev1:double | median_dev2:double | median_dev3:double | median_dev4:double -1.0 | 1.0 | 1.0 | 1.0 +1.0 | 3.5 | 5.5 | 5.5 ; topWithConditions diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java index 01a7fc71f5cdb..067056d1fffa0 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java @@ -1473,7 +1473,7 @@ public enum Cap { * Bugfix for STATS {{expression}} WHERE {{condition}} when the * expression is replaced by something else on planning * e.g. STATS SUM(1) WHERE x==3 is replaced by - * MV_SUM(const)*COUNT(* WHERE x == 3). + * STATS MV_SUM(const)*COUNT(*) WHERE x == 3. */ STATS_WITH_FILTERED_SURROGATE_FIXED, diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/AbstractAggregationTestCase.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/AbstractAggregationTestCase.java index 3196aae2dd5e7..4b799c4172440 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/AbstractAggregationTestCase.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/AbstractAggregationTestCase.java @@ -193,15 +193,13 @@ public void testSurrogateHasFilter() { if (expression instanceof AggregateFunction && expression instanceof SurrogateExpression) { var filter = ((AggregateFunction) expression).filter(); - if (filter != null) { - var surrogate = ((SurrogateExpression) expression).surrogate(); - - if (surrogate != null) { - surrogate.forEachDown(AggregateFunction.class, child -> { - var surrogateFilter = child.filter(); - assertEquals(filter, surrogateFilter); - }); - } + var surrogate = ((SurrogateExpression) expression).surrogate(); + + if (surrogate != null) { + surrogate.forEachDown(AggregateFunction.class, child -> { + var surrogateFilter = child.filter(); + assertEquals(filter, surrogateFilter); + }); } } }