Add support for top-level arithmetic ops to TS|STATS

[felixbarny]

This is what's happening at a high level:

• TranslateTimeSeriesAggregate now not only handles AggregateFunctions, but all Functions, including BinaryScalarFunctions
• Going into TranslateTimeSeriesAggregate, the aggregates are not be split up into evals anymore. The TranslateTimeSeriesAggregate rule now runs earlier in the optimizer (before ReplaceAggregateNestedExpressionWithEval and friends).
  • Enables adding all TimeSeriesAggregateFunctions to the first aggregation phase, without some TimeSeriesAggregateFunctions being placed in nested Evals.
  • Also ensures sure we can properly insert the default last_over_time function for expressions like foo + 1 or max(foo + 1), before the inner foo + 1 is extracted into an eval.
• Nested expressions in the groupings of the TimeSeriesAggregate are still be replaced with an eval to make time bucket handling easier.
• Extracts the injection of the default last_over_time function outside of TranslateTimeSeriesAggregate and into the analysis phase, so that InsertFromAggregateMetricDouble runs after the insertion of last_over_time. If it would execute later, the last_over_time function can't be resolved for downsampling indices where metrics are of type aggregate_metric_double. It needs to run after field resolution as the injection of the default inner agg is type-dependent - we have a different strategy for histograms.
• TimeSeriesGroupByAll has been moved from the initialize to the resolution phase of the analyzer - after InsertDefaultInnerTimeSeriesAggregate, so that it can take that into account. That also fixes a missing reference issue in the nested eval for queries like network.total_bytes_in * 8.

Queries that are supported now but weren't before:

• Bare metric (with group-by-all)
  • TS k8s | STATS network.cost
• Group by all now supports post processing
  • TS k8s | STATS network.cost | SORT network.cost
  • Previously, there was a bug that complained about missing references as the id of the alias changed
• Top-level arithmetic operations between metric and scalar
  • TS k8s | STATS 10 + max(10 + network.total_bytes_in)
  • Also supports implicit last_over_time and group-by-all
  • TS k8s | STATS network.total_bytes_in * 8
• Top-level arithmetic operations between metric and metric
  • Also supports implicit last_over_time and group-by-all
  • TS k8s | STATS in_n_out=network.eth0.rx + network.eth0.tx
  • TS k8s | STATS max(last_over_time(network.eth0.tx::double) / (last_over_time(network.eth0.tx::double) + last_over_time(network.eth0.rx::double)))

closes #139570, #138702, #139580

Child PRs

• Resolve all references in PromqlCommand #140130
• Apply group by all logic not only to top-level aggregates #140248
• Make TimeSeriesAggregate TimestampAware #140270
• Add rule to wrap bare metrics in default over time function #140090

PromQL support will be added in a follow-up:

• PromQL: support for top-level binary operators #140541

[pabloem]

this is awesome. Thanks for tackling it @felixbarny

[dnhatn]

@felixbarny Thank you for tackling this. I've spent quite some time on this. I think TranslateTimeSeriesAggregate should be a rule in the Analyzer, not an optimizer rule. As you found, it should execute before we substitute expressions around aggregations. However, I think the approach you proposed can be fragile, since we might scatter the substitutions for aggregations and groupings. I played with these rules and I think we can move TranslateTimeSeriesAggregate to the Analyzer. Are you okay to continue working this issue? Otherwise, we can share the work.

[felixbarny]

I think TranslateTimeSeriesAggregate should be a rule in the Analyzer, not an optimizer rule.

I guess this implies that TranslatePromqlToTimeSeriesAggregate will also need to be executed during analysis so that it can run before TranslateTimeSeriesAggregate.

When I asked @costin why we don't run the PromQL translation during analysis, this was his response:

1. Preservation of Query Integrity: The optimization process must assume a valid input query and should not modify the abstract syntax tree (AST). This is critical to ensure that any validation failures are reported directly and accurately to the user, without obfuscation from query transformation.
2. Node Self-Sufficiency and Output Alignment: Each node within the query structure must fully and explicitly describe its own output. The existing discrepancy between the output reported by the PromQL tree and its actual translation needs to be resolved. This resolution should be handled during the tree assembly phase.

However, I think the approach you proposed can be fragile, since we might scatter the substitutions for aggregations and groupings.

Fair. Re-using ReplaceAggregateNestedExpressionWithEval but just for groupings was an attempt to simplify and reduce the surface area of the change. But I can try to do without it. I guess it'll still imply creating a nested eval for the time bucket so that both the first and the second pass groupings can re-use it. This will be very similar to what ReplaceAggregateNestedExpressionWithEval is doing, but isolated to just time bucket groupings. Is that what you had in mind?

What do you think of phasing out the change where in the first step, the PromQL and time series aggregate translation is still happening in the optimization phase, but runs early and also doesn't use ReplaceAggregateNestedExpressionWithEval internally? We can then follow-up and separately discuss whether to move it to the analysis phase, which should then also be simpler and incremental change. I'm happy to keep working on it.

[dnhatn]

What do you think of phasing out the change where in the first step, the PromQL and time series aggregate translation is still happening in the optimization phase, but runs early and also doesn't use ReplaceAggregateNestedExpressionWithEval internally? We can then follow-up and separately discuss whether to move it to the analysis phase, which should then also be simpler and incremental change. I'm happy to keep working on it.

++ Moving the PromQL and translate-time-series rules to the beginning of the logical optimizer rules is a good step toward moving them to the analysis phase.

[elasticsearchmachine]

Hi @felixbarny, I've created a changelog YAML for you.

[elasticsearchmachine]

Pinging @elastic/es-storage-engine (Team:StorageEngine)

[kkrik-es]

I like the direction, and the logic seems a bit cleaner. Will leave it to Nhat to approve.

[sidosera]

This looks great! I also love we move PromQL translation earlier in the chain.

I'm happy to accept to unblock, would still love to hear Nhat take when they get a chance.

[dnhatn]

One question, but this looks great. Thanks Felix for all the iterations.

[dnhatn]

I don't think we need to call SubstituteSurrogateAggregations twice consecutively.

[felixbarny]

This is how I had it initially but omitting it caused bwc tests to fail. Something related to class cast exceptions of different block instances.
Maybe we can look for a solution after merging this PR?

[dnhatn]

That works.