Skip to content

Commit 3881eb5

Browse files
authored
ESQL: Remaining serialization tests (elastic#143470)
This adds the remaining serialization tests. It makes a few production code changes: 1. Registers `TO_DATE_RANGE` for serialization. It was serializable and not registered. 2. Normalizes the serialization of `SCORE` and explains how it used to work. It was not broken but held together strangely. 3. Replaces implementing `SurrogateExpression` with `OnlySurrogateExpression` in a few more cases so we don't expect serialization to work. 4. Adds some package private accessors so we can refer to children by name in serialization tests.
1 parent f7f45e8 commit 3881eb5

File tree

47 files changed

+1218
-67
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

47 files changed

+1218
-67
lines changed

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/ExpressionWritables.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToCartesianPoint;
2424
import org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToCartesianShape;
2525
import org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToDateNanos;
26+
import org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToDateRange;
2627
import org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToDatetime;
2728
import org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToDegrees;
2829
import org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToDenseVector;
@@ -230,6 +231,7 @@ public static List<NamedWriteableRegistry.Entry> unaryScalars() {
230231
entries.add(ToCartesianPoint.ENTRY);
231232
entries.add(ToDatetime.ENTRY);
232233
entries.add(ToDateNanos.ENTRY);
234+
entries.add(ToDateRange.ENTRY);
233235
entries.add(ToDegrees.ENTRY);
234236
entries.add(ToDenseVector.ENTRY);
235237
entries.add(ToDouble.ENTRY);

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/PercentileOverTime.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
import org.elasticsearch.xpack.esql.core.tree.NodeInfo;
1414
import org.elasticsearch.xpack.esql.core.tree.Source;
1515
import org.elasticsearch.xpack.esql.core.type.DataType;
16-
import org.elasticsearch.xpack.esql.expression.SurrogateExpression;
16+
import org.elasticsearch.xpack.esql.expression.OnlySurrogateExpression;
1717
import org.elasticsearch.xpack.esql.expression.function.Example;
1818
import org.elasticsearch.xpack.esql.expression.function.FunctionAppliesTo;
1919
import org.elasticsearch.xpack.esql.expression.function.FunctionAppliesToLifecycle;
@@ -27,7 +27,7 @@
2727
/**
2828
* Similar to {@link Percentile}, but it is used to calculate the percentile value over a time series of values from the given field.
2929
*/
30-
public class PercentileOverTime extends TimeSeriesAggregateFunction implements SurrogateExpression, ToAggregator {
30+
public class PercentileOverTime extends TimeSeriesAggregateFunction implements OnlySurrogateExpression, ToAggregator {
3131
@FunctionInfo(
3232
returnType = "double",
3333
description = "Calculates the percentile over time of a field.",

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/StddevOverTime.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
import org.elasticsearch.xpack.esql.core.tree.NodeInfo;
1414
import org.elasticsearch.xpack.esql.core.tree.Source;
1515
import org.elasticsearch.xpack.esql.core.type.DataType;
16-
import org.elasticsearch.xpack.esql.expression.SurrogateExpression;
16+
import org.elasticsearch.xpack.esql.expression.OnlySurrogateExpression;
1717
import org.elasticsearch.xpack.esql.expression.function.Example;
1818
import org.elasticsearch.xpack.esql.expression.function.FunctionAppliesTo;
1919
import org.elasticsearch.xpack.esql.expression.function.FunctionAppliesToLifecycle;
@@ -30,7 +30,7 @@
3030
/**
3131
* Similar to {@link StdDev}, but it is used to calculate the standard deviation over a time series of values from the given field.
3232
*/
33-
public class StddevOverTime extends TimeSeriesAggregateFunction implements SurrogateExpression, ToAggregator {
33+
public class StddevOverTime extends TimeSeriesAggregateFunction implements OnlySurrogateExpression, ToAggregator {
3434
@FunctionInfo(
3535
returnType = "double",
3636
description = "Calculates the population standard deviation over time of a numeric field.",

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/VarianceOverTime.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
import org.elasticsearch.xpack.esql.core.tree.NodeInfo;
1414
import org.elasticsearch.xpack.esql.core.tree.Source;
1515
import org.elasticsearch.xpack.esql.core.type.DataType;
16-
import org.elasticsearch.xpack.esql.expression.SurrogateExpression;
16+
import org.elasticsearch.xpack.esql.expression.OnlySurrogateExpression;
1717
import org.elasticsearch.xpack.esql.expression.function.Example;
1818
import org.elasticsearch.xpack.esql.expression.function.FunctionAppliesTo;
1919
import org.elasticsearch.xpack.esql.expression.function.FunctionAppliesToLifecycle;
@@ -30,7 +30,7 @@
3030
/**
3131
* Similar to {@link Variance}, but it is used to calculate the variance over a time series of values from the given field.
3232
*/
33-
public class VarianceOverTime extends TimeSeriesAggregateFunction implements SurrogateExpression, ToAggregator {
33+
public class VarianceOverTime extends TimeSeriesAggregateFunction implements OnlySurrogateExpression, ToAggregator {
3434
@FunctionInfo(
3535
returnType = "double",
3636
description = "Calculates the population variance over time of a numeric field.",

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/Score.java

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -59,11 +59,7 @@ public Score(
5959
description = "Boolean expression that contains full text function(s) to be scored."
6060
) Expression scorableQuery
6161
) {
62-
this(source, List.of(scorableQuery));
63-
}
64-
65-
protected Score(Source source, List<Expression> children) {
66-
super(source, children);
62+
super(source, List.of(scorableQuery));
6763
}
6864

6965
@Override
@@ -73,7 +69,7 @@ public DataType dataType() {
7369

7470
@Override
7571
public Expression replaceChildren(List<Expression> newChildren) {
76-
return new Score(source(), newChildren);
72+
return new Score(source(), newChildren.getFirst());
7773
}
7874

7975
@Override
@@ -95,12 +91,26 @@ public EvalOperator.ExpressionEvaluator.Factory toEvaluator(EvaluatorMapper.ToEv
9591
@Override
9692
public void writeTo(StreamOutput out) throws IOException {
9793
source().writeTo(out);
98-
out.writeNamedWriteableCollection(this.children());
94+
out.writeOptionalNamedWriteable(children().getFirst());
9995
}
10096

10197
private static Expression readFrom(StreamInput in) throws IOException {
10298
Source source = Source.readFrom((PlanStreamInput) in);
99+
/*
100+
* This is not truly optional. But when the SCORE scalar was originally
101+
* created we serialized it with this pair:
102+
*
103+
* in.readOptionalNamedWriteable(Expression.class);
104+
* out.writeNamedWriteableCollection(this.children());
105+
*
106+
* This pair *is* compatible if we send a single element list. Single
107+
* element lists are serialized as `0x01 <name> <body>`. That's also
108+
* how optional named writeables are serialized.
109+
*/
103110
Expression query = in.readOptionalNamedWriteable(Expression.class);
111+
if (query == null) {
112+
throw new IllegalStateException("query isn't really optional");
113+
}
104114
return new Score(source, query);
105115
}
106116

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
import org.elasticsearch.xpack.esql.core.tree.NodeInfo;
1515
import org.elasticsearch.xpack.esql.core.tree.Source;
1616
import org.elasticsearch.xpack.esql.core.type.DataType;
17-
import org.elasticsearch.xpack.esql.expression.SurrogateExpression;
17+
import org.elasticsearch.xpack.esql.expression.OnlySurrogateExpression;
1818
import org.elasticsearch.xpack.esql.expression.function.Example;
1919
import org.elasticsearch.xpack.esql.expression.function.FunctionInfo;
2020
import org.elasticsearch.xpack.esql.expression.function.OptionalArgument;
@@ -43,7 +43,7 @@
4343
* </ul>
4444
*/
4545

46-
public class ToIntegerSurrogate extends EsqlScalarFunction implements SurrogateExpression, OptionalArgument, ConvertFunction {
46+
public class ToIntegerSurrogate extends EsqlScalarFunction implements OnlySurrogateExpression, OptionalArgument, ConvertFunction {
4747

4848
private final Expression field;
4949
private final Expression base;

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
import org.elasticsearch.xpack.esql.core.tree.NodeInfo;
1717
import org.elasticsearch.xpack.esql.core.tree.Source;
1818
import org.elasticsearch.xpack.esql.core.type.DataType;
19-
import org.elasticsearch.xpack.esql.expression.SurrogateExpression;
19+
import org.elasticsearch.xpack.esql.expression.OnlySurrogateExpression;
2020
import org.elasticsearch.xpack.esql.expression.function.Example;
2121
import org.elasticsearch.xpack.esql.expression.function.FunctionInfo;
2222
import org.elasticsearch.xpack.esql.expression.function.MapParam;
@@ -62,7 +62,7 @@
6262
* expose a single method to users.
6363
* </p>
6464
*/
65-
public class ToIp extends EsqlScalarFunction implements SurrogateExpression, OptionalArgument, ConvertFunction {
65+
public class ToIp extends EsqlScalarFunction implements OnlySurrogateExpression, OptionalArgument, ConvertFunction {
6666
private static final String LEADING_ZEROS = "leading_zeros";
6767
public static final Map<String, DataType> ALLOWED_OPTIONS = Map.ofEntries(Map.entry(LEADING_ZEROS, KEYWORD));
6868

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
import org.elasticsearch.xpack.esql.core.tree.NodeInfo;
1515
import org.elasticsearch.xpack.esql.core.tree.Source;
1616
import org.elasticsearch.xpack.esql.core.type.DataType;
17-
import org.elasticsearch.xpack.esql.expression.SurrogateExpression;
17+
import org.elasticsearch.xpack.esql.expression.OnlySurrogateExpression;
1818
import org.elasticsearch.xpack.esql.expression.function.Example;
1919
import org.elasticsearch.xpack.esql.expression.function.FunctionInfo;
2020
import org.elasticsearch.xpack.esql.expression.function.OptionalArgument;
@@ -46,7 +46,7 @@
4646
* </ul>
4747
*/
4848

49-
public class ToLongSurrogate extends EsqlScalarFunction implements SurrogateExpression, OptionalArgument, ConvertFunction {
49+
public class ToLongSurrogate extends EsqlScalarFunction implements OnlySurrogateExpression, OptionalArgument, ConvertFunction {
5050

5151
private final Expression field;
5252
private final Expression base;

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,22 @@ public String getWriteableName() {
210210
return ENTRY.name;
211211
}
212212

213+
Expression value() {
214+
return value;
215+
}
216+
217+
Expression origin() {
218+
return origin;
219+
}
220+
221+
Expression scale() {
222+
return scale;
223+
}
224+
225+
Expression options() {
226+
return options;
227+
}
228+
213229
@Override
214230
protected TypeResolution resolveType() {
215231
if (childrenResolved() == false) {

0 commit comments

Comments
 (0)