Skip to content

Commit 6a734e6

Browse files
alex-spiesnik9000
andauthored
ESQL: Handle release of 9.2 in test (#137070) (#137436)
I'd made a mistake in #136327 when writing the test for fetching fields that the release of 9.2.0 revealed. This fixes it and adds one more test that we needed from #136327. (cherry picked from commit cd3839c) # Conflicts: # server/src/main/resources/transport/upper_bounds/9.3.csv # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlResolveFieldsResponse.java Co-authored-by: Nik Everett <[email protected]>
1 parent 69e2512 commit 6a734e6

File tree

6 files changed

+118
-24
lines changed

6 files changed

+118
-24
lines changed
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
9204000,9185005
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
min_transport_version,9185004
1+
esql_resolve_fields_response_used,9185005
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
min_transport_version,9202000
1+
esql_resolve_fields_response_used,9204000

x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/DataType.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -976,8 +976,13 @@ Builder underConstruction() {
976976
}
977977
}
978978

979-
private static class DataTypesTransportVersions {
979+
public static class DataTypesTransportVersions {
980980

981+
/**
982+
* The first transport version after the PR that introduced geotile/geohash/geohex, resp.
983+
* after 9.1. We didn't require transport versions at that point in time, as geotile/hash/hex require
984+
* using specific functions to even occur in query plans.
985+
*/
981986
public static final TransportVersion INDEX_SOURCE = TransportVersion.fromName("index_source");
982987

983988
public static final TransportVersion ESQL_DENSE_VECTOR_CREATED_VERSION = TransportVersion.fromName(

x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/AllSupportedFieldsTestCase.java

Lines changed: 94 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,10 @@
4444
import static org.elasticsearch.test.ListMatcher.matchesList;
4545
import static org.elasticsearch.test.MapMatcher.assertMap;
4646
import static org.elasticsearch.test.MapMatcher.matchesMap;
47-
import static org.elasticsearch.xpack.esql.action.EsqlResolveFieldsResponse.RESOLVE_FIELDS_RESPONSE_CREATED_TV;
47+
import static org.elasticsearch.xpack.esql.action.EsqlResolveFieldsResponse.RESOLVE_FIELDS_RESPONSE_USED_TV;
48+
import static org.elasticsearch.xpack.esql.core.type.DataType.DataTypesTransportVersions.ESQL_AGGREGATE_METRIC_DOUBLE_CREATED_VERSION;
49+
import static org.elasticsearch.xpack.esql.core.type.DataType.DataTypesTransportVersions.ESQL_DENSE_VECTOR_CREATED_VERSION;
50+
import static org.elasticsearch.xpack.esql.core.type.DataType.DataTypesTransportVersions.INDEX_SOURCE;
4851
import static org.hamcrest.Matchers.any;
4952
import static org.hamcrest.Matchers.anyOf;
5053
import static org.hamcrest.Matchers.containsString;
@@ -71,8 +74,6 @@
7174
public class AllSupportedFieldsTestCase extends ESRestTestCase {
7275
private static final Logger logger = LogManager.getLogger(FieldExtractorTestCase.class);
7376

74-
private static final TransportVersion INDEX_SOURCE = TransportVersion.fromName("index_source");
75-
7677
@Rule(order = Integer.MIN_VALUE)
7778
public ProfileLogger profileLogger = new ProfileLogger();
7879

@@ -329,6 +330,76 @@ public final void testFetchDenseVector() throws IOException {
329330
assertMap(indexToRow(columns, values), expectedAllValues);
330331
}
331332

333+
/**
334+
* Tests fetching {@code aggregate_metric_double} if possible. Uses the {@code dense_vector_agg_metric_double_if_fns}
335+
* work around if required.
336+
*/
337+
public final void testFetchAggregateMetricDouble() throws IOException {
338+
Map<String, Object> response;
339+
try {
340+
String request = """
341+
| EVAL strjunk = TO_STRING(f_aggregate_metric_double)
342+
| KEEP _index, f_aggregate_metric_double
343+
| LIMIT 1000
344+
""";
345+
if (denseVectorAggMetricDoubleIfVersion() == false) {
346+
request = """
347+
| EVAL junk = TO_AGGREGATE_METRIC_DOUBLE(1) // workaround to enable fetching aggregate_metric_double
348+
""" + request;
349+
}
350+
response = esql(request);
351+
if ((Boolean) response.get("is_partial")) {
352+
Map<?, ?> clusters = (Map<?, ?>) response.get("_clusters");
353+
Map<?, ?> details = (Map<?, ?>) clusters.get("details");
354+
355+
boolean foundError = false;
356+
for (Map.Entry<?, ?> cluster : details.entrySet()) {
357+
String failures = cluster.getValue().toString();
358+
if (denseVectorAggMetricDoubleIfFns()) {
359+
throw new AssertionError("should correctly fetch the aggregate_metric_double: " + failures);
360+
}
361+
foundError |= failures.contains("doesn't understand data type [AGGREGATE_METRIC_DOUBLE]");
362+
}
363+
assertTrue("didn't find errors: " + details, foundError);
364+
return;
365+
}
366+
} catch (ResponseException e) {
367+
if (denseVectorAggMetricDoubleIfFns()) {
368+
throw new AssertionError("should correctly fetch the aggregate_metric_double", e);
369+
}
370+
assertThat(
371+
"old version should fail with this error",
372+
EntityUtils.toString(e.getResponse().getEntity()),
373+
anyOf(
374+
containsString("Unknown function [TO_AGGREGATE_METRIC_DOUBLE]"),
375+
containsString("Cannot use field [f_aggregate_metric_double] with unsupported type"),
376+
containsString("doesn't understand data type [AGGREGATE_METRIC_DOUBLE]")
377+
)
378+
);
379+
// Failure is expected and fine
380+
return;
381+
}
382+
List<?> columns = (List<?>) response.get("columns");
383+
List<?> values = (List<?>) response.get("values");
384+
385+
MapMatcher expectedColumns = matchesMap().entry("f_aggregate_metric_double", "aggregate_metric_double").entry("_index", "keyword");
386+
assertMap(nameToType(columns), expectedColumns);
387+
388+
MapMatcher expectedAllValues = matchesMap();
389+
for (Map.Entry<String, NodeInfo> e : expectedIndices().entrySet()) {
390+
String indexName = e.getKey();
391+
NodeInfo nodeInfo = e.getValue();
392+
MapMatcher expectedValues = matchesMap();
393+
expectedValues = expectedValues.entry(
394+
"f_aggregate_metric_double",
395+
"{\"min\":-302.5,\"max\":702.3,\"sum\":200.0,\"value_count\":25}"
396+
);
397+
expectedValues = expectedValues.entry("_index", indexName);
398+
expectedAllValues = expectedAllValues.entry(indexName, expectedValues);
399+
}
400+
assertMap(indexToRow(columns, values), expectedAllValues);
401+
}
402+
332403
private Map<String, Object> esql(String query) throws IOException {
333404
Request request = new Request("POST", "_query");
334405
XContentBuilder body = JsonXContent.contentBuilder().startObject();
@@ -473,21 +544,15 @@ private Matcher<?> expectedValue(DataType type, NodeInfo nodeInfo) throws IOExce
473544
case GEO_SHAPE -> equalTo("POINT (-71.34 41.12)");
474545
case NULL -> nullValue();
475546
case AGGREGATE_METRIC_DOUBLE -> {
476-
/*
477-
* We need both AGGREGATE_METRIC_DOUBLE_CREATED and RESOLVE_FIELDS_RESPONSE_CREATED_TV
478-
* but RESOLVE_FIELDS_RESPONSE_CREATED_TV came last so it's enough to check just it.
479-
*/
480-
if (minVersion().supports(RESOLVE_FIELDS_RESPONSE_CREATED_TV) == false) {
547+
if (minVersion().supports(RESOLVE_FIELDS_RESPONSE_USED_TV) == false
548+
|| minVersion().supports(ESQL_AGGREGATE_METRIC_DOUBLE_CREATED_VERSION) == false) {
481549
yield nullValue();
482550
}
483551
yield equalTo("{\"min\":-302.5,\"max\":702.3,\"sum\":200.0,\"value_count\":25}");
484552
}
485553
case DENSE_VECTOR -> {
486-
/*
487-
* We need both DENSE_VECTOR_CREATED and RESOLVE_FIELDS_RESPONSE_CREATED_TV
488-
* but RESOLVE_FIELDS_RESPONSE_CREATED_TV came last so it's enough to check just it.
489-
*/
490-
if (minVersion().supports(RESOLVE_FIELDS_RESPONSE_CREATED_TV) == false) {
554+
if (minVersion().supports(RESOLVE_FIELDS_RESPONSE_USED_TV) == false
555+
|| minVersion().supports(ESQL_DENSE_VECTOR_CREATED_VERSION) == false) {
491556
yield nullValue();
492557
}
493558
yield equalTo(List.of(0.5, 10.0, 5.9999995));
@@ -572,15 +637,24 @@ private Matcher<String> expectedType(DataType type) throws IOException {
572637
case BYTE, SHORT -> equalTo("integer");
573638
case HALF_FLOAT, SCALED_FLOAT, FLOAT -> equalTo("double");
574639
case NULL -> equalTo("keyword");
575-
case AGGREGATE_METRIC_DOUBLE, DENSE_VECTOR -> {
576-
/*
577-
* We need both <type_name>_CREATED and RESOLVE_FIELDS_RESPONSE_CREATED_TV
578-
* but RESOLVE_FIELDS_RESPONSE_CREATED_TV came last so it's enough to check just it.
579-
*/
580-
if (minVersion().supports(RESOLVE_FIELDS_RESPONSE_CREATED_TV) == false) {
640+
case AGGREGATE_METRIC_DOUBLE -> {
641+
// RESOLVE_FIELDS_RESPONSE_USED_TV is newer and technically sufficient to check.
642+
// We also check for ESQL_AGGREGATE_METRIC_DOUBLE_CREATED_VERSION for clarity.
643+
// Future data types added here should only require the TV when they were created,
644+
// because it will be after RESOLVE_FIELDS_RESPONSE_USED_TV.
645+
if (minVersion().supports(RESOLVE_FIELDS_RESPONSE_USED_TV) == false
646+
|| minVersion().supports(ESQL_AGGREGATE_METRIC_DOUBLE_CREATED_VERSION) == false) {
647+
yield equalTo("unsupported");
648+
}
649+
yield equalTo("aggregate_metric_double");
650+
}
651+
case DENSE_VECTOR -> {
652+
logger.error("ADFDAFAF " + minVersion());
653+
if (minVersion().supports(RESOLVE_FIELDS_RESPONSE_USED_TV) == false
654+
|| minVersion().supports(ESQL_DENSE_VECTOR_CREATED_VERSION) == false) {
581655
yield equalTo("unsupported");
582656
}
583-
yield equalTo(type.esType());
657+
yield equalTo("dense_vector");
584658
}
585659
default -> equalTo(type.esType());
586660
};

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

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,28 @@
1313
import org.elasticsearch.common.io.stream.StreamInput;
1414
import org.elasticsearch.common.io.stream.StreamOutput;
1515
import org.elasticsearch.core.Nullable;
16+
import org.elasticsearch.xpack.esql.core.type.DataType;
1617

1718
import java.io.IOException;
1819

1920
public class EsqlResolveFieldsResponse extends ActionResponse {
20-
public static final TransportVersion RESOLVE_FIELDS_RESPONSE_CREATED_TV = TransportVersion.fromName(
21+
private static final TransportVersion RESOLVE_FIELDS_RESPONSE_CREATED_TV = TransportVersion.fromName(
2122
"esql_resolve_fields_response_created"
2223
);
2324

25+
/**
26+
* Marks when we started using the minimum transport version to determine whether a data type is supported on all nodes.
27+
* This is about the coordinator - data nodes will be able to respond with the correct data as long as they're on the
28+
* transport version required for the respective data types. See {@link DataType#supportedVersion()}.
29+
* <p>
30+
* Note: this is in 9.2.1, but not 9.2.0 - in 9.2.0 we resorted to workarounds to sometimes enable {@link DataType#DENSE_VECTOR} and
31+
* {@link DataType#AGGREGATE_METRIC_DOUBLE}, even though 9.2.0 nodes already support these types.
32+
* <p>
33+
* This means that mixed clusters with a 9.2.1 coordinator and 9.2.0 data nodes will properly support these types,
34+
* but a 9.2.0 coordinator with 9.2.1+ nodes will still require the workaround.
35+
*/
36+
public static final TransportVersion RESOLVE_FIELDS_RESPONSE_USED_TV = TransportVersion.fromName("esql_resolve_fields_response_used");
37+
2438
private final FieldCapabilitiesResponse caps;
2539
private final TransportVersion minTransportVersion;
2640

0 commit comments

Comments
 (0)