Skip to content

Commit 8f38b13

Browse files
alex-spiesalex-spies
authored
ESQL: Revert "Allow partial results by default in ES|QL (#125060)" (#126286)
This reverts commit 81555cc from #125060. Fix #126275 @idegtiarenko and I investigated and believe this needs reverting: silently dropping results from the query response in case any index is missing can lead to real problems if users don't spot their mistake. I'm also not sure if all the results will get dropped, or only from some nodes/shards/clusters, meaning that this might be hard to spot by users if only some results get dropped. The main PR has no transport version bump, no new ESQL capability, and was merged 15h ago - so it should be safe to just revert it. I noticed there was a linked Serverless PR on the original PR, but it merely disabled some obsolete tests on Serverless and doesn't require reverting itself.
1 parent a58d8e0 commit 8f38b13

File tree

19 files changed

+60
-100
lines changed

19 files changed

+60
-100
lines changed

‎docs/changelog/125060.yaml

-16
This file was deleted.

‎docs/changelog/126286.yaml

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
pr: 126286
2+
summary: Revert "Allow partial results by default in ES|QL"
3+
area: ES|QL
4+
type: bug
5+
issues:
6+
- 126275

‎docs/release-notes/breaking-changes.md

-5
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,6 @@ To learn how to upgrade, check out <uprade docs>.
1414

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

17-
## 9.1.0 [elasticsearch-910-breaking-changes]
18-
19-
ES|QL
20-
: * Allow partial results by default in ES|QL [#125060](https://github.com/elastic/elasticsearch/pull/125060)
21-
2217
## 9.0.0 [elasticsearch-900-breaking-changes]
2318

2419
Allocation

‎test/external-modules/esql-heap-attack/src/javaRestTest/java/org/elasticsearch/xpack/esql/heap_attack/Clusters.java

-1
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ static ElasticsearchCluster buildCluster() {
2121
.module("test-esql-heap-attack")
2222
.setting("xpack.security.enabled", "false")
2323
.setting("xpack.license.self_generated.type", "trial")
24-
.setting("esql.query.allow_partial_results", "false")
2524
.jvmArg("-Xmx512m");
2625
String javaVersion = JvmInfo.jvmInfo().version();
2726
if (javaVersion.equals("20") || javaVersion.equals("21")) {

‎x-pack/plugin/build.gradle

-1
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,6 @@ tasks.named("yamlRestCompatTestTransform").configure({ task ->
124124
task.skipTest("ml/post_data/Test POST data job api, flush, close and verify DataCounts doc", "Flush API is deprecated")
125125
task.replaceValueInMatch("Size", 49, "Test flamegraph from profiling-events")
126126
task.replaceValueInMatch("Size", 49, "Test flamegraph from test-events")
127-
task.skipTest("esql/63_enrich_int_range/Invalid age as double", "TODO: require disable allow_partial_results")
128127
})
129128

130129
tasks.named('yamlRestCompatTest').configure {

‎x-pack/plugin/esql/qa/security/src/javaRestTest/java/org/elasticsearch/xpack/esql/EsqlSecurityIT.java

+6-18
Original file line numberDiff line numberDiff line change
@@ -312,10 +312,7 @@ public void testIndexPatternErrorMessageComparison_ESQL_SearchDSL() throws Excep
312312
searchRequest.setOptions(RequestOptions.DEFAULT.toBuilder().addHeader("es-security-runas-user", "metadata1_read2"));
313313

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

321318
for (ResponseException r : List.of(esqlResp, srchResp)) {
@@ -334,8 +331,7 @@ public void testLimitedPrivilege() throws Exception {
334331
ResponseException.class,
335332
() -> runESQLCommand(
336333
"metadata1_read2",
337-
"FROM index-user1,index-user2 METADATA _index | STATS sum=sum(value), index=VALUES(_index)",
338-
false
334+
"FROM index-user1,index-user2 METADATA _index | STATS sum=sum(value), index=VALUES(_index)"
339335
)
340336
);
341337
assertThat(
@@ -348,7 +344,7 @@ public void testLimitedPrivilege() throws Exception {
348344

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

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

373369
resp = expectThrows(
374370
ResponseException.class,
375-
() -> runESQLCommand("alias_user1", "FROM first-alias,index-user1 METADATA _index | KEEP _index, org, value | LIMIT 10", false)
371+
() -> runESQLCommand("alias_user1", "FROM first-alias,index-user1 METADATA _index | KEEP _index, org, value | LIMIT 10")
376372
);
377373
assertThat(
378374
EntityUtils.toString(resp.getResponse().getEntity()),
@@ -386,8 +382,7 @@ public void testLimitedPrivilege() throws Exception {
386382
ResponseException.class,
387383
() -> runESQLCommand(
388384
"alias_user2",
389-
"from second-alias,index-user2 METADATA _index | stats sum=sum(value), index=VALUES(_index)",
390-
false
385+
"from second-alias,index-user2 METADATA _index | stats sum=sum(value), index=VALUES(_index)"
391386
)
392387
);
393388
assertThat(
@@ -831,10 +826,6 @@ public void testDataStream() throws IOException {
831826
}
832827

833828
protected Response runESQLCommand(String user, String command) throws IOException {
834-
return runESQLCommand(user, command, null);
835-
}
836-
837-
protected Response runESQLCommand(String user, String command, Boolean allowPartialResults) throws IOException {
838829
if (command.toLowerCase(Locale.ROOT).contains("limit") == false) {
839830
// add a (high) limit to avoid warnings on default limit
840831
command += " | limit 10000000";
@@ -848,9 +839,6 @@ protected Response runESQLCommand(String user, String command, Boolean allowPart
848839
request.setJsonEntity(Strings.toString(json));
849840
request.setOptions(RequestOptions.DEFAULT.toBuilder().addHeader("es-security-runas-user", user));
850841
request.addParameter("error_trace", "true");
851-
if (allowPartialResults != null) {
852-
request.addParameter("allow_partial_results", Boolean.toString(allowPartialResults));
853-
}
854842
return client().performRequest(request);
855843
}
856844

‎x-pack/plugin/esql/qa/server/multi-clusters/src/javaRestTest/java/org/elasticsearch/xpack/esql/ccq/EsqlRestValidationIT.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,6 @@ private RestClient remoteClusterClient() throws IOException {
8383

8484
@Before
8585
public void skipTestOnOldVersions() {
86-
assumeTrue("skip on old versions", Clusters.localClusterVersion().equals(Version.V_8_19_0));
86+
assumeTrue("skip on old versions", Clusters.localClusterVersion().equals(Version.V_8_16_0));
8787
}
8888
}

‎x-pack/plugin/esql/qa/server/multi-clusters/src/javaRestTest/java/org/elasticsearch/xpack/esql/ccq/RequestIndexFilteringIT.java

-7
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
import org.junit.rules.TestRule;
2929

3030
import java.io.IOException;
31-
import java.util.List;
3231
import java.util.Map;
3332

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

8988
@Override
9089
public Map<String, Object> runEsql(RestEsqlTestCase.RequestObjectBuilder requestObject) throws IOException {
91-
if (requestObject.allowPartialResults() != null) {
92-
assumeTrue(
93-
"require allow_partial_results on local cluster",
94-
clusterHasCapability("POST", "/_query", List.of(), List.of("support_partial_results")).orElse(false)
95-
);
96-
}
9790
requestObject.includeCCSMetadata(true);
9891
return super.runEsql(requestObject);
9992
}

‎x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/RestEsqlIT.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ public void testInvalidPragma() throws IOException {
111111
request.setJsonEntity("{\"f\":" + i + "}");
112112
assertOK(client().performRequest(request));
113113
}
114-
RequestObjectBuilder builder = requestObjectBuilder().query("from test-index | limit 1 | keep f").allowPartialResults(false);
114+
RequestObjectBuilder builder = requestObjectBuilder().query("from test-index | limit 1 | keep f");
115115
builder.pragmas(Settings.builder().put("data_partitioning", "invalid-option").build());
116116
ResponseException re = expectThrows(ResponseException.class, () -> runEsqlSync(builder));
117117
assertThat(EntityUtils.toString(re.getResponse().getEntity()), containsString("No enum constant"));

‎x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/EsqlRestValidationTestCase.java

-1
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,6 @@ private Request createRequest(String indexName) throws IOException {
129129
final var request = new Request("POST", "/_query");
130130
request.addParameter("error_trace", "true");
131131
request.addParameter("pretty", "true");
132-
request.addParameter("allow_partial_results", Boolean.toString(false));
133132
request.setJsonEntity(
134133
Strings.toString(JsonXContent.contentBuilder().startObject().field("query", "from " + indexName).endObject())
135134
);

‎x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RequestIndexFilteringTestCase.java

+4-13
Original file line numberDiff line numberDiff line change
@@ -198,26 +198,17 @@ public void testIndicesDontExist() throws IOException {
198198
int docsTest1 = 0; // we are interested only in the created index, not necessarily that it has data
199199
indexTimestampData(docsTest1, "test1", "2024-11-26", "id1");
200200

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

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

217-
e = expectThrows(
218-
ResponseException.class,
219-
() -> runEsql(timestampFilter("gte", "2020-01-01").query(from("foo", "test1")).allowPartialResults(false))
220-
);
211+
e = expectThrows(ResponseException.class, () -> runEsql(timestampFilter("gte", "2020-01-01").query(from("foo", "test1"))));
221212
assertEquals(404, e.getResponse().getStatusLine().getStatusCode());
222213
assertThat(e.getMessage(), containsString("index_not_found_exception"));
223214
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 {
226217
var pattern = from("test1");
227218
e = expectThrows(
228219
ResponseException.class,
229-
() -> runEsql(timestampFilter("gte", "2020-01-01").query(pattern + " | LOOKUP JOIN foo ON id1").allowPartialResults(false))
220+
() -> runEsql(timestampFilter("gte", "2020-01-01").query(pattern + " | LOOKUP JOIN foo ON id1"))
230221
);
231222
assertEquals(400, e.getResponse().getStatusLine().getStatusCode());
232223
assertThat(

‎x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java

+5-9
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ public static class RequestObjectBuilder {
132132
private Boolean includeCCSMetadata = null;
133133

134134
private CheckedConsumer<XContentBuilder, IOException> filter;
135-
private Boolean allowPartialResults = null;
135+
private Boolean allPartialResults = null;
136136

137137
public RequestObjectBuilder() throws IOException {
138138
this(randomFrom(XContentType.values()));
@@ -210,15 +210,11 @@ public RequestObjectBuilder filter(CheckedConsumer<XContentBuilder, IOException>
210210
return this;
211211
}
212212

213-
public RequestObjectBuilder allowPartialResults(boolean allowPartialResults) {
214-
this.allowPartialResults = allowPartialResults;
213+
public RequestObjectBuilder allPartialResults(boolean allPartialResults) {
214+
this.allPartialResults = allPartialResults;
215215
return this;
216216
}
217217

218-
public Boolean allowPartialResults() {
219-
return allowPartialResults;
220-
}
221-
222218
public RequestObjectBuilder build() throws IOException {
223219
if (isBuilt == false) {
224220
if (tables != null) {
@@ -1373,8 +1369,8 @@ protected static Request prepareRequestWithOptions(RequestObjectBuilder requestO
13731369
requestObject.build();
13741370
Request request = prepareRequest(mode);
13751371
String mediaType = attachBody(requestObject, request);
1376-
if (requestObject.allowPartialResults != null) {
1377-
request.addParameter("allow_partial_results", String.valueOf(requestObject.allowPartialResults));
1372+
if (requestObject.allPartialResults != null) {
1373+
request.addParameter("allow_partial_results", String.valueOf(requestObject.allPartialResults));
13781374
}
13791375

13801376
RequestOptions.Builder options = request.getOptions().toBuilder();

‎x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractCrossClusterTestCase.java

-6
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
import org.elasticsearch.transport.RemoteClusterAware;
2626
import org.elasticsearch.xcontent.XContentBuilder;
2727
import org.elasticsearch.xcontent.json.JsonXContent;
28-
import org.elasticsearch.xpack.esql.plugin.EsqlPlugin;
2928
import org.junit.After;
3029
import org.junit.Before;
3130

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

80-
@Override
81-
protected Settings nodeSettings() {
82-
return Settings.builder().put(super.nodeSettings()).put(EsqlPlugin.QUERY_ALLOW_PARTIAL_RESULTS.getKey(), false).build();
83-
}
84-
8579
public static class InternalExchangePlugin extends Plugin {
8680
@Override
8781
public List<Setting<?>> getSettings() {

‎x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractEsqlIntegTestCase.java

-8
Original file line numberDiff line numberDiff line change
@@ -139,14 +139,6 @@ protected Collection<Class<? extends Plugin>> nodePlugins() {
139139
return CollectionUtils.appendToCopy(super.nodePlugins(), EsqlPlugin.class);
140140
}
141141

142-
@Override
143-
protected Settings nodeSettings(int nodeOrdinal, Settings otherSettings) {
144-
return Settings.builder()
145-
.put(super.nodeSettings(nodeOrdinal, otherSettings))
146-
.put(EsqlPlugin.QUERY_ALLOW_PARTIAL_RESULTS.getKey(), false)
147-
.build();
148-
}
149-
150142
protected void setRequestCircuitBreakerLimit(ByteSizeValue limit) {
151143
if (limit != null) {
152144
assertAcked(

‎x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClusterCancellationIT.java

+36-7
Original file line numberDiff line numberDiff line change
@@ -14,20 +14,25 @@
1414
import org.elasticsearch.action.index.IndexRequest;
1515
import org.elasticsearch.action.support.PlainActionFuture;
1616
import org.elasticsearch.action.support.WriteRequest;
17-
import org.elasticsearch.common.settings.Settings;
17+
import org.elasticsearch.common.settings.Setting;
1818
import org.elasticsearch.common.transport.TransportAddress;
1919
import org.elasticsearch.compute.operator.DriverTaskRunner;
2020
import org.elasticsearch.compute.operator.exchange.ExchangeService;
2121
import org.elasticsearch.core.TimeValue;
22+
import org.elasticsearch.plugins.Plugin;
2223
import org.elasticsearch.tasks.TaskCancelledException;
2324
import org.elasticsearch.tasks.TaskInfo;
25+
import org.elasticsearch.test.AbstractMultiClustersTestCase;
2426
import org.elasticsearch.transport.TransportService;
2527
import org.elasticsearch.xcontent.XContentBuilder;
2628
import org.elasticsearch.xcontent.json.JsonXContent;
2729
import org.elasticsearch.xpack.esql.EsqlTestUtils;
2830
import org.elasticsearch.xpack.esql.plugin.ComputeService;
31+
import org.junit.After;
32+
import org.junit.Before;
2933

3034
import java.util.ArrayList;
35+
import java.util.Collection;
3136
import java.util.List;
3237
import java.util.concurrent.TimeUnit;
3338

@@ -39,7 +44,7 @@
3944
import static org.hamcrest.Matchers.hasSize;
4045
import static org.hamcrest.Matchers.instanceOf;
4146

42-
public class CrossClusterCancellationIT extends AbstractCrossClusterTestCase {
47+
public class CrossClusterCancellationIT extends AbstractMultiClustersTestCase {
4348
private static final String REMOTE_CLUSTER = "cluster-a";
4449

4550
@Override
@@ -48,11 +53,35 @@ protected List<String> remoteClusterAlias() {
4853
}
4954

5055
@Override
51-
protected Settings nodeSettings() {
52-
return Settings.builder()
53-
.put(super.nodeSettings())
54-
.put(ExchangeService.INACTIVE_SINKS_INTERVAL_SETTING, TimeValue.timeValueMillis(between(3000, 4000)))
55-
.build();
56+
protected Collection<Class<? extends Plugin>> nodePlugins(String clusterAlias) {
57+
List<Class<? extends Plugin>> plugins = new ArrayList<>(super.nodePlugins(clusterAlias));
58+
plugins.add(EsqlPluginWithEnterpriseOrTrialLicense.class);
59+
plugins.add(InternalExchangePlugin.class);
60+
plugins.add(SimplePauseFieldPlugin.class);
61+
return plugins;
62+
}
63+
64+
public static class InternalExchangePlugin extends Plugin {
65+
@Override
66+
public List<Setting<?>> getSettings() {
67+
return List.of(
68+
Setting.timeSetting(
69+
ExchangeService.INACTIVE_SINKS_INTERVAL_SETTING,
70+
TimeValue.timeValueMillis(between(3000, 4000)),
71+
Setting.Property.NodeScope
72+
)
73+
);
74+
}
75+
}
76+
77+
@Before
78+
public void resetPlugin() {
79+
SimplePauseFieldPlugin.resetPlugin();
80+
}
81+
82+
@After
83+
public void releasePlugin() {
84+
SimplePauseFieldPlugin.release();
5685
}
5786

5887
@Override

‎x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/ManyShardsIT.java

-1
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,6 @@ protected Collection<Class<? extends Plugin>> nodePlugins() {
7272
@Override
7373
protected Settings nodeSettings(int nodeOrdinal, Settings otherSettings) {
7474
return Settings.builder()
75-
.put(super.nodeSettings(nodeOrdinal, otherSettings))
7675
.put(ExchangeService.INACTIVE_SINKS_INTERVAL_SETTING, TimeValue.timeValueMillis(between(3000, 5000)))
7776
.build();
7877
}

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/EsqlPlugin.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ public class EsqlPlugin extends Plugin implements ActionPlugin {
105105

106106
public static final Setting<Boolean> QUERY_ALLOW_PARTIAL_RESULTS = Setting.boolSetting(
107107
"esql.query.allow_partial_results",
108-
true,
108+
false,
109109
Setting.Property.NodeScope,
110110
Setting.Property.Dynamic
111111
);

‎x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/CrossClusterEsqlRCS1MissingIndicesIT.java

-2
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@ public class CrossClusterEsqlRCS1MissingIndicesIT extends AbstractRemoteClusterS
4747
.apply(commonClusterConfig)
4848
.setting("remote_cluster.port", "0")
4949
.setting("xpack.ml.enabled", "false")
50-
.setting("esql.query.allow_partial_results", "false")
5150
.setting("xpack.security.remote_cluster_server.ssl.enabled", () -> String.valueOf(SSL_ENABLED_REF.get()))
5251
.setting("xpack.security.remote_cluster_server.ssl.key", "remote-cluster.key")
5352
.setting("xpack.security.remote_cluster_server.ssl.certificate", "remote-cluster.crt")
@@ -63,7 +62,6 @@ public class CrossClusterEsqlRCS1MissingIndicesIT extends AbstractRemoteClusterS
6362
.module("x-pack-enrich")
6463
.apply(commonClusterConfig)
6564
.setting("xpack.ml.enabled", "false")
66-
.setting("esql.query.allow_partial_results", "false")
6765
.setting("xpack.security.remote_cluster_client.ssl.enabled", () -> String.valueOf(SSL_ENABLED_REF.get()))
6866
.build();
6967
}

0 commit comments

Comments
 (0)