From 65b3391b0156eb4ddc302d1c9e348586ce550d1c Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Fri, 5 Dec 2025 01:28:05 +0100 Subject: [PATCH] ESQL: Add createdVersion for types (#137432) * Add createdVersion for types * [CI] Update transport version definitions * [CI] Update transport version definitions * Do not create new transport version for exp histo --------- Co-authored-by: elasticsearchmachine Co-authored-by: Craig Taverner --- .../xpack/esql/core/type/DataType.java | 85 ++++++++++++++----- .../esql/core/type/SupportedVersion.java | 56 +++++++----- 2 files changed, 98 insertions(+), 43 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 7a8c020d413eb..2bb984ccad32d 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 @@ -47,6 +47,10 @@ * {@link DataType#OBJECT}) * *

Process for adding a new data type

+ * We assume that the data type is already supported in ES indices, but not in + * ES|QL. Types that aren't yet enabled in ES will require some adjustments to + * the process. + *

* Note: it is not expected that all the following steps be done in a single PR. * Use capabilities to gate tests as you go, and use as many PRs as you think * appropriate. New data types are complex, and smaller PRs will make reviews @@ -54,13 +58,14 @@ *

    *
  • * Create a new data type and mark it as under construction using - * {@link Builder#underConstruction()}. This makes the type available on + * {@link Builder#underConstruction(TransportVersion)}. This makes the type available on * SNAPSHOT builds, only, prevents some tests from running and prevents documentation * for the new type to be built.
    • *
  • * 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.
    • + * know about the new type. This capability needs to be SNAPSHOT-only as long as + * the type is under construction *
  • * 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 @@ -115,20 +120,30 @@ * EsqlDataTypeConverter#commonType, individual function type checking, the * verifier rules, or other places. We suggest starting with CSV tests and * seeing where they fail.
    • + *
  • + * Ensure the new type doesn't break {@code FROM idx | KEEP *} queries by + * updating AllSupportedFieldsTestCase. Make sure to run this test in bwc + * configurations in release mode. + *
    • *
- * There are some additional steps that should be taken when removing the - * feature flag and getting ready for a release: + * There are some additional steps that should be taken when getting ready for a release: *
    *
  • * Ensure the capabilities for this type are always enabled.
    • *
  • * Mark the type with a new transport version via - * {@link Builder#supportedSince(TransportVersion)}. This will enable the type on - * non-SNAPSHOT builds as long as all nodes in the cluster (and remote clusters) - * support it.
    • + * {@link Builder#supportedSince(TransportVersion, TransportVersion)}. + * This will enable the type on non-SNAPSHOT builds as long as all nodes in the cluster + * (and remote clusters) support it. + * Use the under-construction transport version for the {@code createdVersion} here so that + * existing tests continue to run. + * *
  • * Fix new test failures related to declared function types.
    • *
  • + * Update the expectations in AllSupportedFieldsTestCase and make sure it + * passes in release builds.
    • + *
  • * 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 * the functions that support the type have appropriate docs for it.
    • @@ -299,12 +314,23 @@ public enum DataType implements Writeable { // 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).supportedSince(DataTypesTransportVersions.INDEX_SOURCE) + builder().esType("geohash") + .typeName("GEOHASH") + .estimatedSize(Long.BYTES) + .supportedSince(DataTypesTransportVersions.INDEX_SOURCE, DataTypesTransportVersions.INDEX_SOURCE) ), GEOTILE( - builder().esType("geotile").typeName("GEOTILE").estimatedSize(Long.BYTES).supportedSince(DataTypesTransportVersions.INDEX_SOURCE) + builder().esType("geotile") + .typeName("GEOTILE") + .estimatedSize(Long.BYTES) + .supportedSince(DataTypesTransportVersions.INDEX_SOURCE, DataTypesTransportVersions.INDEX_SOURCE) + ), + GEOHEX( + builder().esType("geohex") + .typeName("GEOHEX") + .estimatedSize(Long.BYTES) + .supportedSince(DataTypesTransportVersions.INDEX_SOURCE, DataTypesTransportVersions.INDEX_SOURCE) ), - GEOHEX(builder().esType("geohex").typeName("GEOHEX").estimatedSize(Long.BYTES).supportedSince(DataTypesTransportVersions.INDEX_SOURCE)), /** * Fields with this type represent a Lucene doc id. This field is a bit magic in that: @@ -328,7 +354,10 @@ public enum DataType implements Writeable { // 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().supportedSince(DataTypesTransportVersions.INDEX_SOURCE) + builder().esType("_tsid") + .estimatedSize(Long.BYTES * 2) + .docValues() + .supportedSince(DataTypesTransportVersions.INDEX_SOURCE, DataTypesTransportVersions.INDEX_SOURCE) ), /** * Fields with this type are the partial result of running a non-time-series aggregation @@ -339,17 +368,25 @@ public enum DataType implements Writeable { AGGREGATE_METRIC_DOUBLE( builder().esType("aggregate_metric_double") .estimatedSize(Double.BYTES * 3 + Integer.BYTES) - .supportedSince(DataTypesTransportVersions.ESQL_AGGREGATE_METRIC_DOUBLE_CREATED_VERSION) + .supportedSince( + DataTypesTransportVersions.ESQL_AGGREGATE_METRIC_DOUBLE_CREATED_VERSION, + DataTypesTransportVersions.ESQL_AGGREGATE_METRIC_DOUBLE_CREATED_VERSION + ) ), /** * Fields with this type are dense vectors, represented as an array of float values. */ DENSE_VECTOR( - builder().esType("dense_vector").estimatedSize(4096).supportedSince(DataTypesTransportVersions.ESQL_DENSE_VECTOR_CREATED_VERSION) + builder().esType("dense_vector") + .estimatedSize(4096) + .supportedSince( + DataTypesTransportVersions.ESQL_DENSE_VECTOR_CREATED_VERSION, + DataTypesTransportVersions.ESQL_DENSE_VECTOR_CREATED_VERSION + ) ); public static final Set UNDER_CONSTRUCTION = Arrays.stream(DataType.values()) - .filter(t -> t.supportedVersion() == SupportedVersion.UNDER_CONSTRUCTION) + .filter(t -> t.supportedVersion().underConstruction()) .collect(Collectors.toSet()); private final String typeName; @@ -952,13 +989,15 @@ Builder counter(DataType counter) { } /** - * 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. + * Marks a type that is supported in production since {@code supportedVersion}. + * When a query tries to use a data type not supported on the nodes it runs on, this is a bug. *

    - * Generally, we should add a dedicated transport version when a type is enabled on release builds. + * On snapshot builds, the {@code createdVersion} is used instead, so that existing tests continue + * to work after release if the type was previously {@link #underConstruction(TransportVersion)}; + * the under-construction version should be used as the {@code createdVersion}. */ - Builder supportedSince(TransportVersion supportedVersion) { - this.supportedVersion = SupportedVersion.supportedSince(supportedVersion); + Builder supportedSince(TransportVersion createdVersion, TransportVersion supportedVersion) { + this.supportedVersion = SupportedVersion.supportedSince(createdVersion, supportedVersion); return this; } @@ -967,8 +1006,12 @@ Builder supportedOnAllNodes() { return this; } - Builder underConstruction() { - this.supportedVersion = SupportedVersion.UNDER_CONSTRUCTION; + /** + * Marks a type that is not supported in production yet, but is supported in snapshot builds + * starting with the given version. + */ + Builder underConstruction(TransportVersion createdVersion) { + this.supportedVersion = SupportedVersion.underConstruction(createdVersion); 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 8881592660861..2e92087c41948 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 @@ -17,6 +17,10 @@ default boolean supportedLocally() { return supportedOn(TransportVersion.current(), Build.current().isSnapshot()); } + default boolean underConstruction() { + return false; + } + SupportedVersion SUPPORTED_ON_ALL_NODES = new SupportedVersion() { @Override public boolean supportedOn(TransportVersion version, boolean currentBuildIsSnapshot) { @@ -37,47 +41,55 @@ public String toString() { *

  • * 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. + * 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. + * able to turn your new type from under construction into released and update + * a few error messages. *
    • *
*

- * 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. + * Snapshot builds treat these as supported starting from the version they were created on, + * so that we can write tests before actually turning on the support for the type. + * Mixed/multi cluster tests with older nodes should be skipped based on SNAPSHOT-only + * 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 supportedOn(TransportVersion version, boolean currentBuildIsSnapshot) { - return currentBuildIsSnapshot; - } + // Check usage of this method to be sure. + static SupportedVersion underConstruction(TransportVersion createdVersion) { + return new SupportedVersion() { + @Override + public boolean supportedOn(TransportVersion version, boolean currentBuildIsSnapshot) { + return currentBuildIsSnapshot && version.supports(createdVersion); + } - @Override - public String toString() { - return "UnderConstruction"; - } - }; + @Override + public String toString() { + return "UnderConstruction"; + } + + @Override + public boolean underConstruction() { + return true; + } + }; + } /** * 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. + * Snapshot builds treat these as supported from their created version onward, so that any existing tests + * using them should continue to work. */ - static SupportedVersion supportedSince(TransportVersion supportedVersion) { + static SupportedVersion supportedSince(TransportVersion createdVersion, TransportVersion supportedVersion) { + assert supportedVersion.onOrAfter(createdVersion) : "support for a type cannot be enabled before its initial creation"; return new SupportedVersion() { @Override public boolean supportedOn(TransportVersion version, boolean currentBuildIsSnapshot) { - return version.supports(supportedVersion) || currentBuildIsSnapshot; + return currentBuildIsSnapshot ? version.supports(createdVersion) : version.supports(supportedVersion); } @Override