ESQL: Update mechanism for under construction data types #135697
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,89 @@ | ||
| /* | ||
| * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
| * or more contributor license agreements. Licensed under the Elastic License | ||
| * 2.0; you may not use this file except in compliance with the Elastic License | ||
| * 2.0. | ||
| */ | ||
|
|
||
| package org.elasticsearch.xpack.esql.core.type; | ||
|
|
||
| import org.elasticsearch.Build; | ||
| import org.elasticsearch.TransportVersion; | ||
|
|
||
| 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) { | ||
| return true; | ||
| } | ||
|
|
||
| @Override | ||
| public String toString() { | ||
| return "SupportedOnAllVersions"; | ||
| } | ||
| }; | ||
|
|
||
| /** | ||
| * Types that are actively being built. These types are | ||
| * <ul> | ||
| * <li>Not returned from Elasticsearch on release builds.</li> | ||
| * <li>Not included in generated documentation</li> | ||
| * <li> | ||
| * 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. | ||
| * </li> | ||
| * </ul> | ||
| * <p> | ||
| * 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) { | ||
| return Build.current().isSnapshot(); | ||
| } | ||
|
|
||
| @Override | ||
| public String toString() { | ||
| return "UnderConstruction"; | ||
| } | ||
| }; | ||
|
|
||
| /** | ||
| * Types that are supported starting with the given version. | ||
| * <p> | ||
| * 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 | ||
| public boolean supports(TransportVersion version) { | ||
| return version.supports(supportedVersion) || Build.current().isSnapshot(); | ||
| } | ||
|
|
||
| @Override | ||
| public String toString() { | ||
| return "SupportedOn[" + supportedVersion + "]"; | ||
| } | ||
| }; | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a subtle change that considers released types supported on snapshot builds, always.
Similarly, under construction types are also always considered supported on snapshot builds.
I thought about making a distinction between "created at version" and "supported from version", but that wouldn't really add anything as we already have capabilities to control the behavior of bwc tests in snapshot builds.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm good with just having the supported from version.
It does feel weird to say that released types are always supported on snapshots though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll add a comment to explain that this is so that we don't break a bunch of existing tests when finally enabling a type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added some javadoc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
This is an interesting compromise that makes tests flow more nicely. We don't discard any backwards compatibility tests that we wrote while working on the new type. That's good. But those tests might be guaranteeing more backwards compatibility than we actually offer in production. That's ok. And it makes it explicit when we disable that backward compatibility. All good.