Skip to content

Commit 2c74e70

Browse files
felixbarnyfelixbarny
authored
Verify PromqlCommand and throw on currently unsupported queries (#137264)
1 parent d221e6a commit 2c74e70

File tree

4 files changed

+169
-3
lines changed

4 files changed

+169
-3
lines changed

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

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,23 +8,29 @@
88
package org.elasticsearch.xpack.esql.plan.logical.promql;
99

1010
import org.elasticsearch.common.io.stream.StreamOutput;
11+
import org.elasticsearch.core.TimeValue;
12+
import org.elasticsearch.xpack.esql.capabilities.PostAnalysisVerificationAware;
1113
import org.elasticsearch.xpack.esql.capabilities.TelemetryAware;
14+
import org.elasticsearch.xpack.esql.common.Failures;
1215
import org.elasticsearch.xpack.esql.core.expression.Expression;
1316
import org.elasticsearch.xpack.esql.core.expression.Literal;
1417
import org.elasticsearch.xpack.esql.core.tree.NodeInfo;
1518
import org.elasticsearch.xpack.esql.core.tree.Source;
1619
import org.elasticsearch.xpack.esql.plan.logical.LogicalPlan;
1720
import org.elasticsearch.xpack.esql.plan.logical.UnaryPlan;
21+
import org.elasticsearch.xpack.esql.plan.logical.promql.selector.Selector;
1822

1923
import java.io.IOException;
2024
import java.time.Duration;
2125
import java.util.Objects;
2226

27+
import static org.elasticsearch.xpack.esql.common.Failure.fail;
28+
2329
/**
2430
* Container plan for embedded PromQL queries.
2531
* Gets eliminated by the analyzer once the query is validated.
2632
*/
27-
public class PromqlCommand extends UnaryPlan implements TelemetryAware {
33+
public class PromqlCommand extends UnaryPlan implements TelemetryAware, PostAnalysisVerificationAware {
2834

2935
private final LogicalPlan promqlPlan;
3036
private final Expression start, end, step;
@@ -123,4 +129,38 @@ public String nodeString() {
123129
sb.append("\n<>]]");
124130
return sb.toString();
125131
}
132+
133+
@Override
134+
public void postAnalysisVerification(Failures failures) {
135+
promqlPlan().forEachDownMayReturnEarly((lp, breakEarly) -> {
136+
if (lp instanceof PromqlFunctionCall fc) {
137+
if (fc instanceof AcrossSeriesAggregate) {
138+
breakEarly.set(true);
139+
fc.forEachDown((childLp -> verifyNonFunctionCall(failures, childLp)));
140+
} else if (fc instanceof WithinSeriesAggregate withinSeriesAggregate) {
141+
failures.add(
142+
fail(
143+
withinSeriesAggregate,
144+
"within time series aggregate function [{}] "
145+
+ "can only be used inside an across time series aggregate function at this time",
146+
withinSeriesAggregate.sourceText()
147+
)
148+
);
149+
}
150+
} else {
151+
verifyNonFunctionCall(failures, lp);
152+
}
153+
});
154+
}
155+
156+
private void verifyNonFunctionCall(Failures failures, LogicalPlan logicalPlan) {
157+
if (logicalPlan instanceof Selector s) {
158+
if (s.labelMatchers().nameLabel().matcher().isRegex()) {
159+
failures.add(fail(s, "regex label selectors on __name__ are not supported at this time [{}]", s.sourceText()));
160+
}
161+
if (s.evaluation().offset() != null && s.evaluation().offset() != TimeValue.ZERO) {
162+
failures.add(fail(s, "offset modifiers are not supported at this time [{}]", s.sourceText()));
163+
}
164+
}
165+
}
126166
}

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,10 @@ public static Matcher from(String value) {
6060
this.value = value;
6161
this.isRegex = isRegex;
6262
}
63+
64+
public boolean isRegex() {
65+
return isRegex;
66+
}
6367
}
6468

6569
private final String name;

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3291,15 +3291,15 @@ private String error(String query, Object... params) {
32913291
return error(query, defaultAnalyzer, VerificationException.class, params);
32923292
}
32933293

3294-
private String error(String query, Analyzer analyzer, Object... params) {
3294+
public static String error(String query, Analyzer analyzer, Object... params) {
32953295
return error(query, analyzer, VerificationException.class, params);
32963296
}
32973297

32983298
private String error(String query, TransportVersion transportVersion, Object... params) {
32993299
return error(query, transportVersion, VerificationException.class, params);
33003300
}
33013301

3302-
private String error(String query, Analyzer analyzer, Class<? extends Exception> exception, Object... params) {
3302+
public static String error(String query, Analyzer analyzer, Class<? extends Exception> exception, Object... params) {
33033303
List<QueryParam> parameters = new ArrayList<>();
33043304
for (Object param : params) {
33053305
if (param == null) {

‎x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/promql/PromqlVerifierTests.java‎

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0; you may not use this file except in compliance with the Elastic License
5+
* 2.0.
6+
*/
7+
8+
package org.elasticsearch.xpack.esql.analysis.promql;
9+
10+
import org.elasticsearch.test.ESTestCase;
11+
import org.elasticsearch.xpack.esql.analysis.Analyzer;
12+
import org.elasticsearch.xpack.esql.analysis.AnalyzerTestUtils;
13+
import org.junit.Ignore;
14+
15+
import java.util.List;
16+
17+
import static org.elasticsearch.xpack.esql.EsqlTestUtils.withDefaultLimitWarning;
18+
import static org.elasticsearch.xpack.esql.analysis.VerifierTests.error;
19+
import static org.hamcrest.Matchers.equalTo;
20+
21+
public class PromqlVerifierTests extends ESTestCase {
22+
23+
private final Analyzer tsdb = AnalyzerTestUtils.analyzer(AnalyzerTestUtils.tsdbIndexResolution());
24+
25+
public void testPromqlMissingAcrossSeriesAggregation() {
26+
assertThat(
27+
error("""
28+
TS test | PROMQL step 5m (
29+
rate(network.bytes_in[5m])
30+
)""", tsdb),
31+
equalTo(
32+
"2:3: within time series aggregate function [rate(network.bytes_in[5m])] can only be used "
33+
+ "inside an across time series aggregate function at this time"
34+
)
35+
);
36+
}
37+
38+
public void testPromqlIllegalNameLabelMatcher() {
39+
assertThat(
40+
error("TS test | PROMQL step 5m ({__name__=~\"*.foo.*\"})", tsdb),
41+
equalTo("1:27: regex label selectors on __name__ are not supported at this time [{__name__=~\"*.foo.*\"}]")
42+
);
43+
}
44+
45+
@Ignore
46+
public void testPromqlSubquery() {
47+
// TODO doesn't parse
48+
// line 1:36: Invalid query 'network.bytes_in'[ValueExpressionContext] given; expected Expression but found
49+
// InstantSelector
50+
assertThat(error("TS test | PROMQL step 5m (avg(rate(network.bytes_in[5m:])))", tsdb), equalTo(""));
51+
assertThat(error("TS test | PROMQL step 5m (avg(rate(network.bytes_in[5m:1m])))", tsdb), equalTo(""));
52+
}
53+
54+
@Ignore
55+
public void testPromqlArithmetricOperators() {
56+
// TODO doesn't parse
57+
// line 1:27: Invalid query '1+1'[ArithmeticBinaryContext] given; expected LogicalPlan but found VectorBinaryArithmetic
58+
assertThat(
59+
error("TS test | PROMQL step 5m (1+1)", tsdb),
60+
equalTo("1:27: arithmetic operators are not supported at this time [foo]")
61+
);
62+
assertThat(
63+
error("TS test | PROMQL step 5m (foo+1)", tsdb),
64+
equalTo("1:27: arithmetic operators are not supported at this time [foo]")
65+
);
66+
assertThat(
67+
error("TS test | PROMQL step 5m (1+foo)", tsdb),
68+
equalTo("1:27: arithmetic operators are not supported at this time [foo]")
69+
);
70+
assertThat(
71+
error("TS test | PROMQL step 5m (foo+bar)", tsdb),
72+
equalTo("1:27: arithmetic operators are not supported at this time [foo]")
73+
);
74+
}
75+
76+
@Ignore
77+
public void testPromqlVectorMatching() {
78+
// TODO doesn't parse
79+
// line 1:27: Invalid query 'method_code_http_errors_rate5m{code="500"}'[ValueExpressionContext] given; expected Expression but
80+
// found InstantSelector
81+
assertThat(
82+
error(
83+
"TS test | PROMQL step 5m (method_code_http_errors_rate5m{code=\"500\"} / ignoring(code) method_http_requests_rate5m)",
84+
tsdb
85+
),
86+
equalTo("")
87+
);
88+
assertThat(
89+
error(
90+
"TS test | PROMQL step 5m (method_code_http_errors_rate5m / ignoring(code) group_left method_http_requests_rate5m)",
91+
tsdb
92+
),
93+
equalTo("")
94+
);
95+
}
96+
97+
public void testPromqlModifier() {
98+
assertThat(
99+
error("TS test | PROMQL step 5m (foo offset 5m)", tsdb),
100+
equalTo("1:27: offset modifiers are not supported at this time [foo offset 5m]")
101+
);
102+
/* TODO
103+
assertThat(
104+
error("TS test | PROMQL step 5m (foo @ start())", tsdb),
105+
equalTo("1:27: @ modifiers are not supported at this time [foo @ start()]")
106+
);*/
107+
}
108+
109+
@Ignore
110+
public void testLogicalSetBinaryOperators() {
111+
// TODO doesn't parse
112+
// line 1:27: Invalid query 'foo'[ValueExpressionContext] given; expected Expression but found InstantSelector
113+
assertThat(error("TS test | PROMQL step 5m (foo and bar)", tsdb), equalTo(""));
114+
assertThat(error("TS test | PROMQL step 5m (foo or bar)", tsdb), equalTo(""));
115+
assertThat(error("TS test | PROMQL step 5m (foo unless bar)", tsdb), equalTo(""));
116+
}
117+
118+
@Override
119+
protected List<String> filteredWarnings() {
120+
return withDefaultLimitWarning(super.filteredWarnings());
121+
}
122+
}

0 commit comments

Comments
 (0)