Skip to content

Commit 08690cb

Browse files
authored
Reject unsupported last_over_time during verification (#136517) (#136582)
This change updates the validation to reject time-series aggregations that cannot be executed. Specifically, if FN(field) is rewritten to FN(last_over_time(field)), and we don't support last_over_time(field), we should throw an error during planning rather than at runtime. Closes #136270 (cherry picked from commit 80b4b95)
1 parent df75d28 commit 08690cb

File tree

3 files changed

+41
-4
lines changed

3 files changed

+41
-4
lines changed

x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/generative/GenerativeRestTest.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,10 +76,9 @@ public abstract class GenerativeRestTest extends ESRestTestCase implements Query
7676
"time-series.*the first aggregation.*is not allowed",
7777
"count_star .* can't be used with TS command",
7878
"time_series aggregate.* can only be used with the TS command",
79-
"Invalid call to dataType on an unresolved object \\?LASTOVERTIME", // https://github.com/elastic/elasticsearch/issues/134791
80-
// https://github.com/elastic/elasticsearch/issues/134793
81-
"class org.elasticsearch.compute.data..*Block cannot be cast to class org.elasticsearch.compute.data..*Block",
82-
"Output has changed from \\[.*\\] to \\[.*\\]" // https://github.com/elastic/elasticsearch/issues/134794
79+
// https://github.com/elastic/elasticsearch/issues/134794
80+
"Output has changed from \\[.*\\] to \\[.*\\]",
81+
"implicit time-series aggregation function .* doesn't support type .*"
8382
);
8483

8584
public static final Set<Pattern> ALLOWED_ERROR_PATTERNS = ALLOWED_ERRORS.stream()

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/TimeSeriesAggregate.java

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,14 @@
1414
import org.elasticsearch.xpack.esql.core.expression.Alias;
1515
import org.elasticsearch.xpack.esql.core.expression.Expression;
1616
import org.elasticsearch.xpack.esql.core.expression.FieldAttribute;
17+
import org.elasticsearch.xpack.esql.core.expression.Literal;
1718
import org.elasticsearch.xpack.esql.core.expression.NamedExpression;
1819
import org.elasticsearch.xpack.esql.core.tree.NodeInfo;
1920
import org.elasticsearch.xpack.esql.core.tree.Source;
21+
import org.elasticsearch.xpack.esql.core.type.DataType;
2022
import org.elasticsearch.xpack.esql.expression.function.aggregate.AggregateFunction;
2123
import org.elasticsearch.xpack.esql.expression.function.aggregate.Count;
24+
import org.elasticsearch.xpack.esql.expression.function.aggregate.LastOverTime;
2225
import org.elasticsearch.xpack.esql.expression.function.aggregate.TimeSeriesAggregateFunction;
2326
import org.elasticsearch.xpack.esql.expression.function.grouping.Bucket;
2427
import org.elasticsearch.xpack.esql.plan.logical.join.LookupJoin;
@@ -201,6 +204,22 @@ protected void checkTimeSeriesAggregates(Failures failures) {
201204
failures.add(
202205
fail(count, "count_star [{}] can't be used with TS command; use count on a field instead", outer.sourceText())
203206
);
207+
// reject COUNT(keyword), but allow COUNT(numeric)
208+
} else if (outer instanceof TimeSeriesAggregateFunction == false && outer.field() instanceof AggregateFunction == false) {
209+
Expression field = outer.field();
210+
var lastOverTime = new LastOverTime(source(), field, new Literal(source(), null, DataType.DATETIME));
211+
if (lastOverTime.typeResolved() != Expression.TypeResolution.TYPE_RESOLVED) {
212+
failures.add(
213+
fail(
214+
this,
215+
"implicit time-series aggregation function [{}] generated from [{}] doesn't support type [{}], "
216+
+ "only numeric types are supported; use the FROM command instead of the TS command",
217+
outer.sourceText().replace(field.sourceText(), "last_over_time(" + field.sourceText() + ")"),
218+
outer.sourceText(),
219+
field.dataType().typeName()
220+
)
221+
);
222+
}
204223
}
205224
if (outer instanceof TimeSeriesAggregateFunction ts) {
206225
outer.field()

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2688,6 +2688,25 @@ public void testNoMetricInStatsByClause() {
26882688
);
26892689
}
26902690

2691+
public void testNoDimensionsInAggsOnlyInByClause() {
2692+
assertThat(
2693+
error("TS test | STATS count(host) BY bucket(@timestamp, 1 minute)", tsdb),
2694+
equalTo(
2695+
"1:11: implicit time-series aggregation function [count(last_over_time(host))] "
2696+
+ "generated from [count(host)] doesn't support type [keyword], only numeric types are supported; "
2697+
+ "use the FROM command instead of the TS command"
2698+
)
2699+
);
2700+
assertThat(
2701+
error("TS test | STATS max(name) BY bucket(@timestamp, 1 minute)", tsdb),
2702+
equalTo(
2703+
"1:11: implicit time-series aggregation function [max(last_over_time(name))] "
2704+
+ "generated from [max(name)] doesn't support type [keyword], only numeric types are supported; "
2705+
+ "use the FROM command instead of the TS command"
2706+
)
2707+
);
2708+
}
2709+
26912710
public void testSortInTimeSeries() {
26922711
assertThat(
26932712
error("TS test | SORT host | STATS avg(last_over_time(network.connections))", tsdb),

0 commit comments

Comments
 (0)