Skip to content

Commit 414972d

Browse files
pabloempabloem
authored
Forbidding dimension fields in aggregated stats - only allowed in grouping attributes (#135981)
* Forbidding dimension fields in aggregated stats - only allowed in grouping attributes - fixes #135335 * fixup * fixup comments * fixup * fixtests
1 parent c19d4d7 commit 414972d

File tree

8 files changed

+99
-61
lines changed

8 files changed

+99
-61
lines changed

‎x-pack/plugin/esql/qa/testFixtures/src/main/resources/k8s-timeseries-absent-over-time.csv-spec‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ absent_over_time_of_keyword
149149
required_capability: ts_command_v0
150150
required_capability: absent_over_time
151151
required_capability: k8s_dataset_additional_fields
152-
TS k8s | STATS is_present = max(absent_over_time(pod)) BY cluster, time_bucket = tbucket(10minute) | SORT time_bucket, cluster | LIMIT 10;
152+
TS k8s | STATS is_present = max(absent_over_time(network.eth0.tx)) BY cluster, time_bucket = tbucket(10minute) | SORT time_bucket, cluster | LIMIT 10;
153153

154154
is_present:boolean | cluster:keyword | time_bucket:datetime
155155
false | prod | 2024-05-10T00:00:00.000Z

‎x-pack/plugin/esql/qa/testFixtures/src/main/resources/k8s-timeseries-count-distinct-over-time.csv-spec‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -130,18 +130,19 @@ event_log:long | cluster:keyword | time_bucket:datetime
130130

131131
count_distinct_over_time_of_keyword
132132
required_capability: ts_command_v0
133-
TS k8s | STATS pod = min(count_distinct_over_time(pod)) BY cluster, time_bucket = bucket(@timestamp,10minute) | SORT time_bucket, cluster | LIMIT 10;
133+
TS k8s | STATS pod = min(count_distinct_over_time(network.eth0.up)) BY cluster, time_bucket = bucket(@timestamp,10minute) | SORT time_bucket, cluster | LIMIT 10;
134134

135135
pod:long | cluster:keyword | time_bucket:datetime
136-
1 | prod | 2024-05-10T00:00:00.000Z
137-
1 | qa | 2024-05-10T00:00:00.000Z
138-
1 | staging | 2024-05-10T00:00:00.000Z
139-
1 | prod | 2024-05-10T00:10:00.000Z
140-
1 | qa | 2024-05-10T00:10:00.000Z
136+
2 | prod | 2024-05-10T00:00:00.000Z
137+
2 | qa | 2024-05-10T00:00:00.000Z
138+
2 | staging | 2024-05-10T00:00:00.000Z
139+
2 | prod | 2024-05-10T00:10:00.000Z
140+
2 | qa | 2024-05-10T00:10:00.000Z
141141
1 | staging | 2024-05-10T00:10:00.000Z
142142
1 | prod | 2024-05-10T00:20:00.000Z
143143
1 | qa | 2024-05-10T00:20:00.000Z
144144
1 | staging | 2024-05-10T00:20:00.000Z
145+
145146
;
146147

147148
count_distinct_over_time_with_filtering

‎x-pack/plugin/esql/qa/testFixtures/src/main/resources/k8s-timeseries-count-over-time.csv-spec‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ event_log:long | cluster:keyword | time_bucket:datetime
128128

129129
count_over_time_of_keyword
130130
required_capability: ts_command_v0
131-
TS k8s | STATS pod = min(count_over_time(pod)) BY cluster, time_bucket = bucket(@timestamp,10minute) | SORT time_bucket, cluster | LIMIT 10;
131+
TS k8s | STATS pod = min(count_over_time(network.eth0.up)) BY cluster, time_bucket = bucket(@timestamp,10minute) | SORT time_bucket, cluster | LIMIT 10;
132132

133133
pod:long | cluster:keyword | time_bucket:datetime
134134
4 | prod | 2024-05-10T00:00:00.000Z

‎x-pack/plugin/esql/qa/testFixtures/src/main/resources/k8s-timeseries-max-over-time.csv-spec‎

Lines changed: 27 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -256,36 +256,37 @@ lacus sociosqu, lacinia suspendisse quisque tristique cursus phasellus. Parturie
256256

257257
max_over_time_of_keyword
258258
required_capability: ts_command_v0
259-
TS k8s | STATS pod = min(max_over_time(pod)) BY time_bucket = bucket(@timestamp,1minute) | SORT time_bucket | LIMIT 10;
260-
261-
pod:keyword | time_bucket:datetime
262-
three | 2024-05-10T00:00:00.000Z
263-
one | 2024-05-10T00:01:00.000Z
264-
three | 2024-05-10T00:02:00.000Z
265-
one | 2024-05-10T00:03:00.000Z
266-
one | 2024-05-10T00:04:00.000Z
267-
one | 2024-05-10T00:05:00.000Z
268-
one | 2024-05-10T00:06:00.000Z
269-
one | 2024-05-10T00:07:00.000Z
270-
one | 2024-05-10T00:08:00.000Z
271-
one | 2024-05-10T00:09:00.000Z
259+
TS k8s | STATS pod = min(max_over_time(network.eth0.up)) BY time_bucket = bucket(@timestamp,1minute) | SORT time_bucket | LIMIT 10;
260+
261+
pod:boolean | time_bucket:datetime
262+
false | 2024-05-10T00:00:00.000Z
263+
false | 2024-05-10T00:01:00.000Z
264+
true | 2024-05-10T00:02:00.000Z
265+
false | 2024-05-10T00:03:00.000Z
266+
false | 2024-05-10T00:04:00.000Z
267+
false | 2024-05-10T00:05:00.000Z
268+
false | 2024-05-10T00:06:00.000Z
269+
false | 2024-05-10T00:07:00.000Z
270+
false | 2024-05-10T00:08:00.000Z
271+
false | 2024-05-10T00:09:00.000Z
272+
272273
;
273274

274275
max_over_time_of_keyword_grouping
275276
required_capability: ts_command_v0
276-
TS k8s | STATS pod = min(max_over_time(pod)) BY cluster, time_bucket = bucket(@timestamp,1minute) | SORT time_bucket, cluster | LIMIT 10;
277-
278-
pod:keyword | cluster:keyword | time_bucket:datetime
279-
three | prod | 2024-05-10T00:00:00.000Z
280-
three | staging | 2024-05-10T00:00:00.000Z
281-
one | prod | 2024-05-10T00:01:00.000Z
282-
one | qa | 2024-05-10T00:01:00.000Z
283-
three | prod | 2024-05-10T00:02:00.000Z
284-
three | qa | 2024-05-10T00:02:00.000Z
285-
three | staging | 2024-05-10T00:02:00.000Z
286-
one | prod | 2024-05-10T00:03:00.000Z
287-
three | qa | 2024-05-10T00:03:00.000Z
288-
one | staging | 2024-05-10T00:03:00.000Z
277+
TS k8s | STATS pod = min(max_over_time(network.eth0.up)) BY cluster, time_bucket = bucket(@timestamp,1minute) | SORT time_bucket, cluster | LIMIT 10;
278+
279+
pod:boolean | cluster:keyword | time_bucket:datetime
280+
false | prod | 2024-05-10T00:00:00.000Z
281+
true | staging | 2024-05-10T00:00:00.000Z
282+
false | prod | 2024-05-10T00:01:00.000Z
283+
false | qa | 2024-05-10T00:01:00.000Z
284+
true | prod | 2024-05-10T00:02:00.000Z
285+
true | qa | 2024-05-10T00:02:00.000Z
286+
true | staging | 2024-05-10T00:02:00.000Z
287+
false | prod | 2024-05-10T00:03:00.000Z
288+
true | qa | 2024-05-10T00:03:00.000Z
289+
false | staging | 2024-05-10T00:03:00.000Z
289290
;
290291

291292
max_over_time_of_aggregate_metric_double

‎x-pack/plugin/esql/qa/testFixtures/src/main/resources/k8s-timeseries-min-over-time.csv-spec‎

Lines changed: 28 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -249,36 +249,38 @@ erat. Placerat mi litora fringilla tellus pretium aliquet ut ridiculus magnis ma
249249

250250
min_over_time_of_keyword
251251
required_capability: ts_command_v0
252-
TS k8s | STATS pod = min(min_over_time(pod)) BY time_bucket = bucket(@timestamp,1minute) | SORT time_bucket | LIMIT 10;
253-
254-
pod:keyword | time_bucket:datetime
255-
three | 2024-05-10T00:00:00.000Z
256-
one | 2024-05-10T00:01:00.000Z
257-
three | 2024-05-10T00:02:00.000Z
258-
one | 2024-05-10T00:03:00.000Z
259-
one | 2024-05-10T00:04:00.000Z
260-
one | 2024-05-10T00:05:00.000Z
261-
one | 2024-05-10T00:06:00.000Z
262-
one | 2024-05-10T00:07:00.000Z
263-
one | 2024-05-10T00:08:00.000Z
264-
one | 2024-05-10T00:09:00.000Z
252+
TS k8s | STATS pod = min(min_over_time(network.eth0.up)) BY time_bucket = bucket(@timestamp,1minute) | SORT time_bucket | LIMIT 10;
253+
254+
pod:boolean | time_bucket:datetime
255+
false | 2024-05-10T00:00:00.000Z
256+
false | 2024-05-10T00:01:00.000Z
257+
false | 2024-05-10T00:02:00.000Z
258+
false | 2024-05-10T00:03:00.000Z
259+
false | 2024-05-10T00:04:00.000Z
260+
false | 2024-05-10T00:05:00.000Z
261+
false | 2024-05-10T00:06:00.000Z
262+
false | 2024-05-10T00:07:00.000Z
263+
false | 2024-05-10T00:08:00.000Z
264+
false | 2024-05-10T00:09:00.000Z
265+
265266
;
266267

267268
min_over_time_of_keyword_grouping
268269
required_capability: ts_command_v0
269-
TS k8s | STATS pod = min(min_over_time(pod)) BY cluster, time_bucket = bucket(@timestamp,1minute) | SORT time_bucket, cluster | LIMIT 10;
270-
271-
pod:keyword | cluster:keyword | time_bucket:datetime
272-
three | prod | 2024-05-10T00:00:00.000Z
273-
three | staging | 2024-05-10T00:00:00.000Z
274-
one | prod | 2024-05-10T00:01:00.000Z
275-
one | qa | 2024-05-10T00:01:00.000Z
276-
three | prod | 2024-05-10T00:02:00.000Z
277-
three | qa | 2024-05-10T00:02:00.000Z
278-
three | staging | 2024-05-10T00:02:00.000Z
279-
one | prod | 2024-05-10T00:03:00.000Z
280-
three | qa | 2024-05-10T00:03:00.000Z
281-
one | staging | 2024-05-10T00:03:00.000Z
270+
TS k8s | STATS pod = min(min_over_time(network.eth0.up)) BY cluster, time_bucket = bucket(@timestamp,1minute) | SORT time_bucket, cluster | LIMIT 10;
271+
272+
pod:boolean | cluster:keyword | time_bucket:datetime
273+
false | prod | 2024-05-10T00:00:00.000Z
274+
true | staging | 2024-05-10T00:00:00.000Z
275+
false | prod | 2024-05-10T00:01:00.000Z
276+
false | qa | 2024-05-10T00:01:00.000Z
277+
false | prod | 2024-05-10T00:02:00.000Z
278+
true | qa | 2024-05-10T00:02:00.000Z
279+
false | staging | 2024-05-10T00:02:00.000Z
280+
false | prod | 2024-05-10T00:03:00.000Z
281+
true | qa | 2024-05-10T00:03:00.000Z
282+
false | staging | 2024-05-10T00:03:00.000Z
283+
282284
;
283285

284286
min_over_time_of_aggregate_metric_double

‎x-pack/plugin/esql/qa/testFixtures/src/main/resources/k8s-timeseries-present-over-time.csv-spec‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ present_over_time_of_keyword
137137
required_capability: ts_command_v0
138138
required_capability: present_over_time
139139
required_capability: k8s_dataset_additional_fields
140-
TS k8s | STATS is_present = max(present_over_time(pod)) BY cluster, time_bucket = tbucket(10minute) | SORT time_bucket, cluster | LIMIT 10;
140+
TS k8s | STATS is_present = max(present_over_time(network.eth0.up)) BY cluster, time_bucket = tbucket(10minute) | SORT time_bucket, cluster | LIMIT 10;
141141

142142
is_present:boolean | cluster:keyword | time_bucket:datetime
143143
true | prod | 2024-05-10T00:00:00.000Z

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,7 @@ public boolean equals(Object obj) {
119119
@Override
120120
public void postAnalysisVerification(Failures failures) {
121121
super.postAnalysisVerification(failures);
122+
// We forbid grouping by a metric field itself. Metric fields are allowed only inside aggregate functions.
122123
groupings().forEach(g -> g.forEachDown(e -> {
123124
if (e instanceof FieldAttribute fieldAttr && fieldAttr.isMetric()) {
124125
failures.add(
@@ -202,6 +203,20 @@ protected void checkTimeSeriesAggregates(Failures failures) {
202203
fail(count, "count_star [{}] can't be used with TS command; use count on a field instead", outer.sourceText())
203204
);
204205
}
206+
outer.forEachDown(FieldAttribute.class, fa -> {
207+
if (fa.isDimension()) {
208+
failures.add(
209+
fail(
210+
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+
)
217+
);
218+
}
219+
});
205220
if (outer instanceof TimeSeriesAggregateFunction ts) {
206221
outer.field()
207222
.forEachDown(

‎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
@@ -2689,6 +2689,25 @@ public void testNoMetricInStatsByClause() {
26892689
);
26902690
}
26912691

2692+
public void testNoDimensionsInAggsOnlyInByClause() {
2693+
assertThat(
2694+
error("TS test | STATS count(host) BY bucket(@timestamp, 1 minute)", tsdb),
2695+
equalTo(
2696+
"1:11: cannot use dimension field [host] in a time-series aggregation function [count(host)]. "
2697+
+ "Dimension fields can only be used for grouping in a BY clause. "
2698+
+ "To aggregate dimension fields, use the FROM command instead of the TS command."
2699+
)
2700+
);
2701+
assertThat(
2702+
error("TS test | STATS count(count_over_time(host)) BY bucket(@timestamp, 1 minute)", tsdb),
2703+
equalTo(
2704+
"1:11: cannot use dimension field [host] in a time-series aggregation function [count(count_over_time(host))]. "
2705+
+ "Dimension fields can only be used for grouping in a BY clause. "
2706+
+ "To aggregate dimension fields, use the FROM command instead of the TS command."
2707+
)
2708+
);
2709+
}
2710+
26922711
public void testSortInTimeSeries() {
26932712
assertThat(
26942713
error("TS test | SORT host | STATS avg(last_over_time(network.connections))", tsdb),

0 commit comments

Comments
 (0)