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: Revert "Allow partial results by default in ES|QL (#125060)" #126286

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 0 additions & 16 deletions docs/changelog/125060.yaml

This file was deleted.

6 changes: 6 additions & 0 deletions docs/changelog/126286.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pr: 126286
summary: Revert "Allow partial results by default in ES|QL"
area: ES|QL
type: bug
issues:
- 126275
5 changes: 0 additions & 5 deletions docs/release-notes/breaking-changes.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,6 @@ To learn how to upgrade, check out <uprade docs>.

% ## Next version [elasticsearch-nextversion-breaking-changes]

## 9.1.0 [elasticsearch-910-breaking-changes]

ES|QL
: * Allow partial results by default in ES|QL [#125060](https://github.com/elastic/elasticsearch/pull/125060)

## 9.0.0 [elasticsearch-900-breaking-changes]

Allocation
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ static ElasticsearchCluster buildCluster() {
.module("test-esql-heap-attack")
.setting("xpack.security.enabled", "false")
.setting("xpack.license.self_generated.type", "trial")
.setting("esql.query.allow_partial_results", "false")
.jvmArg("-Xmx512m");
String javaVersion = JvmInfo.jvmInfo().version();
if (javaVersion.equals("20") || javaVersion.equals("21")) {
Expand Down
1 change: 0 additions & 1 deletion x-pack/plugin/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,6 @@ tasks.named("yamlRestCompatTestTransform").configure({ task ->
task.skipTest("ml/post_data/Test POST data job api, flush, close and verify DataCounts doc", "Flush API is deprecated")
task.replaceValueInMatch("Size", 49, "Test flamegraph from profiling-events")
task.replaceValueInMatch("Size", 49, "Test flamegraph from test-events")
task.skipTest("esql/63_enrich_int_range/Invalid age as double", "TODO: require disable allow_partial_results")
})

tasks.named('yamlRestCompatTest').configure {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -312,10 +312,7 @@ public void testIndexPatternErrorMessageComparison_ESQL_SearchDSL() throws Excep
searchRequest.setOptions(RequestOptions.DEFAULT.toBuilder().addHeader("es-security-runas-user", "metadata1_read2"));

// ES|QL query on the same index pattern
var esqlResp = expectThrows(
ResponseException.class,
() -> runESQLCommand("metadata1_read2", "FROM index-user1,index-user2", false)
);
var esqlResp = expectThrows(ResponseException.class, () -> runESQLCommand("metadata1_read2", "FROM index-user1,index-user2"));
var srchResp = expectThrows(ResponseException.class, () -> client().performRequest(searchRequest));

for (ResponseException r : List.of(esqlResp, srchResp)) {
Expand All @@ -334,8 +331,7 @@ public void testLimitedPrivilege() throws Exception {
ResponseException.class,
() -> runESQLCommand(
"metadata1_read2",
"FROM index-user1,index-user2 METADATA _index | STATS sum=sum(value), index=VALUES(_index)",
false
"FROM index-user1,index-user2 METADATA _index | STATS sum=sum(value), index=VALUES(_index)"
)
);
assertThat(
Expand All @@ -348,7 +344,7 @@ public void testLimitedPrivilege() throws Exception {

resp = expectThrows(
ResponseException.class,
() -> runESQLCommand("metadata1_read2", "FROM index-user1,index-user2 METADATA _index | STATS index=VALUES(_index)", false)
() -> runESQLCommand("metadata1_read2", "FROM index-user1,index-user2 METADATA _index | STATS index=VALUES(_index)")
);
assertThat(
EntityUtils.toString(resp.getResponse().getEntity()),
Expand All @@ -360,7 +356,7 @@ public void testLimitedPrivilege() throws Exception {

resp = expectThrows(
ResponseException.class,
() -> runESQLCommand("metadata1_read2", "FROM index-user1,index-user2 | STATS sum=sum(value)", false)
() -> runESQLCommand("metadata1_read2", "FROM index-user1,index-user2 | STATS sum=sum(value)")
);
assertThat(
EntityUtils.toString(resp.getResponse().getEntity()),
Expand All @@ -372,7 +368,7 @@ public void testLimitedPrivilege() throws Exception {

resp = expectThrows(
ResponseException.class,
() -> runESQLCommand("alias_user1", "FROM first-alias,index-user1 METADATA _index | KEEP _index, org, value | LIMIT 10", false)
() -> runESQLCommand("alias_user1", "FROM first-alias,index-user1 METADATA _index | KEEP _index, org, value | LIMIT 10")
);
assertThat(
EntityUtils.toString(resp.getResponse().getEntity()),
Expand All @@ -386,8 +382,7 @@ public void testLimitedPrivilege() throws Exception {
ResponseException.class,
() -> runESQLCommand(
"alias_user2",
"from second-alias,index-user2 METADATA _index | stats sum=sum(value), index=VALUES(_index)",
false
"from second-alias,index-user2 METADATA _index | stats sum=sum(value), index=VALUES(_index)"
)
);
assertThat(
Expand Down Expand Up @@ -831,10 +826,6 @@ public void testDataStream() throws IOException {
}

protected Response runESQLCommand(String user, String command) throws IOException {
return runESQLCommand(user, command, null);
}

protected Response runESQLCommand(String user, String command, Boolean allowPartialResults) throws IOException {
if (command.toLowerCase(Locale.ROOT).contains("limit") == false) {
// add a (high) limit to avoid warnings on default limit
command += " | limit 10000000";
Expand All @@ -848,9 +839,6 @@ protected Response runESQLCommand(String user, String command, Boolean allowPart
request.setJsonEntity(Strings.toString(json));
request.setOptions(RequestOptions.DEFAULT.toBuilder().addHeader("es-security-runas-user", user));
request.addParameter("error_trace", "true");
if (allowPartialResults != null) {
request.addParameter("allow_partial_results", Boolean.toString(allowPartialResults));
}
return client().performRequest(request);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,6 @@ private RestClient remoteClusterClient() throws IOException {

@Before
public void skipTestOnOldVersions() {
assumeTrue("skip on old versions", Clusters.localClusterVersion().equals(Version.V_8_19_0));
assumeTrue("skip on old versions", Clusters.localClusterVersion().equals(Version.V_8_16_0));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
import org.junit.rules.TestRule;

import java.io.IOException;
import java.util.List;
import java.util.Map;

import static org.elasticsearch.test.MapMatcher.assertMap;
Expand Down Expand Up @@ -88,12 +87,6 @@ protected String from(String... indexName) {

@Override
public Map<String, Object> runEsql(RestEsqlTestCase.RequestObjectBuilder requestObject) throws IOException {
if (requestObject.allowPartialResults() != null) {
assumeTrue(
"require allow_partial_results on local cluster",
clusterHasCapability("POST", "/_query", List.of(), List.of("support_partial_results")).orElse(false)
);
}
requestObject.includeCCSMetadata(true);
return super.runEsql(requestObject);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ public void testInvalidPragma() throws IOException {
request.setJsonEntity("{\"f\":" + i + "}");
assertOK(client().performRequest(request));
}
RequestObjectBuilder builder = requestObjectBuilder().query("from test-index | limit 1 | keep f").allowPartialResults(false);
RequestObjectBuilder builder = requestObjectBuilder().query("from test-index | limit 1 | keep f");
builder.pragmas(Settings.builder().put("data_partitioning", "invalid-option").build());
ResponseException re = expectThrows(ResponseException.class, () -> runEsqlSync(builder));
assertThat(EntityUtils.toString(re.getResponse().getEntity()), containsString("No enum constant"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,6 @@ private Request createRequest(String indexName) throws IOException {
final var request = new Request("POST", "/_query");
request.addParameter("error_trace", "true");
request.addParameter("pretty", "true");
request.addParameter("allow_partial_results", Boolean.toString(false));
request.setJsonEntity(
Strings.toString(JsonXContent.contentBuilder().startObject().field("query", "from " + indexName).endObject())
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -198,26 +198,17 @@ public void testIndicesDontExist() throws IOException {
int docsTest1 = 0; // we are interested only in the created index, not necessarily that it has data
indexTimestampData(docsTest1, "test1", "2024-11-26", "id1");

ResponseException e = expectThrows(
ResponseException.class,
() -> runEsql(timestampFilter("gte", "2020-01-01").query(from("foo")).allowPartialResults(false))
);
ResponseException e = expectThrows(ResponseException.class, () -> runEsql(timestampFilter("gte", "2020-01-01").query(from("foo"))));
assertEquals(400, e.getResponse().getStatusLine().getStatusCode());
assertThat(e.getMessage(), containsString("verification_exception"));
assertThat(e.getMessage(), anyOf(containsString("Unknown index [foo]"), containsString("Unknown index [remote_cluster:foo]")));

e = expectThrows(
ResponseException.class,
() -> runEsql(timestampFilter("gte", "2020-01-01").query(from("foo*")).allowPartialResults(false))
);
e = expectThrows(ResponseException.class, () -> runEsql(timestampFilter("gte", "2020-01-01").query(from("foo*"))));
assertEquals(400, e.getResponse().getStatusLine().getStatusCode());
assertThat(e.getMessage(), containsString("verification_exception"));
assertThat(e.getMessage(), anyOf(containsString("Unknown index [foo*]"), containsString("Unknown index [remote_cluster:foo*]")));

e = expectThrows(
ResponseException.class,
() -> runEsql(timestampFilter("gte", "2020-01-01").query(from("foo", "test1")).allowPartialResults(false))
);
e = expectThrows(ResponseException.class, () -> runEsql(timestampFilter("gte", "2020-01-01").query(from("foo", "test1"))));
assertEquals(404, e.getResponse().getStatusLine().getStatusCode());
assertThat(e.getMessage(), containsString("index_not_found_exception"));
assertThat(e.getMessage(), anyOf(containsString("no such index [foo]"), containsString("no such index [remote_cluster:foo]")));
Expand All @@ -226,7 +217,7 @@ public void testIndicesDontExist() throws IOException {
var pattern = from("test1");
e = expectThrows(
ResponseException.class,
() -> runEsql(timestampFilter("gte", "2020-01-01").query(pattern + " | LOOKUP JOIN foo ON id1").allowPartialResults(false))
() -> runEsql(timestampFilter("gte", "2020-01-01").query(pattern + " | LOOKUP JOIN foo ON id1"))
);
assertEquals(400, e.getResponse().getStatusLine().getStatusCode());
assertThat(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ public static class RequestObjectBuilder {
private Boolean includeCCSMetadata = null;

private CheckedConsumer<XContentBuilder, IOException> filter;
private Boolean allowPartialResults = null;
private Boolean allPartialResults = null;

public RequestObjectBuilder() throws IOException {
this(randomFrom(XContentType.values()));
Expand Down Expand Up @@ -210,15 +210,11 @@ public RequestObjectBuilder filter(CheckedConsumer<XContentBuilder, IOException>
return this;
}

public RequestObjectBuilder allowPartialResults(boolean allowPartialResults) {
this.allowPartialResults = allowPartialResults;
public RequestObjectBuilder allPartialResults(boolean allPartialResults) {
this.allPartialResults = allPartialResults;
return this;
}

public Boolean allowPartialResults() {
return allowPartialResults;
}

public RequestObjectBuilder build() throws IOException {
if (isBuilt == false) {
if (tables != null) {
Expand Down Expand Up @@ -1373,8 +1369,8 @@ protected static Request prepareRequestWithOptions(RequestObjectBuilder requestO
requestObject.build();
Request request = prepareRequest(mode);
String mediaType = attachBody(requestObject, request);
if (requestObject.allowPartialResults != null) {
request.addParameter("allow_partial_results", String.valueOf(requestObject.allowPartialResults));
if (requestObject.allPartialResults != null) {
request.addParameter("allow_partial_results", String.valueOf(requestObject.allPartialResults));
}

RequestOptions.Builder options = request.getOptions().toBuilder();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import org.elasticsearch.transport.RemoteClusterAware;
import org.elasticsearch.xcontent.XContentBuilder;
import org.elasticsearch.xcontent.json.JsonXContent;
import org.elasticsearch.xpack.esql.plugin.EsqlPlugin;
import org.junit.After;
import org.junit.Before;

Expand Down Expand Up @@ -77,11 +76,6 @@ protected Collection<Class<? extends Plugin>> nodePlugins(String clusterAlias) {
return plugins;
}

@Override
protected Settings nodeSettings() {
return Settings.builder().put(super.nodeSettings()).put(EsqlPlugin.QUERY_ALLOW_PARTIAL_RESULTS.getKey(), false).build();
}

public static class InternalExchangePlugin extends Plugin {
@Override
public List<Setting<?>> getSettings() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,14 +139,6 @@ protected Collection<Class<? extends Plugin>> nodePlugins() {
return CollectionUtils.appendToCopy(super.nodePlugins(), EsqlPlugin.class);
}

@Override
protected Settings nodeSettings(int nodeOrdinal, Settings otherSettings) {
return Settings.builder()
.put(super.nodeSettings(nodeOrdinal, otherSettings))
.put(EsqlPlugin.QUERY_ALLOW_PARTIAL_RESULTS.getKey(), false)
.build();
}

protected void setRequestCircuitBreakerLimit(ByteSizeValue limit) {
if (limit != null) {
assertAcked(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,20 +14,25 @@
import org.elasticsearch.action.index.IndexRequest;
import org.elasticsearch.action.support.PlainActionFuture;
import org.elasticsearch.action.support.WriteRequest;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.settings.Setting;
import org.elasticsearch.common.transport.TransportAddress;
import org.elasticsearch.compute.operator.DriverTaskRunner;
import org.elasticsearch.compute.operator.exchange.ExchangeService;
import org.elasticsearch.core.TimeValue;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.tasks.TaskCancelledException;
import org.elasticsearch.tasks.TaskInfo;
import org.elasticsearch.test.AbstractMultiClustersTestCase;
import org.elasticsearch.transport.TransportService;
import org.elasticsearch.xcontent.XContentBuilder;
import org.elasticsearch.xcontent.json.JsonXContent;
import org.elasticsearch.xpack.esql.EsqlTestUtils;
import org.elasticsearch.xpack.esql.plugin.ComputeService;
import org.junit.After;
import org.junit.Before;

import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.concurrent.TimeUnit;

Expand All @@ -39,7 +44,7 @@
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.instanceOf;

public class CrossClusterCancellationIT extends AbstractCrossClusterTestCase {
public class CrossClusterCancellationIT extends AbstractMultiClustersTestCase {
private static final String REMOTE_CLUSTER = "cluster-a";

@Override
Expand All @@ -48,11 +53,35 @@ protected List<String> remoteClusterAlias() {
}

@Override
protected Settings nodeSettings() {
return Settings.builder()
.put(super.nodeSettings())
.put(ExchangeService.INACTIVE_SINKS_INTERVAL_SETTING, TimeValue.timeValueMillis(between(3000, 4000)))
.build();
protected Collection<Class<? extends Plugin>> nodePlugins(String clusterAlias) {
List<Class<? extends Plugin>> plugins = new ArrayList<>(super.nodePlugins(clusterAlias));
plugins.add(EsqlPluginWithEnterpriseOrTrialLicense.class);
plugins.add(InternalExchangePlugin.class);
plugins.add(SimplePauseFieldPlugin.class);
return plugins;
}

public static class InternalExchangePlugin extends Plugin {
@Override
public List<Setting<?>> getSettings() {
return List.of(
Setting.timeSetting(
ExchangeService.INACTIVE_SINKS_INTERVAL_SETTING,
TimeValue.timeValueMillis(between(3000, 4000)),
Setting.Property.NodeScope
)
);
}
}

@Before
public void resetPlugin() {
SimplePauseFieldPlugin.resetPlugin();
}

@After
public void releasePlugin() {
SimplePauseFieldPlugin.release();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ protected Collection<Class<? extends Plugin>> nodePlugins() {
@Override
protected Settings nodeSettings(int nodeOrdinal, Settings otherSettings) {
return Settings.builder()
.put(super.nodeSettings(nodeOrdinal, otherSettings))
.put(ExchangeService.INACTIVE_SINKS_INTERVAL_SETTING, TimeValue.timeValueMillis(between(3000, 5000)))
.build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ public class EsqlPlugin extends Plugin implements ActionPlugin {

public static final Setting<Boolean> QUERY_ALLOW_PARTIAL_RESULTS = Setting.boolSetting(
"esql.query.allow_partial_results",
true,
false,
Setting.Property.NodeScope,
Setting.Property.Dynamic
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ public class CrossClusterEsqlRCS1MissingIndicesIT extends AbstractRemoteClusterS
.apply(commonClusterConfig)
.setting("remote_cluster.port", "0")
.setting("xpack.ml.enabled", "false")
.setting("esql.query.allow_partial_results", "false")
.setting("xpack.security.remote_cluster_server.ssl.enabled", () -> String.valueOf(SSL_ENABLED_REF.get()))
.setting("xpack.security.remote_cluster_server.ssl.key", "remote-cluster.key")
.setting("xpack.security.remote_cluster_server.ssl.certificate", "remote-cluster.crt")
Expand All @@ -63,7 +62,6 @@ public class CrossClusterEsqlRCS1MissingIndicesIT extends AbstractRemoteClusterS
.module("x-pack-enrich")
.apply(commonClusterConfig)
.setting("xpack.ml.enabled", "false")
.setting("esql.query.allow_partial_results", "false")
.setting("xpack.security.remote_cluster_client.ssl.enabled", () -> String.valueOf(SSL_ENABLED_REF.get()))
.build();
}
Expand Down
Loading
Loading