From 9296d5c72215c79f139d02d4986314f0559fc43c Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Tue, 30 Sep 2025 12:02:06 +0200 Subject: [PATCH 01/11] Choose a minimum transport version All the new types added in 9.2 will be considered supported by a remote node if its transport version is at least `INDEX_SOURCE`. We cannot add a new transport version and use that because it would break some queries already working on Serverless. --- .../org/elasticsearch/TransportVersions.java | 4 + .../xpack/esql/core/type/DataType.java | 150 +++++++++++------- ...atedVersion.java => SupportedVersion.java} | 28 ++-- 3 files changed, 117 insertions(+), 65 deletions(-) rename x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/{CreatedVersion.java => SupportedVersion.java} (53%) diff --git a/server/src/main/java/org/elasticsearch/TransportVersions.java b/server/src/main/java/org/elasticsearch/TransportVersions.java index 383b1f8590486..e17f6e3012d4c 100644 --- a/server/src/main/java/org/elasticsearch/TransportVersions.java +++ b/server/src/main/java/org/elasticsearch/TransportVersions.java @@ -279,6 +279,10 @@ static TransportVersion def(int id) { public static final TransportVersion STATE_PARAM_GET_SNAPSHOT = def(9_100_0_00); public static final TransportVersion PROJECT_ID_IN_SNAPSHOTS_DELETIONS_AND_REPO_CLEANUP = def(9_101_0_00); public static final TransportVersion CLUSTER_STATE_PROJECTS_SETTINGS = def(9_108_0_00); + /** + * Also used for determining when some new ESQL data types were introduced, as a workaround to then-missing proper versioning of data + * types. Do not backport without updating that workaround as well, otherwise mixed-version clusters may misbehave. + */ public static final TransportVersion INDEX_SOURCE = def(9_158_0_00); public static final TransportVersion MAX_HEAP_SIZE_PER_NODE_IN_CLUSTER_INFO = def(9_159_0_00); public static final TransportVersion TIMESERIES_DEFAULT_LIMIT = def(9_160_0_00); 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 be96a95d6710d..f9f56ae698b9a 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 @@ -34,8 +34,7 @@ import java.util.function.Function; import static java.util.stream.Collectors.toMap; -import static org.elasticsearch.TransportVersions.INFERENCE_REQUEST_ADAPTIVE_RATE_LIMITING; -import static org.elasticsearch.TransportVersions.ML_INFERENCE_SAGEMAKER_CHAT_COMPLETION; +import static org.elasticsearch.TransportVersions.INDEX_SOURCE; /** * This enum represents data types the ES|QL query processing layer is able to @@ -123,6 +122,7 @@ * verifier rules, or other places. We suggest starting with CSV tests and * seeing where they fail. * + * TODO: update the items below * There are some additional steps that should be taken when removing the * feature flag and getting ready for a release: * */ - DOC_DATA_TYPE(builder().esType("_doc").estimatedSize(Integer.BYTES * 3)), + DOC_DATA_TYPE(builder().esType("_doc").estimatedSize(Integer.BYTES * 3).supportedOnAllNodes()), /** * Fields with this type represent values from the {@link TimeSeriesIdFieldMapper}. * Every document in {@link IndexMode#TIME_SERIES} index will have a single value * for this field and the segments themselves are sorted on this value. */ - TSID_DATA_TYPE(builder().esType("_tsid").estimatedSize(Long.BYTES * 2).docValues()), + TSID_DATA_TYPE( + builder().esType("_tsid") + .estimatedSize(Long.BYTES * 2) + .docValues() + .supportedOn( + // We use INDEX_SOURCE because it's already on Serverless at the time of writing and it's not in stateful versions before 9.2.0. + INDEX_SOURCE + ) + ), /** * Fields with this type are the partial result of running a non-time-series aggregation * inside alongside time-series aggregations. These fields are not parsable from the * mapping and should be hidden from users. */ - PARTIAL_AGG(builder().esType("partial_agg").estimatedSize(1024)), - + PARTIAL_AGG(builder().esType("partial_agg").estimatedSize(1024).supportedOnAllNodes()), AGGREGATE_METRIC_DOUBLE( builder().esType("aggregate_metric_double") .estimatedSize(Double.BYTES * 3 + Integer.BYTES) - .createdVersion( - // Version created just *after* we committed support for aggregate_metric_double - INFERENCE_REQUEST_ADAPTIVE_RATE_LIMITING + .supportedOn( + // We use INDEX_SOURCE because it's already on Serverless at the time of writing and it's not in stateful versions before 9.2.0. + INDEX_SOURCE ) ), - /** * Fields with this type are dense vectors, represented as an array of double values. */ DENSE_VECTOR( builder().esType("dense_vector") .estimatedSize(4096) - .createdVersion( - // Version created just *after* we committed support for dense_vector - ML_INFERENCE_SAGEMAKER_CHAT_COMPLETION + .supportedOn( + // We use INDEX_SOURCE because it's already on Serverless at the time of writing and it's not in stateful versions before 9.2.0. + INDEX_SOURCE ) ); @@ -348,6 +375,7 @@ public enum DataType implements Writeable { * * */ + // TODO: remove the feature flags. We should have only 1 mechanism. public static final Map UNDER_CONSTRUCTION = Map.ofEntries( Map.entry(AGGREGATE_METRIC_DOUBLE, EsqlCorePlugin.AGGREGATE_METRIC_DOUBLE_FEATURE_FLAG), Map.entry(DENSE_VECTOR, EsqlCorePlugin.DENSE_VECTOR_FEATURE_FLAG) @@ -396,7 +424,7 @@ public enum DataType implements Writeable { /** * Version that first created this data type. */ - private final CreatedVersion createdVersion; + private final SupportedVersion supportedVersion; DataType(Builder builder) { String typeString = builder.typeName != null ? builder.typeName : builder.esType; @@ -410,7 +438,8 @@ public enum DataType implements Writeable { this.isCounter = builder.isCounter; this.widenSmallNumeric = builder.widenSmallNumeric; this.counter = builder.counter; - this.createdVersion = builder.createdVersion; + assert (builder.supportedVersion != null) : "version from when a data type is supported is required"; + this.supportedVersion = builder.supportedVersion; } private static final Collection TYPES = Arrays.stream(values()) @@ -753,7 +782,7 @@ public DataType counter() { @Override public void writeTo(StreamOutput out) throws IOException { - if (createdVersion.supports(out.getTransportVersion()) == false) { + if (supportedVersion.supports(out.getTransportVersion()) == false) { /* * TODO when we implement version aware planning flip this to an IllegalStateException * so we throw a 500 error. It'll be our bug then. Right now it's a sign that the user @@ -816,8 +845,8 @@ public boolean isDate() { }; } - public CreatedVersion createdVersion() { - return createdVersion; + public SupportedVersion supportedVersion() { + return supportedVersion; } public static DataType suggestedCast(Set originalTypes) { @@ -888,11 +917,10 @@ private static class Builder { private DataType counter; /** - * The version when this data type was created. We default to the first - * version for which we maintain wire compatibility, which is pretty - * much {@code 8.18.0}. + * The version from when on a {@link DataType} is supported. When a query tries to use a data type + * not supported on the node it runs on, we throw during serialization of the type. */ - private CreatedVersion createdVersion = CreatedVersion.SUPPORTED_ON_ALL_NODES; + private SupportedVersion supportedVersion; Builder() {} @@ -950,8 +978,18 @@ Builder counter(DataType counter) { return this; } - Builder createdVersion(TransportVersion createdVersion) { - this.createdVersion = CreatedVersion.supportedOn(createdVersion); + Builder supportedOn(TransportVersion supportedVersion) { + this.supportedVersion = SupportedVersion.supportedOn(supportedVersion); + return this; + } + + Builder supportedOnAllNodes() { + this.supportedVersion = SupportedVersion.SUPPORTED_ON_ALL_NODES; + return this; + } + + Builder underConstruction() { + this.supportedVersion = SupportedVersion.UNDER_CONSTRUCTION; return this; } } diff --git a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/CreatedVersion.java b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/SupportedVersion.java similarity index 53% rename from x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/CreatedVersion.java rename to x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/SupportedVersion.java index 401e8e3d8197c..b8b5f592c3370 100644 --- a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/CreatedVersion.java +++ b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/SupportedVersion.java @@ -7,15 +7,13 @@ package org.elasticsearch.xpack.esql.core.type; +import org.elasticsearch.Build; import org.elasticsearch.TransportVersion; -/** - * Version that supports a {@link DataType}. - */ -public interface CreatedVersion { +public interface SupportedVersion { boolean supports(TransportVersion version); - CreatedVersion SUPPORTED_ON_ALL_NODES = new CreatedVersion() { + SupportedVersion SUPPORTED_ON_ALL_NODES = new SupportedVersion() { @Override public boolean supports(TransportVersion version) { return true; @@ -27,16 +25,28 @@ public String toString() { } }; - static CreatedVersion supportedOn(TransportVersion createdVersion) { - return new CreatedVersion() { + SupportedVersion UNDER_CONSTRUCTION = new SupportedVersion() { + @Override + public boolean supports(TransportVersion version) { + return Build.current().isSnapshot(); + } + + @Override + public String toString() { + return "UnderConstruction"; + } + }; + + static SupportedVersion supportedOn(TransportVersion supportedVersion) { + return new SupportedVersion() { @Override public boolean supports(TransportVersion version) { - return version.supports(createdVersion); + return Build.current().isSnapshot() || version.supports(supportedVersion); } @Override public String toString() { - return "SupportedOn[" + createdVersion + "]"; + return "SupportedOn[" + supportedVersion + "]"; } }; } From 37fa745eb4dd8e7710bae81c8c70804937b87ed2 Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Tue, 30 Sep 2025 13:42:44 +0200 Subject: [PATCH 02/11] Remove the UNDER_CONSTRUCTION feature flag usage Rely on the SupportedVersion class, instead. --- .../xpack/esql/core/type/DataType.java | 93 +++++++------------ .../esql/core/type/SupportedVersion.java | 23 ++++- .../function/EsqlFunctionRegistry.java | 7 +- 3 files changed, 56 insertions(+), 67 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 f9f56ae698b9a..45696f8117a44 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 @@ -11,11 +11,9 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; -import org.elasticsearch.common.util.FeatureFlag; import org.elasticsearch.index.IndexMode; import org.elasticsearch.index.mapper.SourceFieldMapper; import org.elasticsearch.index.mapper.TimeSeriesIdFieldMapper; -import org.elasticsearch.xpack.esql.core.plugin.EsqlCorePlugin; import org.elasticsearch.xpack.esql.core.util.PlanStreamInput; import org.elasticsearch.xpack.esql.core.util.PlanStreamOutput; @@ -32,6 +30,7 @@ import java.util.Objects; import java.util.Set; import java.util.function.Function; +import java.util.stream.Collectors; import static java.util.stream.Collectors.toMap; import static org.elasticsearch.TransportVersions.INDEX_SOURCE; @@ -53,15 +52,14 @@ * easier. *
    *
  • - * Create a new feature flag for the type in {@link EsqlCorePlugin}. We - * recommend developing the data type over a series of smaller PRs behind - * a feature flag; even for relatively simple data types.
    • + * Create a new data type and mark it as under construction using + * {@link Builder#underConstruction()}. This makes the type available on + * SNAPSHOT builds, only, prevents some tests from running and prevents documentation + * for the new type to be built. *
  • - * Add a capability to EsqlCapabilities related to the new type, and - * gated by the feature flag you just created. Again, using the feature - * flag is preferred over snapshot-only. As development progresses, you may - * need to add more capabilities related to the new type, e.g. for - * supporting specific functions. This is fine, and expected.
    • + * New tests using the type will require a new {@code EsqlCapabilities} entry, + * otherwise bwc tests will fail (even in SNAPSHOT builds) because old nodes don't + * know about the new type. *
  • * Create a new CSV test file for the new type. You'll either need to * create a new data file as well, or add values of the new type to @@ -86,11 +84,6 @@ * as appropriate. That is the main way in which the new type will be tied * into the framework.
    • *
  • - * Add the new type to the {@link DataType#UNDER_CONSTRUCTION} - * collection. This is used by the test framework to disable some checks - * around how functions report their supported types, which would otherwise - * generate a lot of noise while the type is still in development.
    • - *
  • * Add typed data generators to TestCaseSupplier, and make sure all * functions that support the new type have tests for it.
    • *
  • @@ -121,20 +114,20 @@ * EsqlDataTypeConverter#commonType, individual function type checking, the * verifier rules, or other places. We suggest starting with CSV tests and * seeing where they fail.
    • + *
- * TODO: update the items below * There are some additional steps that should be taken when removing the * feature flag and getting ready for a release: *
    *
  • - * Ensure the capabilities for this type are always enabled - *
    • + * Ensure the capabilities for this type are always enabled. *
  • - * Remove the type from the {@link DataType#UNDER_CONSTRUCTION} - * collection
    • + * Mark the type with a new transport version via + * {@link Builder#supportedOn(TransportVersion)}. This will enable the type on + * non-SNAPSHOT builds as long as all nodes in the cluster (and remote clusters) + * support it. *
  • - * Fix new test failures related to declared function types - *
    • + * Fix new test failures related to declared function types. *
  • * Make sure to run the full test suite locally via gradle to generate * the function type tables and helper files with the new type. Ensure all @@ -301,7 +294,7 @@ public enum DataType implements Writeable { // wild estimate for size, based on some test data (airport_city_boundaries) CARTESIAN_SHAPE(builder().esType("cartesian_shape").estimatedSize(200).docValues().supportedOnAllNodes()), GEO_SHAPE(builder().esType("geo_shape").estimatedSize(200).docValues().supportedOnAllNodes()), - // We use INDEX_SOURCE because it's already on Serverless at the time of writing and it's not in stateful versions before 9.2.0. + // We use INDEX_SOURCE because it's already on Serverless at the time of writing, and it's not in stateful versions before 9.2.0. GEOHASH(builder().esType("geohash").typeName("GEOHASH").estimatedSize(Long.BYTES).supportedOn(INDEX_SOURCE)), GEOTILE(builder().esType("geotile").typeName("GEOTILE").estimatedSize(Long.BYTES).supportedOn(INDEX_SOURCE)), GEOHEX(builder().esType("geohex").typeName("GEOHEX").estimatedSize(Long.BYTES).supportedOn(INDEX_SOURCE)), @@ -323,15 +316,8 @@ public enum DataType implements Writeable { * Every document in {@link IndexMode#TIME_SERIES} index will have a single value * for this field and the segments themselves are sorted on this value. */ - TSID_DATA_TYPE( - builder().esType("_tsid") - .estimatedSize(Long.BYTES * 2) - .docValues() - .supportedOn( - // We use INDEX_SOURCE because it's already on Serverless at the time of writing and it's not in stateful versions before 9.2.0. - INDEX_SOURCE - ) - ), + // We use INDEX_SOURCE because it's already on Serverless at the time of writing, and it's not in stateful versions before 9.2.0. + TSID_DATA_TYPE(builder().esType("_tsid").estimatedSize(Long.BYTES * 2).docValues().supportedOn(INDEX_SOURCE)), /** * Fields with this type are the partial result of running a non-time-series aggregation * inside alongside time-series aggregations. These fields are not parsable from the @@ -342,44 +328,25 @@ public enum DataType implements Writeable { builder().esType("aggregate_metric_double") .estimatedSize(Double.BYTES * 3 + Integer.BYTES) .supportedOn( - // We use INDEX_SOURCE because it's already on Serverless at the time of writing and it's not in stateful versions before 9.2.0. + // TODO: Use actual version when it's added to TransportVersions INDEX_SOURCE ) ), /** - * Fields with this type are dense vectors, represented as an array of double values. + * Fields with this type are dense vectors, represented as an array of float values. */ DENSE_VECTOR( builder().esType("dense_vector") .estimatedSize(4096) .supportedOn( - // We use INDEX_SOURCE because it's already on Serverless at the time of writing and it's not in stateful versions before 9.2.0. + // TODO: Use actual version when it's added to TransportVersions INDEX_SOURCE ) ); - /** - * Types that are actively being built. These types are - *
      - *
    • Not returned from Elasticsearch if their associated {@link FeatureFlag} is disabled.
      • - *
    • Not included in generated documentation
      • - *
    • - * Not tested by {@code ErrorsForCasesWithoutExamplesTestCase} subclasses. - * When a function supports a type it includes a test case in its subclass - * of {@code AbstractFunctionTestCase}. If a function does not support. - * them like {@code TO_STRING} then the tests won't notice. See class javadoc - * for instructions on adding new types, but that usually involves adding support - * for that type to a handful of functions. Once you've done that you should be - * able to remove your new type from UNDER_CONSTRUCTION and update a few error - * messages. - *
      • - *
    - */ - // TODO: remove the feature flags. We should have only 1 mechanism. - public static final Map UNDER_CONSTRUCTION = Map.ofEntries( - Map.entry(AGGREGATE_METRIC_DOUBLE, EsqlCorePlugin.AGGREGATE_METRIC_DOUBLE_FEATURE_FLAG), - Map.entry(DENSE_VECTOR, EsqlCorePlugin.DENSE_VECTOR_FEATURE_FLAG) - ); + public static final Set UNDER_CONSTRUCTION = Arrays.stream(DataType.values()) + .filter(t -> t.supportedVersion() == SupportedVersion.UNDER_CONSTRUCTION) + .collect(Collectors.toSet()); private final String typeName; @@ -498,11 +465,7 @@ public static DataType fromTypeName(String name) { public static DataType fromEs(String name) { DataType type = ES_TO_TYPE.get(name); - if (type == null) { - return UNSUPPORTED; - } - FeatureFlag underConstruction = UNDER_CONSTRUCTION.get(type); - if (underConstruction != null && underConstruction.isEnabled() == false) { + if (type == null || type.supportedVersion().supportedLocally() == false) { return UNSUPPORTED; } return type; @@ -978,6 +941,12 @@ Builder counter(DataType counter) { return this; } + /** + * The version from when on a {@link DataType} is supported. When a query tries to use a data type + * not supported on any of the nodes it runs on, it is invalid. + *

    + * Generally, we should add a dedicated transport version when a type is enabled on release builds. + */ Builder supportedOn(TransportVersion supportedVersion) { this.supportedVersion = SupportedVersion.supportedOn(supportedVersion); return this; diff --git a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/SupportedVersion.java b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/SupportedVersion.java index b8b5f592c3370..576ad0bfce815 100644 --- a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/SupportedVersion.java +++ b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/SupportedVersion.java @@ -13,6 +13,10 @@ public interface SupportedVersion { boolean supports(TransportVersion version); + default boolean supportedLocally() { + return supports(TransportVersion.current()); + } + SupportedVersion SUPPORTED_ON_ALL_NODES = new SupportedVersion() { @Override public boolean supports(TransportVersion version) { @@ -25,6 +29,23 @@ public String toString() { } }; + /** + * Types that are actively being built. These types are + *

      + *
    • Not returned from Elasticsearch on release builds.
      • + *
    • Not included in generated documentation
      • + *
    • + * Not tested by {@code ErrorsForCasesWithoutExamplesTestCase} subclasses. + * When a function supports a type it includes a test case in its subclass + * of {@code AbstractFunctionTestCase}. If a function does not support. + * them like {@code TO_STRING} then the tests won't notice. See class javadoc + * for instructions on adding new types, but that usually involves adding support + * for that type to a handful of functions. Once you've done that you should be + * able to remove your new type from UNDER_CONSTRUCTION and update a few error + * messages. + *
      • + *
    + */ SupportedVersion UNDER_CONSTRUCTION = new SupportedVersion() { @Override public boolean supports(TransportVersion version) { @@ -41,7 +62,7 @@ static SupportedVersion supportedOn(TransportVersion supportedVersion) { return new SupportedVersion() { @Override public boolean supports(TransportVersion version) { - return Build.current().isSnapshot() || version.supports(supportedVersion); + return version.supports(supportedVersion) || Build.current().isSnapshot(); } @Override diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/EsqlFunctionRegistry.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/EsqlFunctionRegistry.java index 45e7849b6a603..eb1b7ad19c007 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/EsqlFunctionRegistry.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/EsqlFunctionRegistry.java @@ -10,7 +10,6 @@ import org.elasticsearch.Build; import org.elasticsearch.common.Strings; import org.elasticsearch.common.util.CollectionUtils; -import org.elasticsearch.common.util.FeatureFlag; import org.elasticsearch.xpack.esql.core.QlIllegalArgumentException; import org.elasticsearch.xpack.esql.core.expression.Expression; import org.elasticsearch.xpack.esql.core.expression.MapExpression; @@ -795,9 +794,9 @@ public static ArgSignature paramWithoutAnnotation(String name) { * Remove types that are being actively built. */ private static String[] removeUnderConstruction(String[] types) { - for (Map.Entry underConstruction : DataType.UNDER_CONSTRUCTION.entrySet()) { - if (underConstruction.getValue().isEnabled() == false) { - types = Arrays.stream(types).filter(t -> underConstruction.getKey().typeName().equals(t) == false).toArray(String[]::new); + for (DataType underConstruction : DataType.UNDER_CONSTRUCTION) { + if (underConstruction.supportedVersion().supportedLocally() == false) { + types = Arrays.stream(types).filter(t -> underConstruction.typeName().equals(t) == false).toArray(String[]::new); } } return types; From b33037ca95972b2fb17f840fa7b59753d2afc89f Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Tue, 30 Sep 2025 14:17:16 +0200 Subject: [PATCH 03/11] Update docs/changelog/135697.yaml --- docs/changelog/135697.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 docs/changelog/135697.yaml diff --git a/docs/changelog/135697.yaml b/docs/changelog/135697.yaml new file mode 100644 index 0000000000000..3fbb5060990ff --- /dev/null +++ b/docs/changelog/135697.yaml @@ -0,0 +1,6 @@ +pr: 135697 +summary: Update mechanism for under construction data types +area: ES|QL +type: bug +issues: + - 135193 From 1e2b18ecccb3f78e59bd18b4ca667f35571765c2 Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Wed, 1 Oct 2025 12:05:11 +0200 Subject: [PATCH 04/11] Update AllSupportedFieldsTestCase --- .../xpack/esql/core/type/DataType.java | 1 + .../qa/rest/AllSupportedFieldsTestCase.java | 57 ++++++++++--------- .../xpack/esql/action/LookupJoinTypesIT.java | 2 +- 3 files changed, 33 insertions(+), 27 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 b69604a4523e4..6e66e80822a23 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 @@ -457,6 +457,7 @@ public static DataType fromTypeName(String name) { public static DataType fromEs(String name) { DataType type = ES_TO_TYPE.get(name); + // Ensure that under construction types are not used in non-snapshot builds. if (type == null || type.supportedVersion().supportedLocally() == false) { return UNSUPPORTED; } 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 f4bbdb99f00ab..3197630bfe92a 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 @@ -10,6 +10,7 @@ import com.carrotsearch.randomizedtesting.annotations.ParametersFactory; import org.apache.http.util.EntityUtils; +import org.elasticsearch.Build; import org.elasticsearch.TransportVersion; import org.elasticsearch.client.Request; import org.elasticsearch.client.ResponseException; @@ -46,22 +47,25 @@ import static org.hamcrest.Matchers.any; import static org.hamcrest.Matchers.anyOf; import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.either; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.nullValue; /** - * Creates indices with all supported fields and fetches values from them. + * Creates indices with all supported fields and fetches values from them to + * confirm that release builds correctly handle data types, even if they were + * introduced in later versions. + *

    + * Entirely skipped in snapshot builds; data types that are under + * construction are normally tested well enough in spec tests, skipping + * old versions via {@link org.elasticsearch.xpack.esql.action.EsqlCapabilities}. *

    * In a single cluster where all nodes are on a single version this is * just an "is it plugged in" style smoke test. In a mixed version cluster * this is testing the behavior of fetching potentially unsupported field - * types. The same is true multi-cluster cases. - *

    + * types. The same is true for multi-cluster cases. *

    * This isn't trying to test complex interactions with field loading so we * load constant field values and have simple mappings. - *

    */ public class AllSupportedFieldsTestCase extends ESRestTestCase { private static final Logger logger = LogManager.getLogger(FieldExtractorTestCase.class); @@ -71,6 +75,11 @@ public class AllSupportedFieldsTestCase extends ESRestTestCase { @ParametersFactory(argumentFormatting = "pref=%s mode=%s") public static List args() { + if (Build.current().isSnapshot()) { + // We only test behavior in release builds. Snapshot builds will have data types enabled that are still under construction. + return List.of(); + } + List args = new ArrayList<>(); for (MappedFieldType.FieldExtractPreference extractPreference : Arrays.asList( null, @@ -169,6 +178,7 @@ public void createIndices() throws IOException { } public final void testFetchAll() throws IOException { + // TODO: add _tsid on newer setups Map response = esql(""" , _id, _ignored, _index_mode, _score, _source, _version | LIMIT 1000 @@ -204,7 +214,7 @@ public final void testFetchAll() throws IOException { if (supportedInIndex(type) == false) { continue; } - expectedValues = expectedValues.entry(fieldName(type), expectedValue(nodeInfo.version, type)); + expectedValues = expectedValues.entry(fieldName(type), expectedValue(type)); } expectedValues = expectedValues.entry("_id", any(String.class)) .entry("_ignored", nullValue()) @@ -219,6 +229,7 @@ public final void testFetchAll() throws IOException { profileLogger.clearProfile(); } + // Tests a workaround and will become obsolete once we can determine the actual minimum transport version of all nodes. public final void testFetchDenseVector() throws IOException { Map response; try { @@ -400,7 +411,8 @@ private void createAllTypesDoc(RestClient client, String indexName) throws IOExc client.performRequest(request); } - private Matcher expectedValue(TransportVersion version, DataType type) throws IOException { + // This will become dependent on the minimum transport version of all nodes once we can determine that. + private Matcher expectedValue(DataType type) { return switch (type) { case BOOLEAN -> equalTo(true); case COUNTER_LONG, LONG, COUNTER_INTEGER, INTEGER, UNSIGNED_LONG, SHORT, BYTE -> equalTo(1); @@ -419,19 +431,14 @@ private Matcher expectedValue(TransportVersion version, DataType type) throws case GEO_SHAPE -> equalTo("POINT (-71.34 41.12)"); case NULL -> nullValue(); case AGGREGATE_METRIC_DOUBLE -> { - if (denseVectorAggMetricDoubleIfFns()) { - // If all versions are new we get null. If any are old, some *might* support aggregate_metric_double - yield nullValue(); - } - Matcher expected = equalTo("{\"min\":-302.5,\"max\":702.3,\"sum\":200.0,\"value_count\":25}"); - yield anyOf(nullValue(), expected); + // Currently, we cannot tell if all nodes support it or not so we treat it as unsupported. + // TODO: Fix this once we know the node versions. + yield nullValue(); } case DENSE_VECTOR -> { - if (denseVectorAggMetricDoubleIfFns()) { - // If all versions are new we get null. If any are old, some *might* support dense_vector - yield nullValue(); - } - yield anyOf(nullValue(), expectedDenseVector(version)); + // Currently, we cannot tell if all nodes support it or not so we treat it as unsupported. + // TODO: Fix this once we know the node versions. + yield nullValue(); } default -> throw new AssertionError("unsupported field type [" + type + "]"); }; @@ -449,6 +456,7 @@ private Matcher> expectedDenseVector(TransportVersion version) { private static boolean supportedInIndex(DataType t) { return switch (t) { // These are supported but implied by the index process. + // TODO: tsid case OBJECT, SOURCE, DOC_DATA_TYPE, TSID_DATA_TYPE, // Internal only UNSUPPORTED, PARTIAL_AGG, @@ -500,7 +508,8 @@ private Map nameToValue(List names, List values) { return result; } - private Matcher expectedType(DataType type) throws IOException { + // This will become dependent on the minimum transport version of all nodes once we can determine that. + private Matcher expectedType(DataType type) { return switch (type) { case COUNTER_DOUBLE, COUNTER_LONG, COUNTER_INTEGER -> { if (indexMode == IndexMode.TIME_SERIES) { @@ -512,13 +521,9 @@ private Matcher expectedType(DataType type) throws IOException { case HALF_FLOAT, SCALED_FLOAT, FLOAT -> equalTo("double"); case NULL -> equalTo("keyword"); // Currently unsupported without TS command or KNN function - case AGGREGATE_METRIC_DOUBLE, DENSE_VECTOR -> { - if (denseVectorAggMetricDoubleIfFns()) { - // If all versions are new we get null. If any are old, some *might* support dense_vector - yield equalTo("unsupported"); - } - yield either(equalTo("unsupported")).or(equalTo(type.esType())); - } + case AGGREGATE_METRIC_DOUBLE, DENSE_VECTOR -> + // TODO: Fix this once we know the node versions. + equalTo("unsupported"); default -> equalTo(type.esType()); }; } diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/LookupJoinTypesIT.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/LookupJoinTypesIT.java index 5c7d56511b6e5..cd5b9df03c0e4 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/LookupJoinTypesIT.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/LookupJoinTypesIT.java @@ -1024,7 +1024,7 @@ public void doTest() { } private boolean isValidDataType(DataType dataType) { - return UNDER_CONSTRUCTION.get(dataType) == null || UNDER_CONSTRUCTION.get(dataType).isEnabled(); + return UNDER_CONSTRUCTION.contains(dataType) == false; } private static void saveJoinTypes(Supplier> signatures) throws Exception { From 56614ae6efcc176690a3c9cf0776adf36c01daf7 Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Wed, 1 Oct 2025 12:25:55 +0200 Subject: [PATCH 05/11] Fix some more tests --- .../xpack/esql/core/type/SupportedVersion.java | 2 ++ .../function/AbstractFunctionTestCase.java | 12 ++++++------ .../fulltext/AbstractMatchFullTextFunctionTests.java | 2 +- .../function/fulltext/MatchPhraseTests.java | 2 +- .../function/scalar/conditional/CaseTests.java | 3 +-- .../optimizer/LocalPhysicalPlanOptimizerTests.java | 2 +- .../xpack/esql/tree/EsqlNodeSubclassTests.java | 2 +- 7 files changed, 13 insertions(+), 12 deletions(-) diff --git a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/SupportedVersion.java b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/SupportedVersion.java index 576ad0bfce815..a85363927273f 100644 --- a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/SupportedVersion.java +++ b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/SupportedVersion.java @@ -45,6 +45,8 @@ public String toString() { * messages. *
    • *
+ * Snapshot builds treat these as supported. Mixed/multi cluster tests with older nodes + * have to just be skipped using capabilites, as always. */ SupportedVersion UNDER_CONSTRUCTION = new SupportedVersion() { @Override diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/AbstractFunctionTestCase.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/AbstractFunctionTestCase.java index 750c186c3275b..61de9ce7601f9 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/AbstractFunctionTestCase.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/AbstractFunctionTestCase.java @@ -418,7 +418,7 @@ public static Stream validFunctionParameters() { // We don't test that functions don't take date_period or time_duration. We should. return false; } - if (DataType.UNDER_CONSTRUCTION.containsKey(t)) { + if (DataType.UNDER_CONSTRUCTION.contains(t)) { /* * Types under construction aren't checked because we're actively * adding support for them to functions. That’s *why* they are @@ -763,7 +763,7 @@ public static void testFunctionInfo() { for (int i = 0; i < args.size() && i < types.size(); i++) { typesFromSignature.get(i).add(types.get(i).dataType().esNameIfPossible()); } - if (DataType.UNDER_CONSTRUCTION.containsKey(entry.returnType()) == false) { + if (DataType.UNDER_CONSTRUCTION.contains(entry.returnType()) == false) { returnFromSignature.add(entry.returnType().esNameIfPossible()); } } @@ -771,11 +771,11 @@ public static void testFunctionInfo() { for (int i = 0; i < args.size(); i++) { EsqlFunctionRegistry.ArgSignature arg = args.get(i); Set annotationTypes = Arrays.stream(arg.type()) - .filter(t -> DataType.UNDER_CONSTRUCTION.containsKey(DataType.fromNameOrAlias(t)) == false) + .filter(t -> DataType.UNDER_CONSTRUCTION.contains(DataType.fromNameOrAlias(t)) == false) .collect(Collectors.toCollection(TreeSet::new)); Set signatureTypes = typesFromSignature.get(i) .stream() - .filter(t -> DataType.UNDER_CONSTRUCTION.containsKey(DataType.fromNameOrAlias(t)) == false) + .filter(t -> DataType.UNDER_CONSTRUCTION.contains(DataType.fromNameOrAlias(t)) == false) .collect(Collectors.toCollection(TreeSet::new)); if (signatureTypes.isEmpty()) { log.info("{}: skipping", arg.name()); @@ -796,7 +796,7 @@ public static void testFunctionInfo() { } Set returnTypes = Arrays.stream(description.returnType()) - .filter(t -> DataType.UNDER_CONSTRUCTION.containsKey(DataType.fromNameOrAlias(t)) == false) + .filter(t -> DataType.UNDER_CONSTRUCTION.contains(DataType.fromNameOrAlias(t)) == false) .collect(Collectors.toCollection(TreeSet::new)); assertEquals(returnFromSignature, returnTypes); } @@ -1060,7 +1060,7 @@ static boolean shouldHideSignature(List argTypes, DataType return true; } - for (DataType dt : DataType.UNDER_CONSTRUCTION.keySet()) { + for (DataType dt : DataType.UNDER_CONSTRUCTION) { if (returnType == dt || argTypes.stream().anyMatch(p -> p.dataType() == dt)) { return true; } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/fulltext/AbstractMatchFullTextFunctionTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/fulltext/AbstractMatchFullTextFunctionTests.java index a3ae10d18b3f6..de261d0e43e68 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/fulltext/AbstractMatchFullTextFunctionTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/fulltext/AbstractMatchFullTextFunctionTests.java @@ -338,7 +338,7 @@ private static void addQueryAsStringTestCases(List suppliers) private static void addStringTestCases(List suppliers) { for (DataType fieldType : DataType.stringTypes()) { - if (DataType.UNDER_CONSTRUCTION.containsKey(fieldType)) { + if (DataType.UNDER_CONSTRUCTION.contains(fieldType)) { continue; } for (TestCaseSupplier.TypedDataSupplier queryDataSupplier : stringCases(fieldType)) { diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/fulltext/MatchPhraseTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/fulltext/MatchPhraseTests.java index 6b04ed2f93e88..52f82ab14720c 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/fulltext/MatchPhraseTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/fulltext/MatchPhraseTests.java @@ -55,7 +55,7 @@ private static List testCaseSuppliers() { public static void addStringTestCases(List suppliers) { for (DataType fieldType : DataType.stringTypes()) { - if (DataType.UNDER_CONSTRUCTION.containsKey(fieldType)) { + if (DataType.UNDER_CONSTRUCTION.contains(fieldType)) { continue; } for (TestCaseSupplier.TypedDataSupplier queryDataSupplier : stringCases(fieldType)) { diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/conditional/CaseTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/conditional/CaseTests.java index 400dd7d09e841..57470bc26c376 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/conditional/CaseTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/conditional/CaseTests.java @@ -64,8 +64,7 @@ public class CaseTests extends AbstractScalarFunctionTestCase { ).collect(Collectors.toList()); if (Build.current().isSnapshot()) { t.addAll( - DataType.UNDER_CONSTRUCTION.keySet() - .stream() + DataType.UNDER_CONSTRUCTION.stream() .filter(type -> type != DataType.AGGREGATE_METRIC_DOUBLE && type != DataType.DENSE_VECTOR) .toList() ); diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalPhysicalPlanOptimizerTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalPhysicalPlanOptimizerTests.java index 5980ab3fa6a32..a333cce044e7f 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalPhysicalPlanOptimizerTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalPhysicalPlanOptimizerTests.java @@ -1226,7 +1226,7 @@ private void checkMatchFunctionPushDown( var analyzer = makeAnalyzer("mapping-all-types.json"); // Check for every possible query data type for (DataType fieldDataType : fieldDataTypes) { - if (DataType.UNDER_CONSTRUCTION.containsKey(fieldDataType)) { + if (DataType.UNDER_CONSTRUCTION.contains(fieldDataType)) { continue; } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/tree/EsqlNodeSubclassTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/tree/EsqlNodeSubclassTests.java index 49cc6dfc57f1e..2f848d07f657f 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/tree/EsqlNodeSubclassTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/tree/EsqlNodeSubclassTests.java @@ -707,7 +707,7 @@ public static String randomGrokPattern() { static List DATA_TYPES = DataType.types() .stream() - .filter(d -> DataType.UNDER_CONSTRUCTION.containsKey(d) == false || Build.current().isSnapshot()) + .filter(d -> DataType.UNDER_CONSTRUCTION.contains(d) == false || Build.current().isSnapshot()) .toList(); static EsQueryExec.FieldSort randomFieldSort() { From 95c5c54831ebe2c7ef749a47f0cb4d615a7639c1 Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Wed, 1 Oct 2025 12:32:30 +0200 Subject: [PATCH 06/11] Update comments on _tsid --- .../xpack/esql/qa/rest/AllSupportedFieldsTestCase.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 3197630bfe92a..18df765576566 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 @@ -177,8 +177,8 @@ public void createIndices() throws IOException { } } + // TODO: Also add a test for _tsid once we can determine the minimum transport version of all nodes. public final void testFetchAll() throws IOException { - // TODO: add _tsid on newer setups Map response = esql(""" , _id, _ignored, _index_mode, _score, _source, _version | LIMIT 1000 @@ -456,7 +456,7 @@ private Matcher> expectedDenseVector(TransportVersion version) { private static boolean supportedInIndex(DataType t) { return switch (t) { // These are supported but implied by the index process. - // TODO: tsid + // TODO: current versions already support _tsid; update this once we can tell whether all nodes support it. case OBJECT, SOURCE, DOC_DATA_TYPE, TSID_DATA_TYPE, // Internal only UNSUPPORTED, PARTIAL_AGG, From c79630a891aa5563254bcd3d65bbd37a4063aea2 Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Wed, 1 Oct 2025 13:36:55 +0200 Subject: [PATCH 07/11] Delete docs/changelog/135697.yaml --- docs/changelog/135697.yaml | 6 ------ 1 file changed, 6 deletions(-) delete mode 100644 docs/changelog/135697.yaml diff --git a/docs/changelog/135697.yaml b/docs/changelog/135697.yaml deleted file mode 100644 index 3fbb5060990ff..0000000000000 --- a/docs/changelog/135697.yaml +++ /dev/null @@ -1,6 +0,0 @@ -pr: 135697 -summary: Update mechanism for under construction data types -area: ES|QL -type: bug -issues: - - 135193 From 98ab058d8adac0be7778930e34eb3bb76eccd8c0 Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Wed, 1 Oct 2025 15:00:21 +0200 Subject: [PATCH 08/11] Move comment --- .../src/main/java/org/elasticsearch/TransportVersions.java | 4 ---- .../org/elasticsearch/xpack/esql/core/type/DataType.java | 6 ++++++ 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/TransportVersions.java b/server/src/main/java/org/elasticsearch/TransportVersions.java index 275ce185fc476..841c2ad26cfad 100644 --- a/server/src/main/java/org/elasticsearch/TransportVersions.java +++ b/server/src/main/java/org/elasticsearch/TransportVersions.java @@ -264,10 +264,6 @@ static TransportVersion def(int id) { public static final TransportVersion STATE_PARAM_GET_SNAPSHOT = def(9_100_0_00); public static final TransportVersion PROJECT_ID_IN_SNAPSHOTS_DELETIONS_AND_REPO_CLEANUP = def(9_101_0_00); public static final TransportVersion CLUSTER_STATE_PROJECTS_SETTINGS = def(9_108_0_00); - /** - * Also used for determining when some new ESQL data types were introduced, as a workaround to then-missing proper versioning of data - * types. Do not backport without updating that workaround as well, otherwise mixed-version clusters may misbehave. - */ public static final TransportVersion INDEX_SOURCE = def(9_158_0_00); public static final TransportVersion MAX_HEAP_SIZE_PER_NODE_IN_CLUSTER_INFO = def(9_159_0_00); public static final TransportVersion TIMESERIES_DEFAULT_LIMIT = def(9_160_0_00); 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 6e66e80822a23..ea71ba36fc43b 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 @@ -295,6 +295,9 @@ public enum DataType implements Writeable { CARTESIAN_SHAPE(builder().esType("cartesian_shape").estimatedSize(200).docValues().supportedOnAllNodes()), GEO_SHAPE(builder().esType("geo_shape").estimatedSize(200).docValues().supportedOnAllNodes()), // We use INDEX_SOURCE because it's already on Serverless at the time of writing, and it's not in stateful versions before 9.2.0. + // NOTE: If INDEX_SOURCE somehow gets backported to a version that doesn't actually support these types, we'll be missing validation for + // mixed/multi clusters with remotes that don't support these types. This is low-ish risk because these types require specific + // geo functions to turn up in the query, and those types aren't available before 9.2.0 either. GEOHASH(builder().esType("geohash").typeName("GEOHASH").estimatedSize(Long.BYTES).supportedOn(INDEX_SOURCE)), GEOTILE(builder().esType("geotile").typeName("GEOTILE").estimatedSize(Long.BYTES).supportedOn(INDEX_SOURCE)), GEOHEX(builder().esType("geohex").typeName("GEOHEX").estimatedSize(Long.BYTES).supportedOn(INDEX_SOURCE)), @@ -317,6 +320,9 @@ public enum DataType implements Writeable { * for this field and the segments themselves are sorted on this value. */ // We use INDEX_SOURCE because it's already on Serverless at the time of writing, and it's not in stateful versions before 9.2.0. + // NOTE: If INDEX_SOURCE somehow gets backported to a version that doesn't actually support _tsid, we'll be missing validation for + // mixed/multi clusters with remotes that don't support these types. This is low-ish risk because _tsid requires specifically being + // used in `FROM idx METADATA _tsid` or in the `TS` command, which both weren't available before 9.2.0. TSID_DATA_TYPE(builder().esType("_tsid").estimatedSize(Long.BYTES * 2).docValues().supportedOn(INDEX_SOURCE)), /** * Fields with this type are the partial result of running a non-time-series aggregation From fd1afadbab8496eeac6307497b4fe03df66f6861 Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Wed, 1 Oct 2025 15:48:49 +0200 Subject: [PATCH 09/11] Moar comments --- .../xpack/esql/core/type/SupportedVersion.java | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/SupportedVersion.java b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/SupportedVersion.java index a85363927273f..7c47531faa2c6 100644 --- a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/SupportedVersion.java +++ b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/SupportedVersion.java @@ -45,9 +45,15 @@ public String toString() { * messages. * * - * Snapshot builds treat these as supported. Mixed/multi cluster tests with older nodes - * have to just be skipped using capabilites, as always. + *

+ * Snapshot builds treat these as always supported so that we can write tests before actually + * turning on the support for the type. Mixed/multi cluster tests with older nodes have to be skipped + * based on capabilites, as always. */ + // We used to have a feature-flag based override, so that in-development types could be + // turned on for testing in release builds. If needed, it's fine to bring this back, but we + // need to make sure that other checks for types being under construction are also overridden. + // Check usage of this constant to be sure. SupportedVersion UNDER_CONSTRUCTION = new SupportedVersion() { @Override public boolean supports(TransportVersion version) { @@ -60,6 +66,14 @@ public String toString() { } }; + /** + * Types that are supported starting with the given version. + *

+ * Snapshot builds treat these as always supported, so that any existing tests using them + * continue to work. Otherwise, we'd have to + * update bwc tests to skip older versions based on a capability check, which can be error-prone + * and risks turning off an unrelated bwc test. + */ static SupportedVersion supportedOn(TransportVersion supportedVersion) { return new SupportedVersion() { @Override From dc4b58fb00efd2a382ea460c76d26da85b3e9c44 Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Wed, 1 Oct 2025 15:51:04 +0200 Subject: [PATCH 10/11] Comment formatting --- .../xpack/esql/core/type/SupportedVersion.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/SupportedVersion.java b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/SupportedVersion.java index 7c47531faa2c6..694e5b2c2f2d7 100644 --- a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/SupportedVersion.java +++ b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/SupportedVersion.java @@ -47,8 +47,8 @@ public String toString() { * *

* Snapshot builds treat these as always supported so that we can write tests before actually - * turning on the support for the type. Mixed/multi cluster tests with older nodes have to be skipped - * based on capabilites, as always. + * turning on the support for the type. Mixed/multi cluster tests with older nodes have to be + * skipped based on capabilites, as always. */ // We used to have a feature-flag based override, so that in-development types could be // turned on for testing in release builds. If needed, it's fine to bring this back, but we @@ -70,9 +70,8 @@ public String toString() { * Types that are supported starting with the given version. *

* Snapshot builds treat these as always supported, so that any existing tests using them - * continue to work. Otherwise, we'd have to - * update bwc tests to skip older versions based on a capability check, which can be error-prone - * and risks turning off an unrelated bwc test. + * continue to work. Otherwise, we'd have to update bwc tests to skip older versions based + * on a capability check, which can be error-prone and risks turning off an unrelated bwc test. */ static SupportedVersion supportedOn(TransportVersion supportedVersion) { return new SupportedVersion() { From c297ffb84b880837a492b3c5fbf30f41b1cd3a27 Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Wed, 1 Oct 2025 17:43:46 +0200 Subject: [PATCH 11/11] Remove extra line in javadoc --- .../java/org/elasticsearch/xpack/esql/core/type/DataType.java | 1 - 1 file changed, 1 deletion(-) 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 ea71ba36fc43b..b4e135fa21e7c 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 @@ -114,7 +114,6 @@ * EsqlDataTypeConverter#commonType, individual function type checking, the * verifier rules, or other places. We suggest starting with CSV tests and * seeing where they fail. - * * There are some additional steps that should be taken when removing the * feature flag and getting ready for a release: