Skip to content

Commit ae90dd8

Browse files
alex-spiesalex-spies
committed
Handle null values in profile's minimum version
1 parent 3dd0261 commit ae90dd8

File tree

3 files changed

+41
-16
lines changed

3 files changed

+41
-16
lines changed

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryResponse.java‎

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,8 @@ public Iterator<? extends ToXContent> toXContentChunked(ToXContent.Params params
332332
content.add(ChunkedToXContentHelper.array("drivers", profile.drivers.iterator(), params));
333333
content.add(ChunkedToXContentHelper.array("plans", profile.plans.iterator()));
334334
content.add(ChunkedToXContentHelper.chunk((b, p) -> {
335-
b.field("minimumTransportVersion", profile.minimumVersion().id());
335+
TransportVersion minimumVersion = profile.minimumVersion();
336+
b.field("minimumTransportVersion", minimumVersion == null ? null : minimumVersion.id());
336337
return b;
337338
}));
338339
content.add(ChunkedToXContentHelper.endObject());
@@ -451,7 +452,7 @@ public static Profile readFrom(StreamInput in) throws IOException {
451452
in.getTransportVersion().supports(ESQL_PROFILE_INCLUDE_PLAN)
452453
? in.readCollectionAsImmutableList(PlanProfile::readFrom)
453454
: List.of(),
454-
in.getTransportVersion().supports(ESQL_USE_MINIMUM_VERSION_FOR_ENRICH_RESOLUTION) ? TransportVersion.readVersion(in) : null
455+
in.getTransportVersion().supports(ESQL_USE_MINIMUM_VERSION_FOR_ENRICH_RESOLUTION) ? readOptionalTransportVersion(in) : null
455456
);
456457
}
457458

@@ -462,7 +463,25 @@ public void writeTo(StreamOutput out) throws IOException {
462463
out.writeCollection(plans);
463464
}
464465
if (out.getTransportVersion().supports(ESQL_USE_MINIMUM_VERSION_FOR_ENRICH_RESOLUTION)) {
465-
TransportVersion.writeVersion(minimumVersion, out);
466+
// When retrieving the profile from an older node, there might be no minimum version attached.
467+
// When writing the profile somewhere else, we need to handle the case that the minimum version is null.
468+
writeOptionalTransportVersion(minimumVersion, out);
469+
}
470+
}
471+
472+
private static TransportVersion readOptionalTransportVersion(StreamInput in) throws IOException {
473+
if (in.readBoolean()) {
474+
return TransportVersion.readVersion(in);
475+
}
476+
return null;
477+
}
478+
479+
private static void writeOptionalTransportVersion(@Nullable TransportVersion version, StreamOutput out) throws IOException {
480+
if (version == null) {
481+
out.writeBoolean(false);
482+
} else {
483+
out.writeBoolean(true);
484+
TransportVersion.writeVersion(version, out);
466485
}
467486
}
468487
}

‎x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryResponseProfileTests.java‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
package org.elasticsearch.xpack.esql.action;
99

10+
import org.elasticsearch.TransportVersion;
1011
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
1112
import org.elasticsearch.common.io.stream.Writeable;
1213
import org.elasticsearch.compute.operator.AbstractPageMappingOperator;
@@ -19,8 +20,6 @@
1920

2021
import java.util.List;
2122

22-
import static org.elasticsearch.xpack.esql.EsqlTestUtils.randomMinimumVersion;
23-
2423
public class EsqlQueryResponseProfileTests extends AbstractWireSerializingTestCase<EsqlQueryResponse.Profile> {
2524
@Override
2625
protected Writeable.Reader<EsqlQueryResponse.Profile> instanceReader() {
@@ -39,9 +38,9 @@ protected EsqlQueryResponse.Profile mutateInstance(EsqlQueryResponse.Profile ins
3938
var minimumVersion = instance.minimumVersion();
4039

4140
switch (between(0, 2)) {
42-
case 0 -> drivers = randomValueOtherThan(drivers, this::randomDriverProfiles);
43-
case 1 -> plans = randomValueOtherThan(plans, this::randomPlanProfiles);
44-
case 2 -> minimumVersion = randomValueOtherThan(minimumVersion, EsqlTestUtils::randomMinimumVersion);
41+
case 0 -> drivers = randomValueOtherThan(drivers, EsqlQueryResponseProfileTests::randomDriverProfiles);
42+
case 1 -> plans = randomValueOtherThan(plans, EsqlQueryResponseProfileTests::randomPlanProfiles);
43+
case 2 -> minimumVersion = randomValueOtherThan(minimumVersion, EsqlQueryResponseProfileTests::randomMinimumVersion);
4544
}
4645
return new EsqlQueryResponse.Profile(drivers, plans, minimumVersion);
4746
}
@@ -51,7 +50,7 @@ protected NamedWriteableRegistry getNamedWriteableRegistry() {
5150
return new NamedWriteableRegistry(List.of(AbstractPageMappingOperator.Status.ENTRY));
5251
}
5352

54-
private List<DriverProfile> randomDriverProfiles() {
53+
private static List<DriverProfile> randomDriverProfiles() {
5554
return randomList(
5655
10,
5756
() -> new DriverProfile(
@@ -63,20 +62,20 @@ private List<DriverProfile> randomDriverProfiles() {
6362
randomNonNegativeLong(),
6463
randomNonNegativeLong(),
6564
randomNonNegativeLong(),
66-
randomList(10, this::randomOperatorStatus),
65+
randomList(10, EsqlQueryResponseProfileTests::randomOperatorStatus),
6766
DriverSleeps.empty()
6867
)
6968
);
7069
}
7170

72-
private List<PlanProfile> randomPlanProfiles() {
71+
private static List<PlanProfile> randomPlanProfiles() {
7372
return randomList(
7473
10,
7574
() -> new PlanProfile(randomIdentifier(), randomIdentifier(), randomIdentifier(), randomAlphanumericOfLength(1024))
7675
);
7776
}
7877

79-
private OperatorStatus randomOperatorStatus() {
78+
private static OperatorStatus randomOperatorStatus() {
8079
return new OperatorStatus(
8180
randomAlphaOfLength(4),
8281
randomBoolean()
@@ -89,4 +88,8 @@ private OperatorStatus randomOperatorStatus() {
8988
: null
9089
);
9190
}
91+
92+
public static TransportVersion randomMinimumVersion() {
93+
return randomBoolean() ? null : EsqlTestUtils.randomMinimumVersion();
94+
}
9295
}

‎x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryResponseTests.java‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import org.elasticsearch.common.bytes.BytesReference;
1616
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
1717
import org.elasticsearch.common.io.stream.Writeable;
18+
import org.elasticsearch.common.logging.LoggerMessageFormat;
1819
import org.elasticsearch.common.unit.ByteSizeValue;
1920
import org.elasticsearch.common.util.BigArrays;
2021
import org.elasticsearch.common.util.MockBigArrays;
@@ -1013,6 +1014,8 @@ private EsqlQueryResponse simple(boolean columnar, boolean async) {
10131014
}
10141015

10151016
public void testProfileXContent() {
1017+
TransportVersion minimumVersion = EsqlQueryResponseProfileTests.randomMinimumVersion();
1018+
10161019
try (
10171020
EsqlQueryResponse response = new EsqlQueryResponse(
10181021
List.of(new ColumnInfoImpl("foo", "integer", null)),
@@ -1035,7 +1038,7 @@ public void testProfileXContent() {
10351038
)
10361039
),
10371040
List.of(new PlanProfile("test", "elasticsearch", "node-1", "plan tree")),
1038-
new TransportVersion(1234)
1041+
minimumVersion
10391042
),
10401043
false,
10411044
false,
@@ -1044,7 +1047,7 @@ public void testProfileXContent() {
10441047
null
10451048
);
10461049
) {
1047-
assertThat(Strings.toString(wrapAsToXContent(response), true, false), equalTo("""
1050+
assertThat(Strings.toString(wrapAsToXContent(response), true, false), equalTo(LoggerMessageFormat.format("""
10481051
{
10491052
"documents_found" : 10,
10501053
"values_loaded" : 100,
@@ -1101,9 +1104,9 @@ public void testProfileXContent() {
11011104
"plan" : "plan tree"
11021105
}
11031106
],
1104-
"minimumTransportVersion" : 1234
1107+
"minimumTransportVersion" : {}
11051108
}
1106-
}"""));
1109+
}""", minimumVersion == null ? "null" : minimumVersion.id())));
11071110
}
11081111
}
11091112

0 commit comments

Comments
 (0)