Skip to content

Commit 140af7e

Browse files
Remove folding from MvSort
1 parent d019b65 commit 140af7e

File tree

2 files changed

+82
-11
lines changed
  • x-pack/plugin
    • esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/multivalue
    • src/yamlRestTest/resources/rest-api-spec/test/esql

2 files changed

+82
-11
lines changed

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/multivalue/MvSort.java

Lines changed: 43 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,10 @@
4848
import org.elasticsearch.xpack.esql.planner.PlannerUtils;
4949

5050
import java.io.IOException;
51+
import java.util.ArrayList;
5152
import java.util.Arrays;
5253
import java.util.List;
54+
import java.util.Objects;
5355

5456
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.ParamOrdinal.FIRST;
5557
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.ParamOrdinal.SECOND;
@@ -146,6 +148,19 @@ public boolean foldable() {
146148
return field.foldable() && (order == null || order.foldable());
147149
}
148150

151+
@Override
152+
public Object fold(FoldContext ctx) {
153+
if (order instanceof Literal == false) {
154+
// if we are here, we know that order is foldable, so we can safely fold it
155+
Literal newOrder = Literal.of(ctx, order);
156+
List<Expression> newChildren = new ArrayList<>();
157+
newChildren.add(field);
158+
newChildren.add(newOrder);
159+
return replaceChildren(newChildren).fold(ctx);
160+
}
161+
return super.fold(ctx);
162+
}
163+
149164
@Override
150165
public EvalOperator.ExpressionEvaluator.Factory toEvaluator(ToEvaluator toEvaluator) {
151166
boolean ordering = true;
@@ -246,7 +261,7 @@ public void postOptimizationVerification(Failures failures) {
246261
sourceText(),
247262
BytesRefs.toString(ASC.value()),
248263
BytesRefs.toString(DESC.value()),
249-
BytesRefs.toString(order.fold(FoldContext.small() /* TODO remove me */))
264+
BytesRefs.toString(order)
250265
)
251266
);
252267
}
@@ -255,16 +270,21 @@ public void postOptimizationVerification(Failures failures) {
255270
private boolean isValidOrder() {
256271
boolean isValidOrder = true;
257272
if (order != null && order.foldable()) {
258-
Object obj = order.fold(FoldContext.small() /* TODO remove me */);
259-
String o = null;
260-
if (obj instanceof BytesRef ob) {
261-
o = ob.utf8ToString();
262-
} else if (obj instanceof String os) {
263-
o = os;
264-
}
265-
if (o == null
266-
|| o.equalsIgnoreCase(BytesRefs.toString(ASC.value())) == false
267-
&& o.equalsIgnoreCase(BytesRefs.toString(DESC.value())) == false) {
273+
if (order instanceof Literal literal) {
274+
Object obj = literal.value();
275+
String o = null;
276+
if (obj instanceof BytesRef ob) {
277+
o = ob.utf8ToString();
278+
} else if (obj instanceof String os) {
279+
o = os;
280+
}
281+
if (o == null
282+
|| o.equalsIgnoreCase(BytesRefs.toString(ASC.value())) == false
283+
&& o.equalsIgnoreCase(BytesRefs.toString(DESC.value())) == false) {
284+
isValidOrder = false;
285+
}
286+
} else {
287+
// order should be folded already, so if it is not literal it is invalid
268288
isValidOrder = false;
269289
}
270290
}
@@ -288,6 +308,18 @@ public String toString() {
288308
}
289309
}
290310

311+
@Override
312+
public boolean equals(Object o) {
313+
if (o == null || getClass() != o.getClass()) return false;
314+
MvSort mvSort = (MvSort) o;
315+
return Objects.equals(field(), mvSort.field()) && Objects.equals(order(), mvSort.order());
316+
}
317+
318+
@Override
319+
public int hashCode() {
320+
return Objects.hash(field(), order());
321+
}
322+
291323
private static class Evaluator implements EvalOperator.ExpressionEvaluator {
292324
private final BlockFactory blockFactory;
293325
private final EvalOperator.ExpressionEvaluator field;

x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/230_folding.yml

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -615,3 +615,42 @@ COUNT_DISTINCT with non-foldable precision:
615615
| STATS COUNT_DISTINCT(salary, salary + 1) | LIMIT 1
616616
- match: { error.type: "verification_exception" }
617617
- contains: { error.reason: "second argument of [COUNT_DISTINCT(salary, salary + 1)] must be a constant, received [salary + 1]" }
618+
619+
---
620+
MV_SORT with foldable order:
621+
- do:
622+
esql.query:
623+
body:
624+
query: |
625+
ROW a = [4, 2, -3, 2]
626+
| EVAL sa = mv_sort(a, CONCAT("AS", "C")), sd = mv_sort(a, CONCAT("DE", "SC")) | LIMIT 1
627+
- match: { columns.0.name: "a" }
628+
- match: { columns.1.name: "sa" }
629+
- match: { columns.2.name: "sd" }
630+
- match: { values.0.1: [ -3, 2, 2, 4 ] }
631+
- match: { values.0.2: [ 4, 2, 2, -3 ] }
632+
633+
---
634+
MV_SORT with invalid foldable order:
635+
- do:
636+
catch: bad_request
637+
esql.query:
638+
body:
639+
query: |
640+
ROW a = [4, 2, -3, 2]
641+
| EVAL sa = mv_sort(a, CONCAT("ASC", "DESC")) | LIMIT 1
642+
- match: { error.type: "illegal_argument_exception" }
643+
- contains: { error.reason: "Invalid order value in [mv_sort(a, CONCAT(\"ASC\", \"DESC\"))], expected one of [ASC, DESC] but got [ASCDESC]" }
644+
645+
646+
---
647+
MV_SORT with invalid non-foldable order:
648+
- do:
649+
catch: bad_request
650+
esql.query:
651+
body:
652+
query: |
653+
ROW a = [4, 2, -3, 2], str = "REVERSE"
654+
| EVAL sa = mv_sort(a, CONCAT(str, "DESC")) | LIMIT 1
655+
- match: { error.type: "illegal_argument_exception" }
656+
- contains: { error.reason: "Invalid order value in [mv_sort(a, CONCAT(str, \"DESC\"))], expected one of [ASC, DESC] but got [REVERSEDESC]" }

0 commit comments

Comments
 (0)