Skip to content

Commit 4f9c8de

Browse files
authored
[ES|QL] Add error message when using inline stats on TS before stats (#136348) (#136586)
This commit adds a clearer error message when attempting to use inline stats in a TS query, which is unsupported at the moment. Resolves #136092
1 parent 08690cb commit 4f9c8de

File tree

5 files changed

+130
-1
lines changed

5 files changed

+130
-1
lines changed

docs/changelog/136348.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
pr: 136348
2+
summary: Add error message when using inline stats on TS
3+
area: ES|QL
4+
type: bug
5+
issues:
6+
- 136092

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -559,3 +559,25 @@ cnt:long | cluster:keyword | pod:keyword
559559
1 | prod | two
560560
1 | prod | three
561561
;
562+
563+
inlineStatsAfterStatsCommand
564+
required_capability: ts_command_v0
565+
required_capability: tbucket
566+
TS k8s
567+
| STATS max_cost=max(last_over_time(network.eth0.tx)) BY pod, cluster, time_bucket = TBUCKET(1h)
568+
| INLINE STATS max_cluster_cost=max(max_cost) by cluster
569+
| SORT cluster, pod, time_bucket
570+
| LIMIT 10
571+
;
572+
573+
max_cost:integer | pod:keyword | time_bucket:datetime | max_cluster_cost:integer | cluster:keyword
574+
1149 | one | 2024-05-10T00:00:00.000Z | 1149 | prod
575+
928 | three | 2024-05-10T00:00:00.000Z | 1149 | prod
576+
967 | two | 2024-05-10T00:00:00.000Z | 1149 | prod
577+
1380 | one | 2024-05-10T00:00:00.000Z | 1716 | qa
578+
1241 | three | 2024-05-10T00:00:00.000Z | 1716 | qa
579+
1716 | two | 2024-05-10T00:00:00.000Z | 1716 | qa
580+
581 | one | 2024-05-10T00:00:00.000Z | 1209 | staging
581+
1209 | three | 2024-05-10T00:00:00.000Z | 1209 | staging
582+
973 | two | 2024-05-10T00:00:00.000Z | 1209 | staging
583+
;

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
1111
import org.elasticsearch.common.io.stream.StreamInput;
1212
import org.elasticsearch.common.io.stream.StreamOutput;
13+
import org.elasticsearch.index.IndexMode;
1314
import org.elasticsearch.xpack.esql.capabilities.PostAnalysisVerificationAware;
1415
import org.elasticsearch.xpack.esql.capabilities.TelemetryAware;
1516
import org.elasticsearch.xpack.esql.common.Failures;
@@ -248,6 +249,15 @@ public void postAnalysisVerification(Failures failures) {
248249
}
249250

250251
protected void checkTimeSeriesAggregates(Failures failures) {
252+
Holder<Boolean> isTimeSeries = new Holder<>(false);
253+
child().forEachDown(p -> {
254+
if (p instanceof EsRelation er && er.indexMode() == IndexMode.TIME_SERIES) {
255+
isTimeSeries.set(true);
256+
}
257+
});
258+
if (isTimeSeries.get()) {
259+
return;
260+
}
251261
forEachExpression(
252262
TimeSeriesAggregateFunction.class,
253263
r -> failures.add(fail(r, "time_series aggregate[{}] can only be used with the TS command", r.sourceText()))

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

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,16 @@
1111
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
1212
import org.elasticsearch.common.io.stream.StreamInput;
1313
import org.elasticsearch.common.io.stream.StreamOutput;
14+
import org.elasticsearch.index.IndexMode;
15+
import org.elasticsearch.xpack.esql.capabilities.PostAnalysisPlanVerificationAware;
1416
import org.elasticsearch.xpack.esql.capabilities.TelemetryAware;
17+
import org.elasticsearch.xpack.esql.common.Failures;
1518
import org.elasticsearch.xpack.esql.core.expression.Attribute;
1619
import org.elasticsearch.xpack.esql.core.expression.Expression;
1720
import org.elasticsearch.xpack.esql.core.expression.Expressions;
1821
import org.elasticsearch.xpack.esql.core.tree.NodeInfo;
1922
import org.elasticsearch.xpack.esql.core.tree.Source;
23+
import org.elasticsearch.xpack.esql.core.util.Holder;
2024
import org.elasticsearch.xpack.esql.io.stream.PlanStreamInput;
2125
import org.elasticsearch.xpack.esql.plan.logical.join.InlineJoin;
2226
import org.elasticsearch.xpack.esql.plan.logical.join.Join;
@@ -27,8 +31,10 @@
2731
import java.util.ArrayList;
2832
import java.util.List;
2933
import java.util.Objects;
34+
import java.util.function.BiConsumer;
3035

3136
import static java.util.Collections.emptyList;
37+
import static org.elasticsearch.xpack.esql.common.Failure.fail;
3238
import static org.elasticsearch.xpack.esql.expression.NamedExpressions.mergeOutputAttributes;
3339

3440
/**
@@ -38,7 +44,13 @@
3844
* underlying aggregate.
3945
* </p>
4046
*/
41-
public class InlineStats extends UnaryPlan implements NamedWriteable, SurrogateLogicalPlan, TelemetryAware, SortAgnostic {
47+
public class InlineStats extends UnaryPlan
48+
implements
49+
NamedWriteable,
50+
SurrogateLogicalPlan,
51+
TelemetryAware,
52+
SortAgnostic,
53+
PostAnalysisPlanVerificationAware {
4254
public static final NamedWriteableRegistry.Entry ENTRY = new NamedWriteableRegistry.Entry(
4355
LogicalPlan.class,
4456
"InlineStats",
@@ -152,4 +164,37 @@ public boolean equals(Object obj) {
152164
InlineStats other = (InlineStats) obj;
153165
return Objects.equals(aggregate, other.aggregate);
154166
}
167+
168+
@Override
169+
public BiConsumer<LogicalPlan, Failures> postAnalysisPlanVerification() {
170+
return (p, failures) -> {
171+
// Allow inline stats to be used with TS command if it follows a STATS command
172+
// Examples:
173+
// valid: TS metrics | STATS ...
174+
// valid: TS metrics | STATS ... | INLINE STATS ...
175+
// invalid: TS metrics | INLINE STATS ...
176+
// invalid: TS metrics | INLINE STATS ... | STATS ...
177+
if (p instanceof InlineStats inlineStats) {
178+
Holder<Boolean> foundInlineStats = new Holder<>(false);
179+
Holder<Boolean> foundPreviousStats = new Holder<>(false);
180+
Holder<Boolean> isTimeSeries = new Holder<>(false);
181+
inlineStats.child().forEachUp(lp -> {
182+
if (lp instanceof Aggregate) {
183+
if (foundInlineStats.get() == false) {
184+
foundInlineStats.set(true);
185+
} else {
186+
foundPreviousStats.set(true);
187+
}
188+
} else if (lp instanceof EsRelation er && er.indexMode() == IndexMode.TIME_SERIES) {
189+
isTimeSeries.set(true);
190+
}
191+
});
192+
if (isTimeSeries.get() && foundPreviousStats.get() == false) {
193+
failures.add(
194+
fail(inlineStats, "INLINE STATS [{}] can only be used after STATS when used with TS command", this.sourceText())
195+
);
196+
}
197+
}
198+
};
199+
}
155200
}

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

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2765,6 +2765,52 @@ public void testTextEmbeddingFunctionInvalidInferenceId() {
27652765
);
27662766
}
27672767

2768+
public void testInlineStatsInTSNotAllowed() {
2769+
assertThat(error("TS test | INLINE STATS max(network.connections)", tsdb), equalTo("""
2770+
1:11: INLINE STATS [INLINE STATS max(network.connections)] \
2771+
can only be used after STATS when used with TS command"""));
2772+
2773+
assertThat(error("""
2774+
TS test |
2775+
INLINE STATS max(network.connections) |
2776+
STATS max(max_over_time(network.connections)) BY host, time_bucket = bucket(@timestamp,1minute)""", tsdb), equalTo("""
2777+
2:1: INLINE STATS [INLINE STATS max(network.connections)] \
2778+
can only be used after STATS when used with TS command"""));
2779+
2780+
assertThat(error("TS test | INLINE STATS max_bytes=max(to_long(network.bytes_in)) BY host", tsdb), equalTo("""
2781+
1:11: INLINE STATS [INLINE STATS max_bytes=max(to_long(network.bytes_in)) BY host] \
2782+
can only be used after STATS when used with TS command"""));
2783+
2784+
assertThat(error("TS test | INLINE STATS max(60 * rate(network.bytes_in)), max(network.connections)", tsdb), equalTo("""
2785+
1:11: INLINE STATS [INLINE STATS max(60 * rate(network.bytes_in)), max(network.connections)] \
2786+
can only be used after STATS when used with TS command"""));
2787+
2788+
assertThat(error("TS test METADATA _tsid | INLINE STATS cnt = count_distinct(_tsid) BY metricset, host", tsdb), equalTo("""
2789+
1:26: INLINE STATS [INLINE STATS cnt = count_distinct(_tsid) BY metricset, host] \
2790+
can only be used after STATS when used with TS command"""));
2791+
2792+
assertThat(
2793+
error("""
2794+
TS test |
2795+
INLINE STATS max_cost=max(last_over_time(network.connections)) BY host, time_bucket = bucket(@timestamp,1minute)""", tsdb),
2796+
equalTo("""
2797+
2:1: INLINE STATS [INLINE STATS max_cost=max(last_over_time(network.connections)) \
2798+
BY host, time_bucket = bucket(@timestamp,1minute)] \
2799+
can only be used after STATS when used with TS command""")
2800+
);
2801+
2802+
assertThat(
2803+
error("TS test | INLINE STATS max_bytes=max(to_long(network.bytes_in)) BY host | SORT max_bytes DESC | keep max*, host", tsdb),
2804+
equalTo("""
2805+
1:11: INLINE STATS [INLINE STATS max_bytes=max(to_long(network.bytes_in)) BY host] \
2806+
can only be used after STATS when used with TS command""")
2807+
);
2808+
2809+
assertThat(error("TS test | INLINE STATS max(network.connections) | STATS max(network.connections) by host", tsdb), equalTo("""
2810+
1:11: INLINE STATS [INLINE STATS max(network.connections)] \
2811+
can only be used after STATS when used with TS command"""));
2812+
}
2813+
27682814
private void checkVectorFunctionsNullArgs(String functionInvocation) throws Exception {
27692815
query("from test | eval similarity = " + functionInvocation, fullTextAnalyzer);
27702816
}

0 commit comments

Comments
 (0)