Skip to content

Commit 61e24dd

Browse files
committed
Error handling
1 parent 5cab77b commit 61e24dd

File tree

2 files changed

+36
-3
lines changed

2 files changed

+36
-3
lines changed

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

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010
import org.elasticsearch.common.io.stream.StreamInput;
1111
import org.elasticsearch.common.io.stream.StreamOutput;
1212
import org.elasticsearch.compute.operator.ChangePointOperator;
13+
import org.elasticsearch.xpack.esql.capabilities.PostAnalysisVerificationAware;
14+
import org.elasticsearch.xpack.esql.common.Failures;
1315
import org.elasticsearch.xpack.esql.core.expression.Attribute;
1416
import org.elasticsearch.xpack.esql.core.expression.AttributeSet;
1517
import org.elasticsearch.xpack.esql.core.expression.Expressions;
@@ -28,7 +30,9 @@
2830
import java.util.List;
2931
import java.util.Objects;
3032

31-
public class ChangePoint extends UnaryPlan implements GeneratingPlan<ChangePoint>, SurrogateLogicalPlan {
33+
import static org.elasticsearch.xpack.esql.common.Failure.fail;
34+
35+
public class ChangePoint extends UnaryPlan implements GeneratingPlan<ChangePoint>, SurrogateLogicalPlan, PostAnalysisVerificationAware {
3236

3337
public static final NamedWriteableRegistry.Entry ENTRY = new NamedWriteableRegistry.Entry(
3438
LogicalPlan.class,
@@ -167,15 +171,18 @@ public boolean equals(Object obj) {
167171
&& Objects.equals(targetPvalue, other.targetPvalue);
168172
}
169173

174+
private Order order() {
175+
return new Order(source(), key, Order.OrderDirection.ASC, Order.NullsPosition.ANY);
176+
}
177+
170178
@Override
171179
public LogicalPlan surrogate() {
172180
// ChangePoint should always run on the coordinating node after the data is collected
173181
// in sorted order. This is enforced by adding OrderBy here.
174182
// Furthermore, ChangePoint should be called with at most 1000 data points. That's
175183
// enforced by the Limits here. The first Limit of N+1 data points is necessary to
176184
// generate a possible warning, the second Limit of N is to truncate the output.
177-
Order order = new Order(source(), key, Order.OrderDirection.ASC, Order.NullsPosition.ANY);
178-
OrderBy orderBy = new OrderBy(source(), child(), List.of(order));
185+
OrderBy orderBy = new OrderBy(source(), child(), List.of(order()));
179186
Limit limit = new Limit(
180187
source(),
181188
new Literal(Source.EMPTY, ChangePointOperator.INPUT_VALUE_COUNT_LIMIT + 1, DataType.INTEGER),
@@ -184,4 +191,17 @@ public LogicalPlan surrogate() {
184191
ChangePoint changePoint = new ChangePoint(source(), limit, value, key, targetType, targetPvalue);
185192
return new Limit(source(), new Literal(Source.EMPTY, ChangePointOperator.INPUT_VALUE_COUNT_LIMIT, DataType.INTEGER), changePoint);
186193
}
194+
195+
@Override
196+
public void postAnalysisVerification(Failures failures) {
197+
// Key must be sortable
198+
Order order = order();
199+
if (DataType.isSortable(order.dataType()) == false) {
200+
failures.add(fail(this, "change point key [" + key.name() + "] must be sortable"));
201+
}
202+
// Value must be a number
203+
if (value.dataType().isNumeric() == false) {
204+
failures.add(fail(this, "change point value [" + value.name() + "] must be numeric"));
205+
}
206+
}
187207
}

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2003,6 +2003,19 @@ public void testCategorizeWithFilteredAggregations() {
20032003
);
20042004
}
20052005

2006+
public void testChangePoint() {
2007+
var airports = AnalyzerTestUtils.analyzer(loadMapping("mapping-airports.json", "airports"));
2008+
assertEquals(
2009+
"1:17: change point key [location] must be sortable",
2010+
error("FROM airports | CHANGE_POINT scalerank ON location", airports)
2011+
);
2012+
assertEquals(
2013+
"1:17: change point value [location] must be numeric",
2014+
error("FROM airports | CHANGE_POINT location ON scalerank", airports)
2015+
);
2016+
2017+
}
2018+
20062019
public void testSortByAggregate() {
20072020
assertEquals("1:18: Aggregate functions are not allowed in SORT [COUNT]", error("ROW a = 1 | SORT count(*)"));
20082021
assertEquals("1:28: Aggregate functions are not allowed in SORT [COUNT]", error("ROW a = 1 | SORT to_string(count(*))"));

0 commit comments

Comments
 (0)