From 6b254ed78dd0a25cb0bc5c4f80f6ddf93134ddd7 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Thu, 23 Oct 2025 17:14:14 -0400 Subject: [PATCH 1/2] ESQL: Handle release of 9.2 in test 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. --- .../esql_resolve_fields_response_used.csv | 1 + .../resources/transport/upper_bounds/9.2.csv | 2 +- .../resources/transport/upper_bounds/9.3.csv | 2 +- .../xpack/esql/core/type/DataType.java | 2 +- .../qa/rest/AllSupportedFieldsTestCase.java | 107 +++++++++++++++--- .../action/EsqlResolveFieldsResponse.java | 4 +- 6 files changed, 96 insertions(+), 22 deletions(-) create mode 100644 server/src/main/resources/transport/definitions/referable/esql_resolve_fields_response_used.csv diff --git a/server/src/main/resources/transport/definitions/referable/esql_resolve_fields_response_used.csv b/server/src/main/resources/transport/definitions/referable/esql_resolve_fields_response_used.csv new file mode 100644 index 0000000000000..9f305d6018a4d --- /dev/null +++ b/server/src/main/resources/transport/definitions/referable/esql_resolve_fields_response_used.csv @@ -0,0 +1 @@ +9202000,9185004 diff --git a/server/src/main/resources/transport/upper_bounds/9.2.csv b/server/src/main/resources/transport/upper_bounds/9.2.csv index 803cdb78f8651..80a0b02131480 100644 --- a/server/src/main/resources/transport/upper_bounds/9.2.csv +++ b/server/src/main/resources/transport/upper_bounds/9.2.csv @@ -1 +1 @@ -initial_9.2.1,9185003 +esql_resolve_fields_response_used,9185004 diff --git a/server/src/main/resources/transport/upper_bounds/9.3.csv b/server/src/main/resources/transport/upper_bounds/9.3.csv index 238cf894d79b4..afda7bce80d84 100644 --- a/server/src/main/resources/transport/upper_bounds/9.3.csv +++ b/server/src/main/resources/transport/upper_bounds/9.3.csv @@ -1 +1 @@ -esql_lookup_join_full_text_function,9201000 +esql_resolve_fields_response_used,9202000 diff --git a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/DataType.java b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/DataType.java index b05d6784c072f..eefed35eb6f31 100644 --- a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/DataType.java +++ b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/DataType.java @@ -976,7 +976,7 @@ Builder underConstruction() { } } - private static class DataTypesTransportVersions { + public static class DataTypesTransportVersions { public static final TransportVersion INDEX_SOURCE = TransportVersion.fromName("index_source"); diff --git a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/AllSupportedFieldsTestCase.java b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/AllSupportedFieldsTestCase.java index 0617f03402730..4df6ca088c157 100644 --- a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/AllSupportedFieldsTestCase.java +++ b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/AllSupportedFieldsTestCase.java @@ -44,7 +44,9 @@ import static org.elasticsearch.test.ListMatcher.matchesList; import static org.elasticsearch.test.MapMatcher.assertMap; import static org.elasticsearch.test.MapMatcher.matchesMap; -import static org.elasticsearch.xpack.esql.action.EsqlResolveFieldsResponse.RESOLVE_FIELDS_RESPONSE_CREATED_TV; +import static org.elasticsearch.xpack.esql.action.EsqlResolveFieldsResponse.RESOLVE_FIELDS_RESPONSE_USED_TV; +import static org.elasticsearch.xpack.esql.core.type.DataType.DataTypesTransportVersions.ESQL_AGGREGATE_METRIC_DOUBLE_CREATED_VERSION; +import static org.elasticsearch.xpack.esql.core.type.DataType.DataTypesTransportVersions.ESQL_DENSE_VECTOR_CREATED_VERSION; import static org.hamcrest.Matchers.any; import static org.hamcrest.Matchers.anyOf; import static org.hamcrest.Matchers.containsString; @@ -329,6 +331,76 @@ public final void testFetchDenseVector() throws IOException { assertMap(indexToRow(columns, values), expectedAllValues); } + /** + * Tests fetching {@code aggregate_metric_double} if possible. Uses the {@code dense_vector_agg_metric_double_if_fns} + * work around if required. + */ + public final void testFetchAggregateMetricDouble() throws IOException { + Map response; + try { + String request = """ + | EVAL strjunk = TO_STRING(f_aggregate_metric_double) + | KEEP _index, f_aggregate_metric_double + | LIMIT 1000 + """; + if (denseVectorAggMetricDoubleIfVersion() == false) { + request = """ + | EVAL junk = TO_AGGREGATE_METRIC_DOUBLE(1) // workaround to enable fetching aggregate_metric_double + """ + request; + } + response = esql(request); + if ((Boolean) response.get("is_partial")) { + Map clusters = (Map) response.get("_clusters"); + Map details = (Map) clusters.get("details"); + + boolean foundError = false; + for (Map.Entry cluster : details.entrySet()) { + String failures = cluster.getValue().toString(); + if (denseVectorAggMetricDoubleIfFns()) { + throw new AssertionError("should correctly fetch the aggregate_metric_double: " + failures); + } + foundError |= failures.contains("doesn't understand data type [AGGREGATE_METRIC_DOUBLE]"); + } + assertTrue("didn't find errors: " + details, foundError); + return; + } + } catch (ResponseException e) { + if (denseVectorAggMetricDoubleIfFns()) { + throw new AssertionError("should correctly fetch the aggregate_metric_double", e); + } + assertThat( + "old version should fail with this error", + EntityUtils.toString(e.getResponse().getEntity()), + anyOf( + containsString("Unknown function [TO_AGGREGATE_METRIC_DOUBLE]"), + containsString("Cannot use field [f_aggregate_metric_double] with unsupported type"), + containsString("doesn't understand data type [AGGREGATE_METRIC_DOUBLE]") + ) + ); + // Failure is expected and fine + return; + } + List columns = (List) response.get("columns"); + List values = (List) response.get("values"); + + MapMatcher expectedColumns = matchesMap().entry("f_aggregate_metric_double", "aggregate_metric_double").entry("_index", "keyword"); + assertMap(nameToType(columns), expectedColumns); + + MapMatcher expectedAllValues = matchesMap(); + for (Map.Entry e : expectedIndices().entrySet()) { + String indexName = e.getKey(); + NodeInfo nodeInfo = e.getValue(); + MapMatcher expectedValues = matchesMap(); + expectedValues = expectedValues.entry( + "f_aggregate_metric_double", + "{\"min\":-302.5,\"max\":702.3,\"sum\":200.0,\"value_count\":25}" + ); + expectedValues = expectedValues.entry("_index", indexName); + expectedAllValues = expectedAllValues.entry(indexName, expectedValues); + } + assertMap(indexToRow(columns, values), expectedAllValues); + } + private Map esql(String query) throws IOException { Request request = new Request("POST", "_query"); XContentBuilder body = JsonXContent.contentBuilder().startObject(); @@ -473,21 +545,15 @@ private Matcher expectedValue(DataType type, NodeInfo nodeInfo) throws IOExce case GEO_SHAPE -> equalTo("POINT (-71.34 41.12)"); case NULL -> nullValue(); case AGGREGATE_METRIC_DOUBLE -> { - /* - * We need both AGGREGATE_METRIC_DOUBLE_CREATED and RESOLVE_FIELDS_RESPONSE_CREATED_TV - * but RESOLVE_FIELDS_RESPONSE_CREATED_TV came last so it's enough to check just it. - */ - if (minVersion().supports(RESOLVE_FIELDS_RESPONSE_CREATED_TV) == false) { + if (minVersion().supports(RESOLVE_FIELDS_RESPONSE_USED_TV) == false + || minVersion().supports(ESQL_AGGREGATE_METRIC_DOUBLE_CREATED_VERSION) == false) { yield nullValue(); } yield equalTo("{\"min\":-302.5,\"max\":702.3,\"sum\":200.0,\"value_count\":25}"); } case DENSE_VECTOR -> { - /* - * We need both DENSE_VECTOR_CREATED and RESOLVE_FIELDS_RESPONSE_CREATED_TV - * but RESOLVE_FIELDS_RESPONSE_CREATED_TV came last so it's enough to check just it. - */ - if (minVersion().supports(RESOLVE_FIELDS_RESPONSE_CREATED_TV) == false) { + if (minVersion().supports(RESOLVE_FIELDS_RESPONSE_USED_TV) == false + || minVersion().supports(ESQL_DENSE_VECTOR_CREATED_VERSION) == false) { yield nullValue(); } yield equalTo(List.of(0.5, 10.0, 5.9999995)); @@ -572,15 +638,20 @@ private Matcher expectedType(DataType type) throws IOException { case BYTE, SHORT -> equalTo("integer"); case HALF_FLOAT, SCALED_FLOAT, FLOAT -> equalTo("double"); case NULL -> equalTo("keyword"); - case AGGREGATE_METRIC_DOUBLE, DENSE_VECTOR -> { - /* - * We need both _CREATED and RESOLVE_FIELDS_RESPONSE_CREATED_TV - * but RESOLVE_FIELDS_RESPONSE_CREATED_TV came last so it's enough to check just it. - */ - if (minVersion().supports(RESOLVE_FIELDS_RESPONSE_CREATED_TV) == false) { + case AGGREGATE_METRIC_DOUBLE -> { + if (minVersion().supports(RESOLVE_FIELDS_RESPONSE_USED_TV) == false + || minVersion().supports(ESQL_AGGREGATE_METRIC_DOUBLE_CREATED_VERSION) == false) { + yield equalTo("unsupported"); + } + yield equalTo("aggregate_metric_double"); + } + case DENSE_VECTOR -> { + logger.error("ADFDAFAF " + minVersion()); + if (minVersion().supports(RESOLVE_FIELDS_RESPONSE_USED_TV) == false + || minVersion().supports(ESQL_DENSE_VECTOR_CREATED_VERSION) == false) { yield equalTo("unsupported"); } - yield equalTo(type.esType()); + yield equalTo("dense_vector"); } default -> equalTo(type.esType()); }; diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlResolveFieldsResponse.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlResolveFieldsResponse.java index 45dcfda444354..92a412b9c2621 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlResolveFieldsResponse.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlResolveFieldsResponse.java @@ -17,10 +17,12 @@ import java.io.IOException; public class EsqlResolveFieldsResponse extends ActionResponse { - public static final TransportVersion RESOLVE_FIELDS_RESPONSE_CREATED_TV = TransportVersion.fromName( + private static final TransportVersion RESOLVE_FIELDS_RESPONSE_CREATED_TV = TransportVersion.fromName( "esql_resolve_fields_response_created" ); + public static final TransportVersion RESOLVE_FIELDS_RESPONSE_USED_TV = TransportVersion.fromName("esql_resolve_fields_response_used"); + private final FieldCapabilitiesResponse caps; private final TransportVersion minTransportVersion; From 48c373c7b1be1653492f6fba13c89a9dda6abc0a Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Fri, 24 Oct 2025 16:28:53 +0200 Subject: [PATCH 2/2] Document relevant transport versions better --- .../elasticsearch/xpack/esql/core/type/DataType.java | 5 +++++ .../esql/qa/rest/AllSupportedFieldsTestCase.java | 7 +++++-- .../xpack/esql/action/EsqlResolveFieldsResponse.java | 12 ++++++++++++ 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/DataType.java b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/DataType.java index eefed35eb6f31..bff8ab8c7037f 100644 --- a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/DataType.java +++ b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/DataType.java @@ -978,6 +978,11 @@ Builder underConstruction() { public static class DataTypesTransportVersions { + /** + * The first transport version after the PR that introduced geotile/geohash/geohex, resp. + * after 9.1. We didn't require transport versions at that point in time, as geotile/hash/hex require + * using specific functions to even occur in query plans. + */ public static final TransportVersion INDEX_SOURCE = TransportVersion.fromName("index_source"); public static final TransportVersion ESQL_DENSE_VECTOR_CREATED_VERSION = TransportVersion.fromName( diff --git a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/AllSupportedFieldsTestCase.java b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/AllSupportedFieldsTestCase.java index 4df6ca088c157..c7ae359993bb4 100644 --- a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/AllSupportedFieldsTestCase.java +++ b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/AllSupportedFieldsTestCase.java @@ -47,6 +47,7 @@ import static org.elasticsearch.xpack.esql.action.EsqlResolveFieldsResponse.RESOLVE_FIELDS_RESPONSE_USED_TV; import static org.elasticsearch.xpack.esql.core.type.DataType.DataTypesTransportVersions.ESQL_AGGREGATE_METRIC_DOUBLE_CREATED_VERSION; import static org.elasticsearch.xpack.esql.core.type.DataType.DataTypesTransportVersions.ESQL_DENSE_VECTOR_CREATED_VERSION; +import static org.elasticsearch.xpack.esql.core.type.DataType.DataTypesTransportVersions.INDEX_SOURCE; import static org.hamcrest.Matchers.any; import static org.hamcrest.Matchers.anyOf; import static org.hamcrest.Matchers.containsString; @@ -73,8 +74,6 @@ public class AllSupportedFieldsTestCase extends ESRestTestCase { private static final Logger logger = LogManager.getLogger(FieldExtractorTestCase.class); - private static final TransportVersion INDEX_SOURCE = TransportVersion.fromName("index_source"); - @Rule(order = Integer.MIN_VALUE) public ProfileLogger profileLogger = new ProfileLogger(); @@ -639,6 +638,10 @@ private Matcher expectedType(DataType type) throws IOException { case HALF_FLOAT, SCALED_FLOAT, FLOAT -> equalTo("double"); case NULL -> equalTo("keyword"); case AGGREGATE_METRIC_DOUBLE -> { + // RESOLVE_FIELDS_RESPONSE_USED_TV is newer and technically sufficient to check. + // We also check for ESQL_AGGREGATE_METRIC_DOUBLE_CREATED_VERSION for clarity. + // Future data types added here should only require the TV when they were created, + // because it will be after RESOLVE_FIELDS_RESPONSE_USED_TV. if (minVersion().supports(RESOLVE_FIELDS_RESPONSE_USED_TV) == false || minVersion().supports(ESQL_AGGREGATE_METRIC_DOUBLE_CREATED_VERSION) == false) { yield equalTo("unsupported"); diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlResolveFieldsResponse.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlResolveFieldsResponse.java index 92a412b9c2621..aaef3da09e2fd 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlResolveFieldsResponse.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlResolveFieldsResponse.java @@ -13,6 +13,7 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.core.Nullable; +import org.elasticsearch.xpack.esql.core.type.DataType; import java.io.IOException; @@ -21,6 +22,17 @@ public class EsqlResolveFieldsResponse extends ActionResponse { "esql_resolve_fields_response_created" ); + /** + * Marks when we started using the minimum transport version to determine whether a data type is supported on all nodes. + * This is about the coordinator - data nodes will be able to respond with the correct data as long as they're on the + * transport version required for the respective data types. See {@link DataType#supportedVersion()}. + *

+ * 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 + * {@link DataType#AGGREGATE_METRIC_DOUBLE}, even though 9.2.0 nodes already support these types. + *

+ * This means that mixed clusters with a 9.2.1 coordinator and 9.2.0 data nodes will properly support these types, + * but a 9.2.0 coordinator with 9.2.1+ nodes will still require the workaround. + */ public static final TransportVersion RESOLVE_FIELDS_RESPONSE_USED_TV = TransportVersion.fromName("esql_resolve_fields_response_used"); private final FieldCapabilitiesResponse caps;