Skip to content

Commit 5ec58af

Browse files
dnhatnKubik42
authored andcommitted
Reject unsupported last_over_time during verification (elastic#136517)
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 elastic#136270
1 parent 3cf6a86 commit 5ec58af

File tree

3 files changed

+22
-19
lines changed

3 files changed

+22
-19
lines changed

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,7 @@ 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"
79+
"implicit time-series aggregation function .* doesn't support type .*"
8280
);
8381

8482
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: 14 additions & 9 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;
@@ -202,21 +205,23 @@ protected void checkTimeSeriesAggregates(Failures failures) {
202205
failures.add(
203206
fail(count, "count_star [{}] can't be used with TS command; use count on a field instead", outer.sourceText())
204207
);
205-
}
206-
outer.forEachDown(FieldAttribute.class, fa -> {
207-
if (fa.isDimension()) {
208+
// reject COUNT(keyword), but allow COUNT(numeric)
209+
} else if (outer instanceof TimeSeriesAggregateFunction == false && outer.field() instanceof AggregateFunction == false) {
210+
Expression field = outer.field();
211+
var lastOverTime = new LastOverTime(source(), field, new Literal(source(), null, DataType.DATETIME));
212+
if (lastOverTime.typeResolved() != Expression.TypeResolution.TYPE_RESOLVED) {
208213
failures.add(
209214
fail(
210215
this,
211-
"cannot use dimension field [{}] in a time-series aggregation function [{}]. "
212-
+ "Dimension fields can only be used for grouping in a BY clause. To aggregate "
213-
+ "dimension fields, use the FROM command instead of the TS command.",
214-
fa.sourceText(),
215-
outer.sourceText()
216+
"implicit time-series aggregation function [{}] generated from [{}] doesn't support type [{}], "
217+
+ "only numeric types are supported; use the FROM command instead of the TS command",
218+
outer.sourceText().replace(field.sourceText(), "last_over_time(" + field.sourceText() + ")"),
219+
outer.sourceText(),
220+
field.dataType().typeName()
216221
)
217222
);
218223
}
219-
});
224+
}
220225
if (outer instanceof TimeSeriesAggregateFunction ts) {
221226
outer.field()
222227
.forEachDown(

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2696,17 +2696,17 @@ public void testNoDimensionsInAggsOnlyInByClause() {
26962696
assertThat(
26972697
error("TS test | STATS count(host) BY bucket(@timestamp, 1 minute)", tsdb),
26982698
equalTo(
2699-
"1:11: cannot use dimension field [host] in a time-series aggregation function [count(host)]. "
2700-
+ "Dimension fields can only be used for grouping in a BY clause. "
2701-
+ "To aggregate dimension fields, use the FROM command instead of the TS command."
2699+
"1:11: implicit time-series aggregation function [count(last_over_time(host))] "
2700+
+ "generated from [count(host)] doesn't support type [keyword], only numeric types are supported; "
2701+
+ "use the FROM command instead of the TS command"
27022702
)
27032703
);
27042704
assertThat(
2705-
error("TS test | STATS count(count_over_time(host)) BY bucket(@timestamp, 1 minute)", tsdb),
2705+
error("TS test | STATS max(name) BY bucket(@timestamp, 1 minute)", tsdb),
27062706
equalTo(
2707-
"1:11: cannot use dimension field [host] in a time-series aggregation function [count(count_over_time(host))]. "
2708-
+ "Dimension fields can only be used for grouping in a BY clause. "
2709-
+ "To aggregate dimension fields, use the FROM command instead of the TS command."
2707+
"1:11: implicit time-series aggregation function [max(last_over_time(name))] "
2708+
+ "generated from [max(name)] doesn't support type [keyword], only numeric types are supported; "
2709+
+ "use the FROM command instead of the TS command"
27102710
)
27112711
);
27122712
}

0 commit comments

Comments
 (0)