Skip to content

Commit f84b339

Browse files
idegtiarenkoidegtiarenko
authored
Cleanup esql request building api (#138398) (#138494)
(cherry picked from commit daab115)
1 parent 0b95ab2 commit f84b339

39 files changed

+182
-427
lines changed

‎x-pack/plugin/core/src/main/java/module-info.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,6 @@
6969
exports org.elasticsearch.xpack.core.eql;
7070
exports org.elasticsearch.xpack.core.esql;
7171
exports org.elasticsearch.xpack.core.esql.action;
72-
exports org.elasticsearch.xpack.core.esql.action.internal; // TODO: qualify to esql when modularized
7372
exports org.elasticsearch.xpack.core.frozen;
7473
exports org.elasticsearch.xpack.core.graph.action;
7574
exports org.elasticsearch.xpack.core.graph;

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/esql/action/EsqlQueryRequestBuilder.java‎

Lines changed: 0 additions & 44 deletions
This file was deleted.

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/esql/action/internal/SharedSecrets.java‎

Lines changed: 0 additions & 41 deletions
This file was deleted.

‎x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/esql/action/EsqlQueryRequestBuilderTests.java‎

Lines changed: 0 additions & 22 deletions
This file was deleted.

‎x-pack/plugin/esql/qa/action/src/internalClusterTest/java/org/elasticsearch/test/esql/qa/action/CoreEsqlActionIT.java‎

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,11 @@
1515
import org.elasticsearch.test.ESIntegTestCase;
1616
import org.elasticsearch.xpack.core.ClientHelper;
1717
import org.elasticsearch.xpack.core.esql.action.ColumnInfo;
18-
import org.elasticsearch.xpack.core.esql.action.EsqlQueryRequest;
19-
import org.elasticsearch.xpack.core.esql.action.EsqlQueryRequestBuilder;
2018
import org.elasticsearch.xpack.core.esql.action.EsqlQueryResponse;
2119
import org.elasticsearch.xpack.core.esql.action.EsqlResponse;
2220
import org.elasticsearch.xpack.esql.action.ColumnInfoImpl;
21+
import org.elasticsearch.xpack.esql.action.EsqlQueryAction;
22+
import org.elasticsearch.xpack.esql.action.EsqlQueryRequest;
2323
import org.elasticsearch.xpack.esql.core.type.DataType;
2424
import org.junit.Before;
2525

@@ -30,6 +30,7 @@
3030

3131
import static java.util.concurrent.TimeUnit.SECONDS;
3232
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
33+
import static org.elasticsearch.xpack.esql.action.EsqlQueryRequest.syncEsqlQueryRequest;
3334
import static org.hamcrest.Matchers.contains;
3435

3536
// A subset of test scenarios exercised through the xpack core ES|QL
@@ -54,8 +55,7 @@ protected Collection<Class<? extends Plugin>> nodePlugins() {
5455

5556
public void testRowTypesAndValues() {
5657
var query = "row a = 1, b = \"x\", c = 1000000000000, d = 1.1";
57-
var request = EsqlQueryRequestBuilder.newRequestBuilder(client()).query(query);
58-
try (EsqlQueryResponse queryResp = run(request)) {
58+
try (EsqlQueryResponse queryResp = run(syncEsqlQueryRequest(query))) {
5959
logger.info("response=" + queryResp);
6060
EsqlResponse resp = queryResp.response();
6161
assertThat(resp.columns().stream().map(ColumnInfo::name).toList(), contains("a", "b", "c", "d"));
@@ -69,8 +69,7 @@ public void testRowTypesAndValues() {
6969

7070
public void testRowStatsProjectGroupByInt() {
7171
var query = "row a = 1, b = 2 | stats count(b) by a | keep a";
72-
var request = EsqlQueryRequestBuilder.newRequestBuilder(client()).query(query);
73-
try (var queryResp = run(request)) {
72+
try (var queryResp = run(syncEsqlQueryRequest(query))) {
7473
logger.info("response=" + queryResp);
7574
var resp = queryResp.response();
7675
assertThat(resp.columns().stream().map(ColumnInfo::name).toList(), contains("a"));
@@ -81,8 +80,7 @@ public void testRowStatsProjectGroupByInt() {
8180

8281
public void testFrom() {
8382
var query = "from test | keep item, cost, color, sale | sort item";
84-
var request = EsqlQueryRequestBuilder.newRequestBuilder(client()).query(query);
85-
try (var queryResp = run(request)) {
83+
try (var queryResp = run(syncEsqlQueryRequest(query))) {
8684
var resp = queryResp.response();
8785
logger.info("response=" + queryResp);
8886
assertThat(resp.columns().stream().map(ColumnInfo::name).toList(), contains("item", "cost", "color", "sale"));
@@ -108,8 +106,7 @@ public void testFrom() {
108106
public void testAccessAfterClose() {
109107
for (var closedQueryResp : new boolean[] { true, false }) {
110108
var query = "row a = 1";
111-
var request = EsqlQueryRequestBuilder.newRequestBuilder(client()).query(query);
112-
var queryResp = run(request);
109+
var queryResp = run(syncEsqlQueryRequest(query));
113110
var resp = queryResp.response();
114111
var rows = resp.rows();
115112
var rowItr = rows.iterator();
@@ -136,19 +133,17 @@ public void testAccessAfterClose() {
136133
}
137134
}
138135

139-
protected EsqlQueryResponse run(EsqlQueryRequestBuilder<? extends EsqlQueryRequest, ? extends EsqlQueryResponse> request) {
136+
protected EsqlQueryResponse run(EsqlQueryRequest request) {
140137
try {
141138
// The variants here ensure API usage patterns
142139
if (randomBoolean()) {
143-
return request.execute().actionGet(30, SECONDS);
144-
} else if (randomBoolean()) {
145-
return client().execute(request.action(), request.request()).actionGet(30, SECONDS);
140+
return client().execute(EsqlQueryAction.INSTANCE, request).actionGet(30, SECONDS);
146141
} else {
147142
return ClientHelper.executeWithHeaders(
148143
Map.of("Foo", "bar"),
149144
"origin",
150145
client(),
151-
() -> request.execute().actionGet(30, SECONDS)
146+
() -> client().execute(EsqlQueryAction.INSTANCE, request).actionGet(30, SECONDS)
152147
);
153148
}
154149
} catch (ElasticsearchTimeoutException e) {

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -344,8 +344,7 @@ protected EsqlQueryResponse runQuery(EsqlQueryRequest request) {
344344
}
345345

346346
protected EsqlQueryResponse runQuery(String query, Boolean ccsMetadataInResponse) {
347-
EsqlQueryRequest request = EsqlQueryRequest.syncEsqlQueryRequest();
348-
request.query(query);
347+
EsqlQueryRequest request = EsqlQueryRequest.syncEsqlQueryRequest(query);
349348
request.pragmas(AbstractEsqlIntegTestCase.randomPragmas());
350349
request.profile(randomInt(5) == 2);
351350
request.columnar(randomBoolean());

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

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,7 @@ public void setupQueryNode() {
5151
}
5252

5353
protected CCSTelemetrySnapshot getTelemetryFromQuery(String query, String client) throws ExecutionException, InterruptedException {
54-
EsqlQueryRequest request = EsqlQueryRequest.syncEsqlQueryRequest();
55-
request.query(query);
54+
EsqlQueryRequest request = EsqlQueryRequest.syncEsqlQueryRequest(query);
5655
request.pragmas(AbstractEsqlIntegTestCase.randomPragmas());
5756
request.columnar(randomBoolean());
5857
request.includeCCSMetadata(randomBoolean());
@@ -78,8 +77,7 @@ protected CCSTelemetrySnapshot getTelemetryFromQuery(EsqlQueryRequest request, S
7877
}
7978

8079
protected CCSTelemetrySnapshot getTelemetryFromAsyncQuery(String query) throws Exception {
81-
EsqlQueryRequest request = EsqlQueryRequest.asyncEsqlQueryRequest();
82-
request.query(query);
80+
EsqlQueryRequest request = EsqlQueryRequest.asyncEsqlQueryRequest(query);
8381
request.pragmas(AbstractEsqlIntegTestCase.randomPragmas());
8482
request.columnar(randomBoolean());
8583
request.includeCCSMetadata(randomBoolean());
@@ -111,9 +109,8 @@ protected CCSTelemetrySnapshot getTelemetryFromAsyncQuery(EsqlQueryRequest reque
111109
return getTelemetrySnapshot(queryNode);
112110
}
113111

114-
protected CCSTelemetrySnapshot getTelemetryFromFailedQuery(String query) throws Exception {
115-
EsqlQueryRequest request = EsqlQueryRequest.syncEsqlQueryRequest();
116-
request.query(query);
112+
protected CCSTelemetrySnapshot getTelemetryFromFailedQuery(String query) {
113+
EsqlQueryRequest request = EsqlQueryRequest.syncEsqlQueryRequest(query);
117114
request.pragmas(AbstractEsqlIntegTestCase.randomPragmas());
118115
request.columnar(randomBoolean());
119116
request.includeCCSMetadata(randomBoolean());

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -243,8 +243,7 @@ static String enrichVendors(Enrich.Mode mode) {
243243
}
244244

245245
protected EsqlQueryResponse runQuery(String query, Boolean ccsMetadataInResponse) {
246-
EsqlQueryRequest request = EsqlQueryRequest.syncEsqlQueryRequest();
247-
request.query(query);
246+
EsqlQueryRequest request = EsqlQueryRequest.syncEsqlQueryRequest(query);
248247
request.pragmas(AbstractEsqlIntegTestCase.randomPragmas());
249248
if (randomBoolean()) {
250249
request.profile(true);

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ protected void setRequestCircuitBreakerLimit(ByteSizeValue limit) {
166166
}
167167

168168
protected final EsqlQueryResponse run(String esqlCommands) {
169-
return run(syncEsqlQueryRequest().query(esqlCommands).pragmas(randomPragmas()));
169+
return run(syncEsqlQueryRequest(esqlCommands).pragmas(randomPragmas()));
170170
}
171171

172172
public EsqlQueryResponse run(EsqlQueryRequest request) {

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

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
import static org.elasticsearch.test.hamcrest.OptionalMatchers.isEmpty;
4444
import static org.elasticsearch.test.hamcrest.OptionalMatchers.isPresent;
4545
import static org.elasticsearch.xpack.esql.EsqlTestUtils.getValuesList;
46+
import static org.elasticsearch.xpack.esql.action.EsqlQueryRequest.asyncEsqlQueryRequest;
4647
import static org.hamcrest.Matchers.empty;
4748
import static org.hamcrest.Matchers.equalTo;
4849
import static org.hamcrest.Matchers.greaterThanOrEqualTo;
@@ -234,14 +235,12 @@ private void testFinishingBeforeTimeout(boolean keepOnCompletion) {
234235

235236
scriptPermits.release(numberOfDocs());
236237

237-
var request = EsqlQueryRequestBuilder.newAsyncEsqlQueryRequestBuilder(client())
238-
.query("from test | stats sum(pause_me)")
239-
.pragmas(queryPragmas())
238+
var request = asyncEsqlQueryRequest("from test | stats sum(pause_me)").pragmas(queryPragmas())
240239
.waitForCompletionTimeout(TimeValue.timeValueSeconds(60))
241240
.keepOnCompletion(keepOnCompletion)
242241
.keepAlive(randomKeepAlive());
243242

244-
try (var response = request.execute().actionGet(60, TimeUnit.SECONDS)) {
243+
try (var response = client().execute(EsqlQueryAction.INSTANCE, request).actionGet(60, TimeUnit.SECONDS)) {
245244
assertThat(response.isRunning(), is(false));
246245
assertThat(response.columns(), equalTo(List.of(new ColumnInfoImpl("sum(pause_me)", "long", null))));
247246
assertThat(getValuesList(response).size(), equalTo(1));
@@ -270,16 +269,14 @@ private void testFinishingBeforeTimeout(boolean keepOnCompletion) {
270269
public void testUpdateKeepAlive() throws Exception {
271270
long nowInMillis = System.currentTimeMillis();
272271
TimeValue keepAlive = timeValueSeconds(between(30, 60));
273-
var request = EsqlQueryRequestBuilder.newAsyncEsqlQueryRequestBuilder(client())
274-
.query("from test | stats sum(pause_me)")
275-
.pragmas(queryPragmas())
272+
var request = asyncEsqlQueryRequest("from test | stats sum(pause_me)").pragmas(queryPragmas())
276273
.waitForCompletionTimeout(TimeValue.timeValueMillis(between(1, 10)))
277274
.keepOnCompletion(randomBoolean())
278275
.keepAlive(keepAlive);
279276
final String asyncId;
280277
long currentExpiration;
281278
try {
282-
try (EsqlQueryResponse initialResponse = request.execute().actionGet(60, TimeUnit.SECONDS)) {
279+
try (EsqlQueryResponse initialResponse = client().execute(EsqlQueryAction.INSTANCE, request).actionGet(60, TimeUnit.SECONDS)) {
283280
assertThat(initialResponse.isRunning(), is(true));
284281
assertTrue(initialResponse.asyncExecutionId().isPresent());
285282
asyncId = initialResponse.asyncExecutionId().get();
@@ -373,15 +370,14 @@ private EsqlQueryResponse sendAsyncQuery() {
373370

374371
scriptPermits.release(between(1, 5));
375372
var pragmas = queryPragmas();
376-
return EsqlQueryRequestBuilder.newAsyncEsqlQueryRequestBuilder(client())
377-
.query("from test | stats sum(pause_me)")
378-
.pragmas(pragmas)
379-
// deliberately small timeout, to frequently trigger incomplete response
380-
.waitForCompletionTimeout(TimeValue.timeValueNanos(randomIntBetween(1, 20)))
381-
.keepOnCompletion(randomBoolean())
382-
.keepAlive(randomKeepAlive())
383-
.execute()
384-
.actionGet(60, TimeUnit.SECONDS);
373+
return client().execute(
374+
EsqlQueryAction.INSTANCE,
375+
asyncEsqlQueryRequest("from test | stats sum(pause_me)").pragmas(pragmas)
376+
// deliberately small timeout, to frequently trigger incomplete response
377+
.waitForCompletionTimeout(TimeValue.timeValueNanos(randomIntBetween(1, 20)))
378+
.keepOnCompletion(randomBoolean())
379+
.keepAlive(randomKeepAlive())
380+
).actionGet(60, TimeUnit.SECONDS);
385381
}
386382

387383
private QueryPragmas queryPragmas() {

0 commit comments

Comments
 (0)