From fcf476c18bede95225aa0d5446ac0cc4ca8f5465 Mon Sep 17 00:00:00 2001 From: Stanislav Malyshev Date: Thu, 24 Apr 2025 13:08:16 -0600 Subject: [PATCH 01/10] Revert "ESQL: Revert "Allow partial results by default in ES|QL (#125060)" (#126286)" This reverts commit 8f38b13059ed39a7dbea63c525479e30b94c772c. Restore changes from https://github.com/elastic/elasticsearch/pull/125060 now that the breakage should be fixed. --- docs/changelog/125060.yaml | 16 +++++++ docs/changelog/126286.yaml | 6 --- docs/release-notes/breaking-changes.md | 5 +++ .../xpack/esql/heap_attack/Clusters.java | 1 + x-pack/plugin/build.gradle | 1 + .../xpack/esql/EsqlSecurityIT.java | 24 ++++++++--- .../xpack/esql/ccq/EsqlRestValidationIT.java | 2 +- .../esql/ccq/RequestIndexFilteringIT.java | 7 +++ .../xpack/esql/qa/single_node/RestEsqlIT.java | 2 +- .../qa/rest/EsqlRestValidationTestCase.java | 1 + .../rest/RequestIndexFilteringTestCase.java | 17 ++++++-- .../xpack/esql/qa/rest/RestEsqlTestCase.java | 14 +++--- .../action/AbstractCrossClusterTestCase.java | 6 +++ .../action/AbstractEsqlIntegTestCase.java | 8 ++++ .../action/CrossClusterCancellationIT.java | 43 +++---------------- .../xpack/esql/action/ManyShardsIT.java | 1 + .../xpack/esql/plugin/EsqlPlugin.java | 2 +- .../CrossClusterEsqlRCS1MissingIndicesIT.java | 2 + .../RemoteClusterSecurityEsqlIT.java | 2 + 19 files changed, 100 insertions(+), 60 deletions(-) create mode 100644 docs/changelog/125060.yaml delete mode 100644 docs/changelog/126286.yaml diff --git a/docs/changelog/125060.yaml b/docs/changelog/125060.yaml new file mode 100644 index 0000000000000..3c5b04810a258 --- /dev/null +++ b/docs/changelog/125060.yaml @@ -0,0 +1,16 @@ +pr: 125060 +summary: Allow partial results by default in ES|QL +area: ES|QL +type: breaking +issues: [122802] + +breaking: + title: Allow partial results by default in ES|QL + area: ES|QL + details: >- + In earlier versions of {es}, ES|QL would fail the entire query if it encountered any error. ES|QL now returns partial results instead of failing when encountering errors. + + impact: >- + Callers should check the `is_partial` flag returned in the response to determine if the result is partial or complete. If returning partial results is not desired, this option can be overridden per request via an `allow_partial_results` parameter in the query URL or globally via the cluster setting `esql.query.allow_partial_results`. + + notable: true diff --git a/docs/changelog/126286.yaml b/docs/changelog/126286.yaml deleted file mode 100644 index 3e8b1604e4670..0000000000000 --- a/docs/changelog/126286.yaml +++ /dev/null @@ -1,6 +0,0 @@ -pr: 126286 -summary: Revert "Allow partial results by default in ES|QL" -area: ES|QL -type: bug -issues: - - 126275 diff --git a/docs/release-notes/breaking-changes.md b/docs/release-notes/breaking-changes.md index 2a7f4ef731e3a..cc8fac24dc8ce 100644 --- a/docs/release-notes/breaking-changes.md +++ b/docs/release-notes/breaking-changes.md @@ -12,6 +12,11 @@ If you are migrating from a version prior to version 9.0, you must first upgrade % ## 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] Aggregations: diff --git a/test/external-modules/esql-heap-attack/src/javaRestTest/java/org/elasticsearch/xpack/esql/heap_attack/Clusters.java b/test/external-modules/esql-heap-attack/src/javaRestTest/java/org/elasticsearch/xpack/esql/heap_attack/Clusters.java index 9fe176ca6be9d..1c237404a78cc 100644 --- a/test/external-modules/esql-heap-attack/src/javaRestTest/java/org/elasticsearch/xpack/esql/heap_attack/Clusters.java +++ b/test/external-modules/esql-heap-attack/src/javaRestTest/java/org/elasticsearch/xpack/esql/heap_attack/Clusters.java @@ -21,6 +21,7 @@ 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")) { diff --git a/x-pack/plugin/build.gradle b/x-pack/plugin/build.gradle index 38a4017a5cbef..7adcf50a256e2 100644 --- a/x-pack/plugin/build.gradle +++ b/x-pack/plugin/build.gradle @@ -125,6 +125,7 @@ tasks.named("yamlRestCompatTestTransform").configure({ task -> task.replaceValueInMatch("Size", 49, "Test flamegraph from profiling-events") task.replaceValueInMatch("Size", 49, "Test flamegraph from test-events") task.skipTest("esql/90_non_indexed/fetch", "Temporary until backported") + task.skipTest("esql/63_enrich_int_range/Invalid age as double", "TODO: require disable allow_partial_results") }) tasks.named('yamlRestCompatTest').configure { diff --git a/x-pack/plugin/esql/qa/security/src/javaRestTest/java/org/elasticsearch/xpack/esql/EsqlSecurityIT.java b/x-pack/plugin/esql/qa/security/src/javaRestTest/java/org/elasticsearch/xpack/esql/EsqlSecurityIT.java index 8f4c1f3980272..8982dd6676811 100644 --- a/x-pack/plugin/esql/qa/security/src/javaRestTest/java/org/elasticsearch/xpack/esql/EsqlSecurityIT.java +++ b/x-pack/plugin/esql/qa/security/src/javaRestTest/java/org/elasticsearch/xpack/esql/EsqlSecurityIT.java @@ -315,7 +315,10 @@ public void testIndexPatternErrorMessageComparison_ESQL_SearchDSL() throws Excep setUser(searchRequest, "metadata1_read2"); // ES|QL query on the same index pattern - var esqlResp = expectThrows(ResponseException.class, () -> runESQLCommand("metadata1_read2", "FROM index-user1,index-user2")); + var esqlResp = expectThrows( + ResponseException.class, + () -> runESQLCommand("metadata1_read2", "FROM index-user1,index-user2", false) + ); var srchResp = expectThrows(ResponseException.class, () -> client().performRequest(searchRequest)); for (ResponseException r : List.of(esqlResp, srchResp)) { @@ -334,7 +337,8 @@ public void testLimitedPrivilege() throws Exception { ResponseException.class, () -> runESQLCommand( "metadata1_read2", - "FROM index-user1,index-user2 METADATA _index | STATS sum=sum(value), index=VALUES(_index)" + "FROM index-user1,index-user2 METADATA _index | STATS sum=sum(value), index=VALUES(_index)", + false ) ); assertThat( @@ -347,7 +351,7 @@ public void testLimitedPrivilege() throws Exception { resp = expectThrows( ResponseException.class, - () -> runESQLCommand("metadata1_read2", "FROM index-user1,index-user2 METADATA _index | STATS index=VALUES(_index)") + () -> runESQLCommand("metadata1_read2", "FROM index-user1,index-user2 METADATA _index | STATS index=VALUES(_index)", false) ); assertThat( EntityUtils.toString(resp.getResponse().getEntity()), @@ -359,7 +363,7 @@ public void testLimitedPrivilege() throws Exception { resp = expectThrows( ResponseException.class, - () -> runESQLCommand("metadata1_read2", "FROM index-user1,index-user2 | STATS sum=sum(value)") + () -> runESQLCommand("metadata1_read2", "FROM index-user1,index-user2 | STATS sum=sum(value)", false) ); assertThat( EntityUtils.toString(resp.getResponse().getEntity()), @@ -371,7 +375,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") + () -> runESQLCommand("alias_user1", "FROM first-alias,index-user1 METADATA _index | KEEP _index, org, value | LIMIT 10", false) ); assertThat( EntityUtils.toString(resp.getResponse().getEntity()), @@ -385,7 +389,8 @@ public void testLimitedPrivilege() throws Exception { ResponseException.class, () -> runESQLCommand( "alias_user2", - "from second-alias,index-user2 METADATA _index | stats sum=sum(value), index=VALUES(_index)" + "from second-alias,index-user2 METADATA _index | stats sum=sum(value), index=VALUES(_index)", + false ) ); assertThat( @@ -859,6 +864,10 @@ 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"; @@ -872,6 +881,9 @@ protected Response runESQLCommand(String user, String command) throws IOExceptio request.setJsonEntity(Strings.toString(json)); setUser(request, user); request.addParameter("error_trace", "true"); + if (allowPartialResults != null) { + request.addParameter("allow_partial_results", Boolean.toString(allowPartialResults)); + } return client().performRequest(request); } diff --git a/x-pack/plugin/esql/qa/server/multi-clusters/src/javaRestTest/java/org/elasticsearch/xpack/esql/ccq/EsqlRestValidationIT.java b/x-pack/plugin/esql/qa/server/multi-clusters/src/javaRestTest/java/org/elasticsearch/xpack/esql/ccq/EsqlRestValidationIT.java index 55500aa1c9537..aed164f9e8873 100644 --- a/x-pack/plugin/esql/qa/server/multi-clusters/src/javaRestTest/java/org/elasticsearch/xpack/esql/ccq/EsqlRestValidationIT.java +++ b/x-pack/plugin/esql/qa/server/multi-clusters/src/javaRestTest/java/org/elasticsearch/xpack/esql/ccq/EsqlRestValidationIT.java @@ -83,6 +83,6 @@ private RestClient remoteClusterClient() throws IOException { @Before public void skipTestOnOldVersions() { - assumeTrue("skip on old versions", Clusters.localClusterVersion().equals(Version.V_8_16_0)); + assumeTrue("skip on old versions", Clusters.localClusterVersion().equals(Version.V_8_19_0)); } } diff --git a/x-pack/plugin/esql/qa/server/multi-clusters/src/javaRestTest/java/org/elasticsearch/xpack/esql/ccq/RequestIndexFilteringIT.java b/x-pack/plugin/esql/qa/server/multi-clusters/src/javaRestTest/java/org/elasticsearch/xpack/esql/ccq/RequestIndexFilteringIT.java index d8c68dd5281aa..5011b2d1c593e 100644 --- a/x-pack/plugin/esql/qa/server/multi-clusters/src/javaRestTest/java/org/elasticsearch/xpack/esql/ccq/RequestIndexFilteringIT.java +++ b/x-pack/plugin/esql/qa/server/multi-clusters/src/javaRestTest/java/org/elasticsearch/xpack/esql/ccq/RequestIndexFilteringIT.java @@ -28,6 +28,7 @@ import org.junit.rules.TestRule; import java.io.IOException; +import java.util.List; import java.util.Map; import static org.elasticsearch.test.MapMatcher.assertMap; @@ -87,6 +88,12 @@ protected String from(String... indexName) { @Override public Map 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); } diff --git a/x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/RestEsqlIT.java b/x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/RestEsqlIT.java index 56f34d4178768..31addd7022076 100644 --- a/x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/RestEsqlIT.java +++ b/x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/RestEsqlIT.java @@ -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"); + RequestObjectBuilder builder = requestObjectBuilder().query("from test-index | limit 1 | keep f").allowPartialResults(false); 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")); diff --git a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/EsqlRestValidationTestCase.java b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/EsqlRestValidationTestCase.java index 9ec4f60f4c843..6a66d5a4270c7 100644 --- a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/EsqlRestValidationTestCase.java +++ b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/EsqlRestValidationTestCase.java @@ -129,6 +129,7 @@ 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()) ); diff --git a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RequestIndexFilteringTestCase.java b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RequestIndexFilteringTestCase.java index 1fdc11174ee09..0183e180c15d8 100644 --- a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RequestIndexFilteringTestCase.java +++ b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RequestIndexFilteringTestCase.java @@ -198,17 +198,26 @@ 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")))); + ResponseException e = expectThrows( + ResponseException.class, + () -> runEsql(timestampFilter("gte", "2020-01-01").query(from("foo")).allowPartialResults(false)) + ); 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*")))); + e = expectThrows( + ResponseException.class, + () -> runEsql(timestampFilter("gte", "2020-01-01").query(from("foo*")).allowPartialResults(false)) + ); 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")))); + e = expectThrows( + ResponseException.class, + () -> runEsql(timestampFilter("gte", "2020-01-01").query(from("foo", "test1")).allowPartialResults(false)) + ); 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]"))); @@ -217,7 +226,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")) + () -> runEsql(timestampFilter("gte", "2020-01-01").query(pattern + " | LOOKUP JOIN foo ON id1").allowPartialResults(false)) ); assertEquals(400, e.getResponse().getStatusLine().getStatusCode()); assertThat( diff --git a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java index 32dab37b02c0d..cb476cb5f8ef2 100644 --- a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java +++ b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java @@ -131,7 +131,7 @@ public static class RequestObjectBuilder { private Boolean includeCCSMetadata = null; private CheckedConsumer filter; - private Boolean allPartialResults = null; + private Boolean allowPartialResults = null; public RequestObjectBuilder() throws IOException { this(randomFrom(XContentType.values())); @@ -209,11 +209,15 @@ public RequestObjectBuilder filter(CheckedConsumer return this; } - public RequestObjectBuilder allPartialResults(boolean allPartialResults) { - this.allPartialResults = allPartialResults; + public RequestObjectBuilder allowPartialResults(boolean allowPartialResults) { + this.allowPartialResults = allowPartialResults; return this; } + public Boolean allowPartialResults() { + return allowPartialResults; + } + public RequestObjectBuilder build() throws IOException { if (isBuilt == false) { if (tables != null) { @@ -1376,8 +1380,8 @@ protected static Request prepareRequestWithOptions(RequestObjectBuilder requestO requestObject.build(); Request request = prepareRequest(mode); String mediaType = attachBody(requestObject, request); - if (requestObject.allPartialResults != null) { - request.addParameter("allow_partial_results", String.valueOf(requestObject.allPartialResults)); + if (requestObject.allowPartialResults != null) { + request.addParameter("allow_partial_results", String.valueOf(requestObject.allowPartialResults)); } RequestOptions.Builder options = request.getOptions().toBuilder(); diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractCrossClusterTestCase.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractCrossClusterTestCase.java index bea757cbf484f..992572fc3220d 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractCrossClusterTestCase.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractCrossClusterTestCase.java @@ -25,6 +25,7 @@ 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; @@ -76,6 +77,11 @@ protected Collection> 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> getSettings() { diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractEsqlIntegTestCase.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractEsqlIntegTestCase.java index 5406f3f773205..00d30066899ae 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractEsqlIntegTestCase.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractEsqlIntegTestCase.java @@ -139,6 +139,14 @@ protected Collection> 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( diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClusterCancellationIT.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClusterCancellationIT.java index 8131af2da408d..191c44cec51d8 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClusterCancellationIT.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClusterCancellationIT.java @@ -14,25 +14,20 @@ import org.elasticsearch.action.index.IndexRequest; import org.elasticsearch.action.support.PlainActionFuture; import org.elasticsearch.action.support.WriteRequest; -import org.elasticsearch.common.settings.Setting; +import org.elasticsearch.common.settings.Settings; 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; @@ -44,7 +39,7 @@ import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.instanceOf; -public class CrossClusterCancellationIT extends AbstractMultiClustersTestCase { +public class CrossClusterCancellationIT extends AbstractCrossClusterTestCase { private static final String REMOTE_CLUSTER = "cluster-a"; @Override @@ -53,35 +48,11 @@ protected List remoteClusterAlias() { } @Override - protected Collection> nodePlugins(String clusterAlias) { - List> 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> 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(); + protected Settings nodeSettings() { + return Settings.builder() + .put(super.nodeSettings()) + .put(ExchangeService.INACTIVE_SINKS_INTERVAL_SETTING, TimeValue.timeValueMillis(between(3000, 4000))) + .build(); } @Override diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/ManyShardsIT.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/ManyShardsIT.java index 506c143b85150..f17eb18e0c371 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/ManyShardsIT.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/ManyShardsIT.java @@ -71,6 +71,7 @@ protected Collection> 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(); } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/EsqlPlugin.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/EsqlPlugin.java index 2657cda192308..d4544eb0e1efe 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/EsqlPlugin.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/EsqlPlugin.java @@ -107,7 +107,7 @@ public class EsqlPlugin extends Plugin implements ActionPlugin { public static final Setting QUERY_ALLOW_PARTIAL_RESULTS = Setting.boolSetting( "esql.query.allow_partial_results", - false, + true, Setting.Property.NodeScope, Setting.Property.Dynamic ); diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/CrossClusterEsqlRCS1MissingIndicesIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/CrossClusterEsqlRCS1MissingIndicesIT.java index 23f33b2351c21..f5c06129eb025 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/CrossClusterEsqlRCS1MissingIndicesIT.java +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/CrossClusterEsqlRCS1MissingIndicesIT.java @@ -47,6 +47,7 @@ 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") @@ -62,6 +63,7 @@ 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(); } diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityEsqlIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityEsqlIT.java index dcf993ea4ce7a..7f65d38bd049e 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityEsqlIT.java +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityEsqlIT.java @@ -78,6 +78,7 @@ public class RemoteClusterSecurityEsqlIT extends AbstractRemoteClusterSecurityTe .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") @@ -97,6 +98,7 @@ public class RemoteClusterSecurityEsqlIT extends AbstractRemoteClusterSecurityTe .module("ingest-common") .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())) .setting("xpack.security.remote_cluster_client.ssl.certificate_authorities", "remote-cluster-ca.crt") .setting("xpack.security.authc.token.enabled", "true") From 4700e42e1e17db29cd6fccb7bde4a806b76dd70e Mon Sep 17 00:00:00 2001 From: Stanislav Malyshev Date: Thu, 24 Apr 2025 14:26:23 -0600 Subject: [PATCH 02/10] This test should work as is --- .../qa/rest/RequestIndexFilteringTestCase.java | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RequestIndexFilteringTestCase.java b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RequestIndexFilteringTestCase.java index 0183e180c15d8..1fdc11174ee09 100644 --- a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RequestIndexFilteringTestCase.java +++ b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RequestIndexFilteringTestCase.java @@ -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]"))); @@ -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( From 4dbf58fc7a07fdf8e54d0a0c8842cf1debc51d56 Mon Sep 17 00:00:00 2001 From: Stanislav Malyshev Date: Thu, 24 Apr 2025 14:31:58 -0600 Subject: [PATCH 03/10] Unpatch the tests --- .../remotecluster/CrossClusterEsqlRCS1MissingIndicesIT.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/CrossClusterEsqlRCS1MissingIndicesIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/CrossClusterEsqlRCS1MissingIndicesIT.java index f5c06129eb025..23f33b2351c21 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/CrossClusterEsqlRCS1MissingIndicesIT.java +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/CrossClusterEsqlRCS1MissingIndicesIT.java @@ -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") @@ -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(); } From 0f4b1b21ec59bc170160adcac1918dadeb85ef25 Mon Sep 17 00:00:00 2001 From: Stanislav Malyshev Date: Thu, 24 Apr 2025 14:39:57 -0600 Subject: [PATCH 04/10] Fix test --- .../xpack/security/failurestore/FailureStoreSecurityRestIT.java | 1 + 1 file changed, 1 insertion(+) diff --git a/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/failurestore/FailureStoreSecurityRestIT.java b/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/failurestore/FailureStoreSecurityRestIT.java index d1ae6abc70125..68d6145985baf 100644 --- a/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/failurestore/FailureStoreSecurityRestIT.java +++ b/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/failurestore/FailureStoreSecurityRestIT.java @@ -2771,6 +2771,7 @@ Request toEsqlRequest() throws IOException { json.endObject(); var request = new Request("POST", "_query"); request.setJsonEntity(Strings.toString(json)); + request.addParameter("allow_partial_results", Boolean.toString(false)); return request; } } From 7ae2631bdf6087405ca97bde9c97bfce84683d58 Mon Sep 17 00:00:00 2001 From: Stanislav Malyshev Date: Thu, 24 Apr 2025 15:50:14 -0600 Subject: [PATCH 05/10] Add security exceptions to the list of fatals and remove test overrides --- .../xpack/esql/EsqlSecurityIT.java | 24 +++++-------------- .../xpack/esql/qa/single_node/RestEsqlIT.java | 2 +- .../qa/rest/EsqlRestValidationTestCase.java | 1 - .../action/AbstractCrossClusterTestCase.java | 5 ---- .../action/AbstractEsqlIntegTestCase.java | 8 ------- .../esql/plugin/ClusterComputeHandler.java | 20 +++++----------- .../xpack/esql/plugin/ComputeService.java | 6 ++--- .../xpack/esql/session/EsqlCCSUtils.java | 15 ++++++++++++ .../RemoteClusterSecurityEsqlIT.java | 2 -- .../FailureStoreSecurityRestIT.java | 1 - 10 files changed, 30 insertions(+), 54 deletions(-) diff --git a/x-pack/plugin/esql/qa/security/src/javaRestTest/java/org/elasticsearch/xpack/esql/EsqlSecurityIT.java b/x-pack/plugin/esql/qa/security/src/javaRestTest/java/org/elasticsearch/xpack/esql/EsqlSecurityIT.java index 8982dd6676811..8f4c1f3980272 100644 --- a/x-pack/plugin/esql/qa/security/src/javaRestTest/java/org/elasticsearch/xpack/esql/EsqlSecurityIT.java +++ b/x-pack/plugin/esql/qa/security/src/javaRestTest/java/org/elasticsearch/xpack/esql/EsqlSecurityIT.java @@ -315,10 +315,7 @@ public void testIndexPatternErrorMessageComparison_ESQL_SearchDSL() throws Excep setUser(searchRequest, "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)) { @@ -337,8 +334,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( @@ -351,7 +347,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()), @@ -363,7 +359,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()), @@ -375,7 +371,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()), @@ -389,8 +385,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( @@ -864,10 +859,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"; @@ -881,9 +872,6 @@ protected Response runESQLCommand(String user, String command, Boolean allowPart request.setJsonEntity(Strings.toString(json)); setUser(request, user); request.addParameter("error_trace", "true"); - if (allowPartialResults != null) { - request.addParameter("allow_partial_results", Boolean.toString(allowPartialResults)); - } return client().performRequest(request); } diff --git a/x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/RestEsqlIT.java b/x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/RestEsqlIT.java index 31addd7022076..56f34d4178768 100644 --- a/x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/RestEsqlIT.java +++ b/x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/RestEsqlIT.java @@ -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")); diff --git a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/EsqlRestValidationTestCase.java b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/EsqlRestValidationTestCase.java index 6a66d5a4270c7..9ec4f60f4c843 100644 --- a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/EsqlRestValidationTestCase.java +++ b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/EsqlRestValidationTestCase.java @@ -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()) ); diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractCrossClusterTestCase.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractCrossClusterTestCase.java index 992572fc3220d..260345aabab7e 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractCrossClusterTestCase.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractCrossClusterTestCase.java @@ -77,11 +77,6 @@ protected Collection> 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> getSettings() { diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractEsqlIntegTestCase.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractEsqlIntegTestCase.java index 00d30066899ae..5406f3f773205 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractEsqlIntegTestCase.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractEsqlIntegTestCase.java @@ -139,14 +139,6 @@ protected Collection> 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( diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/ClusterComputeHandler.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/ClusterComputeHandler.java index 8d8df698b4b9e..059c933ea11fa 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/ClusterComputeHandler.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/ClusterComputeHandler.java @@ -7,7 +7,6 @@ package org.elasticsearch.xpack.esql.plugin; -import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.ActionListenerResponseHandler; import org.elasticsearch.action.OriginalIndices; @@ -17,7 +16,6 @@ import org.elasticsearch.compute.operator.exchange.ExchangeSourceHandler; import org.elasticsearch.core.Releasable; import org.elasticsearch.core.TimeValue; -import org.elasticsearch.index.IndexNotFoundException; import org.elasticsearch.tasks.CancellableTask; import org.elasticsearch.tasks.Task; import org.elasticsearch.tasks.TaskCancelledException; @@ -90,18 +88,12 @@ void startComputeOnRemoteCluster( if (receivedResults == false && EsqlCCSUtils.shouldIgnoreRuntimeError(executionInfo, clusterAlias, e)) { EsqlCCSUtils.markClusterWithFinalStateAndNoShards(executionInfo, clusterAlias, EsqlExecutionInfo.Cluster.Status.SKIPPED, e); l.onResponse(DriverCompletionInfo.EMPTY); - } else if (configuration.allowPartialResults() - && (ExceptionsHelper.unwrapCause(e) instanceof IndexNotFoundException) == false) { - EsqlCCSUtils.markClusterWithFinalStateAndNoShards( - executionInfo, - clusterAlias, - EsqlExecutionInfo.Cluster.Status.PARTIAL, - e - ); - l.onResponse(DriverCompletionInfo.EMPTY); - } else { - l.onFailure(e); - } + } else if (configuration.allowPartialResults() && EsqlCCSUtils.canAllowPartial(e)) { + EsqlCCSUtils.markClusterWithFinalStateAndNoShards(executionInfo, clusterAlias, EsqlExecutionInfo.Cluster.Status.PARTIAL, e); + l.onResponse(DriverCompletionInfo.EMPTY); + } else { + l.onFailure(e); + } }); ExchangeService.openExchange( transportService, diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/ComputeService.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/ComputeService.java index 00443a553da94..dc2b90d39928e 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/ComputeService.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/ComputeService.java @@ -7,7 +7,6 @@ package org.elasticsearch.xpack.esql.plugin; -import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.OriginalIndices; import org.elasticsearch.action.search.SearchRequest; @@ -31,7 +30,6 @@ import org.elasticsearch.core.Releasable; import org.elasticsearch.core.Releasables; import org.elasticsearch.core.Tuple; -import org.elasticsearch.index.IndexNotFoundException; import org.elasticsearch.index.query.SearchExecutionContext; import org.elasticsearch.logging.LogManager; import org.elasticsearch.logging.Logger; @@ -63,6 +61,7 @@ import org.elasticsearch.xpack.esql.planner.LocalExecutionPlanner; import org.elasticsearch.xpack.esql.planner.PlannerUtils; import org.elasticsearch.xpack.esql.session.Configuration; +import org.elasticsearch.xpack.esql.session.EsqlCCSUtils; import org.elasticsearch.xpack.esql.session.Result; import java.util.ArrayList; @@ -433,8 +432,7 @@ public void executePlan( ); dataNodesListener.onResponse(r.getCompletionInfo()); }, e -> { - if (configuration.allowPartialResults() - && (ExceptionsHelper.unwrapCause(e) instanceof IndexNotFoundException) == false) { + if (configuration.allowPartialResults() && EsqlCCSUtils.canAllowPartial(e)) { execInfo.swapCluster( LOCAL_CLUSTER, (k, v) -> new EsqlExecutionInfo.Cluster.Builder(v).setStatus( diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlCCSUtils.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlCCSUtils.java index ed324cbc2e959..2d2fb7f175888 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlCCSUtils.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlCCSUtils.java @@ -7,6 +7,8 @@ package org.elasticsearch.xpack.esql.session; +import org.elasticsearch.ElasticsearchException; +import org.elasticsearch.ElasticsearchSecurityException; import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.OriginalIndices; @@ -17,6 +19,7 @@ import org.elasticsearch.common.util.set.Sets; import org.elasticsearch.compute.operator.DriverCompletionInfo; import org.elasticsearch.core.Nullable; +import org.elasticsearch.index.IndexNotFoundException; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.indices.IndicesExpressionGrouper; import org.elasticsearch.license.XPackLicenseState; @@ -368,4 +371,16 @@ public static boolean shouldIgnoreRuntimeError(EsqlExecutionInfo executionInfo, return ExceptionsHelper.isRemoteUnavailableException(e); } + + /** + * Check whether this exception can be tolerated when partial results are on, or should be treated as fatal. + * @return true if the exception can be tolerated, false if it should be treated as fatal. + */ + public static boolean canAllowPartial(Exception e) { + Throwable unwrapped = ExceptionsHelper.unwrapCause(e); + if (unwrapped instanceof IndexNotFoundException || unwrapped instanceof ElasticsearchSecurityException) { + return false; + } + return true; + } } diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityEsqlIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityEsqlIT.java index 7f65d38bd049e..dcf993ea4ce7a 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityEsqlIT.java +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityEsqlIT.java @@ -78,7 +78,6 @@ public class RemoteClusterSecurityEsqlIT extends AbstractRemoteClusterSecurityTe .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") @@ -98,7 +97,6 @@ public class RemoteClusterSecurityEsqlIT extends AbstractRemoteClusterSecurityTe .module("ingest-common") .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())) .setting("xpack.security.remote_cluster_client.ssl.certificate_authorities", "remote-cluster-ca.crt") .setting("xpack.security.authc.token.enabled", "true") diff --git a/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/failurestore/FailureStoreSecurityRestIT.java b/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/failurestore/FailureStoreSecurityRestIT.java index 68d6145985baf..d1ae6abc70125 100644 --- a/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/failurestore/FailureStoreSecurityRestIT.java +++ b/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/failurestore/FailureStoreSecurityRestIT.java @@ -2771,7 +2771,6 @@ Request toEsqlRequest() throws IOException { json.endObject(); var request = new Request("POST", "_query"); request.setJsonEntity(Strings.toString(json)); - request.addParameter("allow_partial_results", Boolean.toString(false)); return request; } } From d0085d0a704eac3f02f130253d3b1411189c063c Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Thu, 24 Apr 2025 21:58:38 +0000 Subject: [PATCH 06/10] [CI] Auto commit changes from spotless --- .../xpack/esql/action/AbstractCrossClusterTestCase.java | 1 - .../java/org/elasticsearch/xpack/esql/session/EsqlCCSUtils.java | 1 - 2 files changed, 2 deletions(-) diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractCrossClusterTestCase.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractCrossClusterTestCase.java index 260345aabab7e..bea757cbf484f 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractCrossClusterTestCase.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractCrossClusterTestCase.java @@ -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; diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlCCSUtils.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlCCSUtils.java index 2d2fb7f175888..ccbfeaccf24a9 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlCCSUtils.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlCCSUtils.java @@ -7,7 +7,6 @@ package org.elasticsearch.xpack.esql.session; -import org.elasticsearch.ElasticsearchException; import org.elasticsearch.ElasticsearchSecurityException; import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.action.ActionListener; From 9a3fff196de13e0b5b9c181078f6315d00385e76 Mon Sep 17 00:00:00 2001 From: Stanislav Malyshev Date: Thu, 24 Apr 2025 17:59:23 -0600 Subject: [PATCH 07/10] Restore some overrides --- .../xpack/esql/action/AbstractCrossClusterTestCase.java | 6 ++++++ .../xpack/esql/action/AbstractEsqlIntegTestCase.java | 8 ++++++++ 2 files changed, 14 insertions(+) diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractCrossClusterTestCase.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractCrossClusterTestCase.java index bea757cbf484f..992572fc3220d 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractCrossClusterTestCase.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractCrossClusterTestCase.java @@ -25,6 +25,7 @@ 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; @@ -76,6 +77,11 @@ protected Collection> 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> getSettings() { diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractEsqlIntegTestCase.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractEsqlIntegTestCase.java index 5406f3f773205..00d30066899ae 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractEsqlIntegTestCase.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractEsqlIntegTestCase.java @@ -139,6 +139,14 @@ protected Collection> 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( From 671a029570292845f076df6def142dc7cf9ef74c Mon Sep 17 00:00:00 2001 From: Stanislav Malyshev Date: Thu, 24 Apr 2025 18:45:55 -0600 Subject: [PATCH 08/10] this test needs the tweak too --- .../org/elasticsearch/xpack/esql/qa/single_node/RestEsqlIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/RestEsqlIT.java b/x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/RestEsqlIT.java index 56f34d4178768..31addd7022076 100644 --- a/x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/RestEsqlIT.java +++ b/x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/RestEsqlIT.java @@ -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"); + RequestObjectBuilder builder = requestObjectBuilder().query("from test-index | limit 1 | keep f").allowPartialResults(false); 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")); From d8e2d1a63d8cb127185cfb01bac42f6d4dd807c5 Mon Sep 17 00:00:00 2001 From: Stanislav Malyshev Date: Fri, 25 Apr 2025 01:09:40 -0600 Subject: [PATCH 09/10] Update docs/changelog/127351.yaml --- docs/changelog/127351.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 docs/changelog/127351.yaml diff --git a/docs/changelog/127351.yaml b/docs/changelog/127351.yaml new file mode 100644 index 0000000000000..06a6b63f6568e --- /dev/null +++ b/docs/changelog/127351.yaml @@ -0,0 +1,5 @@ +pr: 127351 +summary: Allow partial results by default in ES|QL - Take 2 +area: ES|QL +type: enhancement +issues: [] From af30623fff4f69722dec448d52bf53c151a77456 Mon Sep 17 00:00:00 2001 From: Stanislav Malyshev Date: Fri, 25 Apr 2025 01:11:07 -0600 Subject: [PATCH 10/10] Fix changelog --- docs/changelog/125060.yaml | 16 ---------------- docs/changelog/127351.yaml | 17 ++++++++++++++--- 2 files changed, 14 insertions(+), 19 deletions(-) delete mode 100644 docs/changelog/125060.yaml diff --git a/docs/changelog/125060.yaml b/docs/changelog/125060.yaml deleted file mode 100644 index 3c5b04810a258..0000000000000 --- a/docs/changelog/125060.yaml +++ /dev/null @@ -1,16 +0,0 @@ -pr: 125060 -summary: Allow partial results by default in ES|QL -area: ES|QL -type: breaking -issues: [122802] - -breaking: - title: Allow partial results by default in ES|QL - area: ES|QL - details: >- - In earlier versions of {es}, ES|QL would fail the entire query if it encountered any error. ES|QL now returns partial results instead of failing when encountering errors. - - impact: >- - Callers should check the `is_partial` flag returned in the response to determine if the result is partial or complete. If returning partial results is not desired, this option can be overridden per request via an `allow_partial_results` parameter in the query URL or globally via the cluster setting `esql.query.allow_partial_results`. - - notable: true diff --git a/docs/changelog/127351.yaml b/docs/changelog/127351.yaml index 06a6b63f6568e..ca16c5695f004 100644 --- a/docs/changelog/127351.yaml +++ b/docs/changelog/127351.yaml @@ -1,5 +1,16 @@ pr: 127351 -summary: Allow partial results by default in ES|QL - Take 2 +summary: Allow partial results by default in ES|QL area: ES|QL -type: enhancement -issues: [] +type: breaking +issues: [122802] + +breaking: + title: Allow partial results by default in ES|QL + area: ES|QL + details: >- + In earlier versions of {es}, ES|QL would fail the entire query if it encountered any error. ES|QL now returns partial results instead of failing when encountering errors. + + impact: >- + Callers should check the `is_partial` flag returned in the response to determine if the result is partial or complete. If returning partial results is not desired, this option can be overridden per request via an `allow_partial_results` parameter in the query URL or globally via the cluster setting `esql.query.allow_partial_results`. + + notable: true