From e46bb36684605c0f6fdc348e482d92166d167208 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Wed, 26 Nov 2025 11:47:54 +0100 Subject: [PATCH 1/4] Add ToDouble conversion and projection for AcrossSeriesAggregate output --- .../TranslatePromqlToTimeSeriesAggregate.java | 35 +++++++++++++++---- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/promql/TranslatePromqlToTimeSeriesAggregate.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/promql/TranslatePromqlToTimeSeriesAggregate.java index 164eaf2831e72..182d4f5979241 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/promql/TranslatePromqlToTimeSeriesAggregate.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/promql/TranslatePromqlToTimeSeriesAggregate.java @@ -38,6 +38,7 @@ import org.elasticsearch.xpack.esql.plan.logical.Eval; import org.elasticsearch.xpack.esql.plan.logical.Filter; import org.elasticsearch.xpack.esql.plan.logical.LogicalPlan; +import org.elasticsearch.xpack.esql.plan.logical.Project; import org.elasticsearch.xpack.esql.plan.logical.TimeSeriesAggregate; import org.elasticsearch.xpack.esql.plan.logical.promql.AcrossSeriesAggregate; import org.elasticsearch.xpack.esql.plan.logical.promql.PlaceholderRelation; @@ -181,11 +182,31 @@ private static MapResult mapFunction(PromqlCommand promqlCommand, PromqlFunction List aggs = new ArrayList<>(); List groupings = new ArrayList<>(acrossAggregate.groupings().size()); Alias stepBucket = createStepBucketAlias(promqlCommand, acrossAggregate); - initAggregatesAndGroupings(acrossAggregate, target, aggs, groupings, stepBucket.toAttribute()); + Attribute valueAttribute = initAggregatesAndGroupings(acrossAggregate, target, aggs, groupings, stepBucket.toAttribute()); LogicalPlan p = childResult.plan; p = new Eval(stepBucket.source(), p, List.of(stepBucket)); p = new TimeSeriesAggregate(acrossAggregate.source(), p, groupings, aggs, null); + + // ToDouble conversion of the metric using an eval to ensure a consistent output type + Alias convertedValue = new Alias( + acrossAggregate.source(), + valueAttribute.name(), + new ToDouble(acrossAggregate.source(), valueAttribute), + valueAttribute.id() + ); + p = new Eval(acrossAggregate.source(), p, List.of(convertedValue)); + + // Project to maintain the correct output order, as declared in AcrossSeriesAggregate#output: + // [value, step, ...groupings] + List projections = new ArrayList<>(); + projections.add(convertedValue.toAttribute()); // converted value first + projections.add(stepBucket.toAttribute()); // step second + for (NamedExpression grouping : acrossAggregate.groupings()) { + projections.add(grouping.toAttribute()); // then groupings + } + p = new Project(acrossAggregate.source(), p, projections); + result = new MapResult(p, extras); } else { throw new QlIllegalArgumentException("Unsupported PromQL function call: {}", functionCall); @@ -194,7 +215,7 @@ private static MapResult mapFunction(PromqlCommand promqlCommand, PromqlFunction return result; } - private static void initAggregatesAndGroupings( + private static Attribute initAggregatesAndGroupings( AcrossSeriesAggregate acrossAggregate, Expression target, List aggs, @@ -205,13 +226,11 @@ private static void initAggregatesAndGroupings( Function esqlFunction = PromqlFunctionRegistry.INSTANCE.buildEsqlFunction( acrossAggregate.functionName(), acrossAggregate.source(), - // to double conversion of the metric to ensure a consistent output type - // TODO it's probably more efficient to wrap the function in the ToDouble - // but for some reason this doesn't work if you have an inner and outer aggregation - List.of(new ToDouble(target.source(), target)) + List.of(target) ); - aggs.add(new Alias(acrossAggregate.source(), acrossAggregate.sourceText(), esqlFunction, acrossAggregate.valueId())); + Alias value = new Alias(acrossAggregate.source(), acrossAggregate.sourceText(), esqlFunction, acrossAggregate.valueId()); + aggs.add(value); // timestamp/step aggs.add(stepBucket); @@ -222,6 +241,8 @@ private static void initAggregatesAndGroupings( aggs.add(grouping); groupings.add(grouping.toAttribute()); } + + return value.toAttribute(); } private static Alias createStepBucketAlias(PromqlCommand promqlCommand, AcrossSeriesAggregate acrossAggregate) { From 65d5afb00ef9bd178c9c54089afa59c42510353f Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Wed, 26 Nov 2025 18:16:21 +0100 Subject: [PATCH 2/4] Add comment for return type --- .../promql/TranslatePromqlToTimeSeriesAggregate.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/promql/TranslatePromqlToTimeSeriesAggregate.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/promql/TranslatePromqlToTimeSeriesAggregate.java index 182d4f5979241..345a8a5a014f8 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/promql/TranslatePromqlToTimeSeriesAggregate.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/promql/TranslatePromqlToTimeSeriesAggregate.java @@ -215,6 +215,16 @@ private static MapResult mapFunction(PromqlCommand promqlCommand, PromqlFunction return result; } + /** + * Initializes the aggregates and groupings for an AcrossSeriesAggregate. + * + * @param acrossAggregate the AcrossSeriesAggregate node + * @param target the target expression to aggregate + * @param aggs the list to populate with aggregate expressions + * @param groupings the list to populate with grouping expressions + * @param stepBucket the step bucket attribute + * @return the attribute representing the main aggregate value (e.g., avg(metric)) + */ private static Attribute initAggregatesAndGroupings( AcrossSeriesAggregate acrossAggregate, Expression target, From e56d62a058e3cdcea02aba5fcb3377b71daa8281 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Wed, 26 Nov 2025 20:00:22 +0100 Subject: [PATCH 3/4] Avoid duplicate name ids --- .../logical/promql/TranslatePromqlToTimeSeriesAggregate.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/promql/TranslatePromqlToTimeSeriesAggregate.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/promql/TranslatePromqlToTimeSeriesAggregate.java index 345a8a5a014f8..c24d69c4ec055 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/promql/TranslatePromqlToTimeSeriesAggregate.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/promql/TranslatePromqlToTimeSeriesAggregate.java @@ -193,7 +193,7 @@ private static MapResult mapFunction(PromqlCommand promqlCommand, PromqlFunction acrossAggregate.source(), valueAttribute.name(), new ToDouble(acrossAggregate.source(), valueAttribute), - valueAttribute.id() + acrossAggregate.valueId() ); p = new Eval(acrossAggregate.source(), p, List.of(convertedValue)); @@ -239,7 +239,7 @@ private static Attribute initAggregatesAndGroupings( List.of(target) ); - Alias value = new Alias(acrossAggregate.source(), acrossAggregate.sourceText(), esqlFunction, acrossAggregate.valueId()); + Alias value = new Alias(acrossAggregate.source(), acrossAggregate.sourceText(), esqlFunction); aggs.add(value); // timestamp/step From 47a8c8b469fc188884a0eba3134c4ab93eb46815 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Thu, 27 Nov 2025 08:29:00 +0100 Subject: [PATCH 4/4] Eval and project using TimeSeriesAggregate#output --- .../TranslatePromqlToTimeSeriesAggregate.java | 33 ++++++------------- 1 file changed, 10 insertions(+), 23 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/promql/TranslatePromqlToTimeSeriesAggregate.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/promql/TranslatePromqlToTimeSeriesAggregate.java index c24d69c4ec055..e736b5299db9a 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/promql/TranslatePromqlToTimeSeriesAggregate.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/promql/TranslatePromqlToTimeSeriesAggregate.java @@ -182,28 +182,27 @@ private static MapResult mapFunction(PromqlCommand promqlCommand, PromqlFunction List aggs = new ArrayList<>(); List groupings = new ArrayList<>(acrossAggregate.groupings().size()); Alias stepBucket = createStepBucketAlias(promqlCommand, acrossAggregate); - Attribute valueAttribute = initAggregatesAndGroupings(acrossAggregate, target, aggs, groupings, stepBucket.toAttribute()); + initAggregatesAndGroupings(acrossAggregate, target, aggs, groupings, stepBucket.toAttribute()); LogicalPlan p = childResult.plan; p = new Eval(stepBucket.source(), p, List.of(stepBucket)); - p = new TimeSeriesAggregate(acrossAggregate.source(), p, groupings, aggs, null); - + TimeSeriesAggregate tsAggregate = new TimeSeriesAggregate(acrossAggregate.source(), p, groupings, aggs, null); + p = tsAggregate; // ToDouble conversion of the metric using an eval to ensure a consistent output type Alias convertedValue = new Alias( acrossAggregate.source(), - valueAttribute.name(), - new ToDouble(acrossAggregate.source(), valueAttribute), + acrossAggregate.sourceText(), + new ToDouble(acrossAggregate.source(), p.output().getFirst().toAttribute()), acrossAggregate.valueId() ); p = new Eval(acrossAggregate.source(), p, List.of(convertedValue)); - // Project to maintain the correct output order, as declared in AcrossSeriesAggregate#output: // [value, step, ...groupings] List projections = new ArrayList<>(); - projections.add(convertedValue.toAttribute()); // converted value first - projections.add(stepBucket.toAttribute()); // step second - for (NamedExpression grouping : acrossAggregate.groupings()) { - projections.add(grouping.toAttribute()); // then groupings + projections.add(convertedValue.toAttribute()); + List output = tsAggregate.output(); + for (int i = 1; i < output.size(); i++) { + projections.add(output.get(i)); } p = new Project(acrossAggregate.source(), p, projections); @@ -215,17 +214,7 @@ private static MapResult mapFunction(PromqlCommand promqlCommand, PromqlFunction return result; } - /** - * Initializes the aggregates and groupings for an AcrossSeriesAggregate. - * - * @param acrossAggregate the AcrossSeriesAggregate node - * @param target the target expression to aggregate - * @param aggs the list to populate with aggregate expressions - * @param groupings the list to populate with grouping expressions - * @param stepBucket the step bucket attribute - * @return the attribute representing the main aggregate value (e.g., avg(metric)) - */ - private static Attribute initAggregatesAndGroupings( + private static void initAggregatesAndGroupings( AcrossSeriesAggregate acrossAggregate, Expression target, List aggs, @@ -251,8 +240,6 @@ private static Attribute initAggregatesAndGroupings( aggs.add(grouping); groupings.add(grouping.toAttribute()); } - - return value.toAttribute(); } private static Alias createStepBucketAlias(PromqlCommand promqlCommand, AcrossSeriesAggregate acrossAggregate) {