Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ESQL: Introduce language versioning to REST API #106824

Merged
merged 30 commits into from
Apr 3, 2024
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
1f45d65
Add version enum
alex-spies Mar 27, 2024
c4d5a8e
Parse + validate esql.version
alex-spies Mar 27, 2024
26b9fe6
Add tests
alex-spies Mar 27, 2024
887759a
Refactor EsqlVersion.toString
alex-spies Mar 27, 2024
da6c0e1
Make checkstyle happy
alex-spies Mar 27, 2024
8e6de63
Avoid forbidden APIs
alex-spies Mar 27, 2024
98b816f
Use proper emojis
alex-spies Mar 28, 2024
d5bef79
Add disambiguation counter to version
alex-spies Mar 28, 2024
f0a2aae
Make parsing stricter
alex-spies Mar 28, 2024
9e97e43
Rename esql.version -> version
alex-spies Mar 28, 2024
11ba353
Update docs/changelog/106824.yaml
alex-spies Mar 28, 2024
a29fc71
Remove obsolete TODO
alex-spies Mar 28, 2024
85576a7
Spotless
alex-spies Mar 28, 2024
5920994
Refactor month and numberThisMonth
alex-spies Apr 2, 2024
a633632
Add validation tests for non-snapshot builds
alex-spies Apr 2, 2024
c8c9684
Merge remote-tracking branch 'upstream/main' into esql-version
alex-spies Apr 2, 2024
de6304e
Merge remote-tracking branch 'upstream/main' into esql-version
alex-spies Apr 2, 2024
be3f255
Use randomization to test both sync/async
alex-spies Apr 2, 2024
5b15c30
Add empty version string to test
alex-spies Apr 2, 2024
c6f06db
Address feedback
alex-spies Apr 3, 2024
eb795fb
Improve assertions
alex-spies Apr 3, 2024
8a9c0f8
Rename nightly -> snapshot
alex-spies Apr 3, 2024
0b5fb1f
Add latest version to error message when missing
alex-spies Apr 3, 2024
962f16b
Add latest version to error message if invalid
alex-spies Apr 3, 2024
3b7c031
Add latest version in error if using snapshot
alex-spies Apr 3, 2024
20118c6
Remove emoji from error message
alex-spies Apr 3, 2024
037f1f7
Throw IAE instead of AssertionError
alex-spies Apr 3, 2024
ab3be62
Use rocket emoji for first released version
alex-spies Apr 3, 2024
b181c7c
Merge remote-tracking branch 'upstream/main' into esql-version
alex-spies Apr 3, 2024
acb2703
Fix test
alex-spies Apr 3, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions docs/changelog/106824.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 106824
summary: "ESQL: Introduce language versioning to REST API"
area: ES|QL
type: enhancement
issues: []
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ protected EsqlQueryRequest(StreamInput in) throws IOException {
super(in);
}

// Use the unparsed version String, so we don't have to serialize a version object.
public abstract String esqlVersion();
alex-spies marked this conversation as resolved.
Show resolved Hide resolved

public abstract String query();

public abstract QueryBuilder filter();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ public final ActionType<Response> action() {
return action;
}

public abstract EsqlQueryRequestBuilder<Request, Response> esqlVersion(String esqlVersion);

public abstract EsqlQueryRequestBuilder<Request, Response> query(String query);

public abstract EsqlQueryRequestBuilder<Request, Response> filter(QueryBuilder filter);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.elasticsearch.tasks.TaskId;
import org.elasticsearch.xpack.esql.parser.TypedParamValue;
import org.elasticsearch.xpack.esql.plugin.QueryPragmas;
import org.elasticsearch.xpack.esql.version.EsqlVersion;

import java.io.IOException;
import java.util.List;
Expand All @@ -35,6 +36,7 @@ public class EsqlQueryRequest extends org.elasticsearch.xpack.core.esql.action.E

private boolean async;

private String esqlVersion;
private String query;
private boolean columnar;
private boolean profile;
Expand All @@ -45,6 +47,7 @@ public class EsqlQueryRequest extends org.elasticsearch.xpack.core.esql.action.E
private TimeValue waitForCompletionTimeout = DEFAULT_WAIT_FOR_COMPLETION;
private TimeValue keepAlive = DEFAULT_KEEP_ALIVE;
private boolean keepOnCompletion;
private boolean onSnapshotBuild = Build.current().isSnapshot();

static EsqlQueryRequest syncEsqlQueryRequest() {
return new EsqlQueryRequest(false);
Expand All @@ -65,17 +68,54 @@ public EsqlQueryRequest(StreamInput in) throws IOException {
@Override
public ActionRequestValidationException validate() {
ActionRequestValidationException validationException = null;
if (Strings.hasText(esqlVersion) == false) {
// TODO: make this required
// "https://github.com/elastic/elasticsearch/issues/104890"
// validationException = addValidationError(invalidVersion("is required"), validationException);
} else {
EsqlVersion version = EsqlVersion.parse(esqlVersion);
not-napoleon marked this conversation as resolved.
Show resolved Hide resolved
if (version == null) {
validationException = addValidationError(invalidVersion("has invalid value [" + esqlVersion + "]"), validationException);
} else if (version == EsqlVersion.SNAPSHOT && onSnapshotBuild == false) {
validationException = addValidationError(
invalidVersion("with value [" + esqlVersion + "] only allowed in snapshot builds"),
validationException
);
}
}
if (Strings.hasText(query) == false) {
validationException = addValidationError("[query] is required", validationException);
validationException = addValidationError("[" + RequestXContent.QUERY_FIELD + "] is required", validationException);
alex-spies marked this conversation as resolved.
Show resolved Hide resolved
}
if (Build.current().isSnapshot() == false && pragmas.isEmpty() == false) {
validationException = addValidationError("[pragma] only allowed in snapshot builds", validationException);
if (onSnapshotBuild == false && pragmas.isEmpty() == false) {
validationException = addValidationError(
"[" + RequestXContent.PRAGMA_FIELD + "] only allowed in snapshot builds",
validationException
);
}
return validationException;
}

private static String invalidVersion(String reason) {
return "["
+ RequestXContent.ESQL_VERSION_FIELD
+ "] "
+ reason
+ ", latest available version is ["
+ EsqlVersion.latestReleased()
+ "]";
}

public EsqlQueryRequest() {}

public void esqlVersion(String esqlVersion) {
this.esqlVersion = esqlVersion;
}

@Override
public String esqlVersion() {
return esqlVersion;
}

public void query(String query) {
this.query = query;
}
Expand Down Expand Up @@ -174,4 +214,9 @@ public Task createTask(long id, String type, String action, TaskId parentTaskId,
// Pass the query as the description
return new CancellableTask(id, type, action, query, parentTaskId, headers);
}

// Setter for tests
void onSnapshotBuild(boolean onSnapshotBuild) {
this.onSnapshotBuild = onSnapshotBuild;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@ private EsqlQueryRequestBuilder(ElasticsearchClient client, EsqlQueryRequest req
super(client, EsqlQueryAction.INSTANCE, request);
}

@Override
public EsqlQueryRequestBuilder esqlVersion(String esqlVersion) {
request.esqlVersion(esqlVersion);
return this;
}

@Override
public EsqlQueryRequestBuilder query(String query) {
request.query(query);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,11 @@ final class RequestXContent {
PARAM_PARSER.declareString(constructorArg(), TYPE);
}

private static final ParseField QUERY_FIELD = new ParseField("query");
static final ParseField ESQL_VERSION_FIELD = new ParseField("version");
static final ParseField QUERY_FIELD = new ParseField("query");
private static final ParseField COLUMNAR_FIELD = new ParseField("columnar");
private static final ParseField FILTER_FIELD = new ParseField("filter");
private static final ParseField PRAGMA_FIELD = new ParseField("pragma");
static final ParseField PRAGMA_FIELD = new ParseField("pragma");
private static final ParseField PARAMS_FIELD = new ParseField("params");
private static final ParseField LOCALE_FIELD = new ParseField("locale");
private static final ParseField PROFILE_FIELD = new ParseField("profile");
Expand All @@ -72,6 +73,7 @@ static EsqlQueryRequest parseAsync(XContentParser parser) {
}

private static void objectParserCommon(ObjectParser<EsqlQueryRequest, ?> parser) {
parser.declareString(EsqlQueryRequest::esqlVersion, ESQL_VERSION_FIELD);
parser.declareString(EsqlQueryRequest::query, QUERY_FIELD);
parser.declareBoolean(EsqlQueryRequest::columnar, COLUMNAR_FIELD);
parser.declareObject(EsqlQueryRequest::filter, (p, c) -> AbstractQueryBuilder.parseTopLevelQuery(p), FILTER_FIELD);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
/*
* 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.version;

import org.elasticsearch.common.Strings;
import org.elasticsearch.common.VersionId;

import java.util.Arrays;
import java.util.Comparator;
import java.util.LinkedHashMap;
import java.util.Map;

public enum EsqlVersion implements VersionId<EsqlVersion> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better make this a class similar to Version - enum are quite restrictive and impossible to extend.
Having a class gives us future extension points - in the worse case we won't use them and the class will be like an enum.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can flip it to a class with constants when we need that. If we can get away with an enum for a while I'd prefer that.

/**
* Breaking changes go here until the next version is released.
*/
SNAPSHOT(Integer.MAX_VALUE, 12, 99, "📷"),
PARTY_POPPER(2024, 4, "🎉");

static final Map<String, EsqlVersion> VERSION_MAP_WITH_AND_WITHOUT_EMOJI = versionMapWithAndWithoutEmoji();

private static Map<String, EsqlVersion> versionMapWithAndWithoutEmoji() {
Map<String, EsqlVersion> stringToVersion = new LinkedHashMap<>(EsqlVersion.values().length * 2);

for (EsqlVersion version : EsqlVersion.values()) {
putVersionAssertNoDups(stringToVersion, version.versionStringWithoutEmoji(), version);
putVersionAssertNoDups(stringToVersion, version.toString(), version);
}

return stringToVersion;
}

private static void putVersionAssertNoDups(Map<String, EsqlVersion> stringToVersion, String versionString, EsqlVersion version) {
EsqlVersion existingVersionForKey = stringToVersion.put(versionString, version);
if (existingVersionForKey != null) {
throw new AssertionError("Duplicate esql version with version string [" + versionString + "]");
}
}

/**
* Accepts a version string with the emoji suffix or without it.
* E.g. both "2024.04.01.🎉" and "2024.04.01" will be interpreted as {@link EsqlVersion#PARTY_POPPER}.
*/
public static EsqlVersion parse(String versionString) {
return VERSION_MAP_WITH_AND_WITHOUT_EMOJI.get(versionString);
}

public static EsqlVersion latestReleased() {
return Arrays.stream(EsqlVersion.values()).filter(v -> v != SNAPSHOT).max(Comparator.comparingInt(EsqlVersion::id)).get();
}

private int year;
private byte month;
private byte revision;
private String emoji;

EsqlVersion(int year, int month, String emoji) {
this(year, month, 1, emoji);
}

EsqlVersion(int year, int month, int revision, String emoji) {
if ((1 <= revision && revision <= 99) == false) {
throw new AssertionError("Version revision number must be between 1 and 99 but was [" + revision + "]");
}
if ((1 <= month && month <= 12) == false) {
throw new AssertionError("Version month must be between 1 and 12 but was [" + month + "]");
}
if ((emoji.codePointCount(0, emoji.length()) == 1) == false) {
throw new AssertionError("Version emoji must be a single unicode character but was [" + emoji + "]");
}
this.year = year;
this.month = (byte) month;
this.revision = (byte) revision;
this.emoji = emoji;
}

public int year() {
return year;
}

public byte month() {
return month;
}

public byte revision() {
return revision;
}

public String emoji() {
return emoji;
}

public String versionStringWithoutEmoji() {
return this == SNAPSHOT ? "snapshot" : Strings.format("%d.%02d.%02d", year, month, revision);
}

@Override
public String toString() {
return versionStringWithoutEmoji() + "." + emoji;
}

@Override
public int id() {
return this == SNAPSHOT ? Integer.MAX_VALUE : (10000 * year + 100 * month + revision);
}
}
Loading