Skip to content

Commit ba78f85

Browse files
committed
Verify step is aligned with range selector lookback
1 parent bd082e0 commit ba78f85

File tree

3 files changed

+36
-9
lines changed

3 files changed

+36
-9
lines changed

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

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,10 @@
1313
import org.elasticsearch.xpack.esql.common.Failures;
1414
import org.elasticsearch.xpack.esql.core.tree.NodeInfo;
1515
import org.elasticsearch.xpack.esql.core.tree.Source;
16+
import org.elasticsearch.xpack.esql.expression.promql.subquery.Subquery;
1617
import org.elasticsearch.xpack.esql.plan.logical.LogicalPlan;
1718
import org.elasticsearch.xpack.esql.plan.logical.UnaryPlan;
19+
import org.elasticsearch.xpack.esql.plan.logical.promql.selector.RangeSelector;
1820
import org.elasticsearch.xpack.esql.plan.logical.promql.selector.Selector;
1921

2022
import java.io.IOException;
@@ -140,12 +142,27 @@ public void postAnalysisVerification(Failures failures) {
140142
if (p instanceof AcrossSeriesAggregate == false) {
141143
failures.add(fail(p, "only aggregations across timeseries are supported at this time (found [{}])", p.sourceText()));
142144
}
143-
p.forEachDown(Selector.class, s -> {
144-
if (s.labelMatchers().nameLabel().matcher().isRegex()) {
145-
failures.add(fail(s, "regex label selectors on __name__ are not supported at this time [{}]", s.sourceText()));
145+
p.forEachDown(lp -> {
146+
if (lp instanceof Selector s) {
147+
if (s.labelMatchers().nameLabel().matcher().isRegex()) {
148+
failures.add(fail(s, "regex label selectors on __name__ are not supported at this time [{}]", s.sourceText()));
149+
}
150+
if (s.evaluation() != null && s.evaluation().offset() != null && s.evaluation().offset().isZero() == false) {
151+
failures.add(fail(s, "offset modifiers are not supported at this time [{}]", s.sourceText()));
152+
}
146153
}
147-
if (s.evaluation() != null && s.evaluation().offset() != null && s.evaluation().offset().isZero() == false) {
148-
failures.add(fail(s, "offset modifiers are not supported at this time [{}]", s.sourceText()));
154+
if (step() != null && lp instanceof RangeSelector rs) {
155+
Duration rangeDuration = (Duration) rs.range().fold(null);
156+
if (rangeDuration.equals(step()) == false) {
157+
failures.add(
158+
fail(
159+
rs.range(),
160+
"the duration for range vector selector [{}] "
161+
+ "must be equal to the query's step for range queries at this time",
162+
rs.range().sourceText()
163+
)
164+
);
165+
}
149166
}
150167
});
151168
}

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,16 @@ public void testPromqlMissingAcrossSeriesAggregation() {
3232
);
3333
}
3434

35+
public void testPromqlStepAndRangeMisaligned() {
36+
assertThat(
37+
error("""
38+
TS test | PROMQL step 1m (
39+
avg(rate(network.bytes_in[5m]))
40+
)""", tsdb),
41+
equalTo("2:29: the duration for range vector selector [5m] must be equal to the query's step for range queries at this time")
42+
);
43+
}
44+
3545
public void testPromqlIllegalNameLabelMatcher() {
3646
assertThat(
3747
error("TS test | PROMQL step 5m (avg({__name__=~\"*.foo.*\"}))", tsdb),

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/promql/PromqlLogicalPlanOptimizerTests.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ public void testAvgAvgOverTimeOutput() {
101101
// | STATS AVG(AVG_OVER_TIME(`metrics.system.memory.utilization`)) BY host.name, TBUCKET(1h) | LIMIT 10000"
102102
var plan = planPromql("""
103103
TS k8s
104-
| promql step 5m ( avg by (pod) (avg_over_time(network.bytes_in{pod=~"host-0|host-1|host-2"}[1h])) )
104+
| promql step 1h ( avg by (pod) (avg_over_time(network.bytes_in{pod=~"host-0|host-1|host-2"}[1h])) )
105105
| LIMIT 1000
106106
""");
107107

@@ -137,7 +137,7 @@ public void testPromqlAvgWithoutByDimension() {
137137
// | STATS AVG(AVG_OVER_TIME(`metrics.system.memory.utilization`)) BY TBUCKET(1h) | LIMIT 10000"
138138
var plan = planPromql("""
139139
TS k8s
140-
| promql step 5m (
140+
| promql step 1h (
141141
avg(avg_over_time(network.bytes_in[1h]))
142142
)
143143
| LIMIT 1000
@@ -152,7 +152,7 @@ public void testRangeSelector() {
152152
// | STATS AVG(AVG_OVER_TIME(`metrics.system.memory.utilization`)) BY host.name, TBUCKET(1h) | LIMIT 10000"
153153
var plan = planPromql("""
154154
TS k8s
155-
| promql step 10 ( max by (pod) (avg_over_time(network.bytes_in[1h])) )
155+
| promql step 1h ( max by (pod) (avg_over_time(network.bytes_in[1h])) )
156156
""");
157157

158158
System.out.println(plan);
@@ -164,7 +164,7 @@ public void testRate() {
164164
// | STATS AVG(RATE(`metrics.system.cpu.time`)) BY host.name, TBUCKET(1h) | LIMIT 10000"
165165
String testQuery = """
166166
TS k8s
167-
| promql step 42 (
167+
| promql step 1h (
168168
avg by (pod) (rate(network.bytes_in[1h]))
169169
)
170170
""";

0 commit comments

Comments
 (0)