Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
9313000
2 changes: 1 addition & 1 deletion server/src/main/resources/transport/upper_bounds/9.4.csv
Original file line number Diff line number Diff line change
@@ -1 +1 @@
transform_indices_options,9312000
sql_optional_allow_partial_search_results,9313000
1 change: 1 addition & 0 deletions x-pack/plugin/sql/sql-action/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* This project contains transport-level requests and responses that are shared between x-pack plugin and qa tests
*/
apply plugin: 'elasticsearch.build'
apply plugin: 'elasticsearch.transport-version-references'

description = 'Request and response objects shared by the Elasticsearch plugin and qa tests'

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/
package org.elasticsearch.xpack.sql.action;

import org.elasticsearch.TransportVersion;
import org.elasticsearch.action.ActionRequestValidationException;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.io.stream.StreamInput;
Expand Down Expand Up @@ -47,6 +48,10 @@
* Request to perform an sql query
*/
public class SqlQueryRequest extends AbstractSqlQueryRequest {
public static final TransportVersion OPTIONAL_ALLOW_PARTIAL_SEARCH_RESULTS = TransportVersion.fromName(
"sql_optional_allow_partial_search_results"
);

private static final ObjectParser<SqlQueryRequest, Void> PARSER = objectParser(SqlQueryRequest::new);
static final ParseField COLUMNAR = new ParseField(COLUMNAR_NAME);
static final ParseField FIELD_MULTI_VALUE_LENIENCY = new ParseField(FIELD_MULTI_VALUE_LENIENCY_NAME);
Expand Down Expand Up @@ -106,7 +111,7 @@ public class SqlQueryRequest extends AbstractSqlQueryRequest {
private boolean keepOnCompletion = DEFAULT_KEEP_ON_COMPLETION;
private TimeValue keepAlive = DEFAULT_KEEP_ALIVE;

private boolean allowPartialSearchResults = Protocol.ALLOW_PARTIAL_SEARCH_RESULTS;
private Boolean allowPartialSearchResults;

public SqlQueryRequest() {
super();
Expand All @@ -130,7 +135,7 @@ public SqlQueryRequest(
TimeValue waitForCompletionTimeout,
boolean keepOnCompletion,
TimeValue keepAlive,
boolean allowPartialSearchResults
Boolean allowPartialSearchResults
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It seems the request is only initialized here:

Unless I miss something it is actually always supplying it with non-null allowPartialSearchResults

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think you're right, that's overprotective. Let me simplify it (we can also avoid the transport version bump)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

) {
super(query, params, filter, runtimeMappings, zoneId, catalog, fetchSize, requestTimeout, pageTimeout, requestInfo);
this.cursor = cursor;
Expand Down Expand Up @@ -162,7 +167,11 @@ public SqlQueryRequest(StreamInput in) throws IOException {
this.waitForCompletionTimeout = in.readOptionalTimeValue();
this.keepOnCompletion = in.readBoolean();
this.keepAlive = in.readOptionalTimeValue();
allowPartialSearchResults = in.readBoolean();
if (in.getTransportVersion().supports(OPTIONAL_ALLOW_PARTIAL_SEARCH_RESULTS)) {
allowPartialSearchResults = in.readOptionalBoolean();
} else {
allowPartialSearchResults = in.readBoolean();
}
}

/**
Expand Down Expand Up @@ -256,12 +265,12 @@ public TimeValue keepAlive() {
return keepAlive;
}

public SqlQueryRequest allowPartialSearchResults(boolean allowPartialSearchResults) {
public SqlQueryRequest allowPartialSearchResults(Boolean allowPartialSearchResults) {
this.allowPartialSearchResults = allowPartialSearchResults;
return this;
}

public boolean allowPartialSearchResults() {
public Boolean allowPartialSearchResults() {
return allowPartialSearchResults;
}

Expand Down Expand Up @@ -294,7 +303,11 @@ public void writeTo(StreamOutput out) throws IOException {
out.writeOptionalTimeValue(waitForCompletionTimeout);
out.writeBoolean(keepOnCompletion);
out.writeOptionalTimeValue(keepAlive);
out.writeBoolean(allowPartialSearchResults);
if (out.getTransportVersion().supports(OPTIONAL_ALLOW_PARTIAL_SEARCH_RESULTS)) {
out.writeOptionalBoolean(allowPartialSearchResults);
} else {
out.writeBoolean(allowPartialSearchResults != null && allowPartialSearchResults);
}
}

@Override
Expand All @@ -320,7 +333,7 @@ public boolean equals(Object obj) {
&& indexIncludeFrozen == ((SqlQueryRequest) obj).indexIncludeFrozen
&& Objects.equals(binaryCommunication, ((SqlQueryRequest) obj).binaryCommunication)
&& keepOnCompletion == ((SqlQueryRequest) obj).keepOnCompletion
&& allowPartialSearchResults == ((SqlQueryRequest) obj).allowPartialSearchResults
&& Objects.equals(allowPartialSearchResults, ((SqlQueryRequest) obj).allowPartialSearchResults)
&& Objects.equals(cursor, ((SqlQueryRequest) obj).cursor)
&& Objects.equals(columnar, ((SqlQueryRequest) obj).columnar)
&& Objects.equals(waitForCompletionTimeout, ((SqlQueryRequest) obj).waitForCompletionTimeout)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ protected TestSqlQueryRequest createTestInstance() {
randomTimeValue(),
randomBoolean(),
randomTVGreaterThan(MIN_KEEP_ALIVE),
randomBoolean()
randomBoolean() ? true : null
);
}

Expand Down Expand Up @@ -119,7 +119,7 @@ protected TestSqlQueryRequest mutateInstance(TestSqlQueryRequest instance) {
request -> request.keepOnCompletion(randomValueOtherThan(request.keepOnCompletion(), ESTestCase::randomBoolean)),
request -> request.keepAlive(randomValueOtherThan(request.keepAlive(), () -> randomTVGreaterThan(MIN_KEEP_ALIVE))),
request -> request.allowPartialSearchResults(
randomValueOtherThan(request.allowPartialSearchResults(), ESTestCase::randomBoolean)
randomValueOtherThan(request.allowPartialSearchResults(), () -> randomBoolean() ? true : null)
)
);
TestSqlQueryRequest newRequest = new TestSqlQueryRequest(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public TestSqlQueryRequest(
TimeValue waitForCompletionTimeout,
boolean keepOnCompletion,
TimeValue keepAlive,
boolean allowPartialSearchResults
Boolean allowPartialSearchResults
) {
super(
query,
Expand Down Expand Up @@ -87,7 +87,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
ProtoShim.toProto(this.waitForCompletionTimeout()),
this.keepOnCompletion(),
ProtoShim.toProto(this.keepAlive()),
this.allowPartialSearchResults(),
this.allowPartialSearchResults() != null && this.allowPartialSearchResults(),
null
);
return SqlTestUtils.toXContentBuilder(builder, this, protoInstance);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,8 @@ private void searchWithPointInTime(SearchRequest search, SqlConfiguration cfg, A
options = IndicesOptions.builder(options).crossProjectModeOptions(new IndicesOptions.CrossProjectModeOptions(true)).build();
}
final OpenPointInTimeRequest openPitRequest = new OpenPointInTimeRequest(search.indices()).indicesOptions(options)
.keepAlive(cfg.pageTimeout());
.keepAlive(cfg.pageTimeout())
.allowPartialSearchResults(cfg.allowPartialSearchResults());
if (cfg.crossProject() && cfg.projectRouting() != null) {
openPitRequest.projectRouting(cfg.projectRouting());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,10 @@ public static void operation(
) {
// The configuration is always created however when dealing with the next page, only the timeouts are relevant
// the rest having default values (since the query is already created)
boolean crossProjectEnabled = new CrossProjectModeDecider(clusterService.getSettings()).crossProjectEnabled();
boolean allowPartialSearchResults = request.allowPartialSearchResults() != null
? request.allowPartialSearchResults()
: crossProjectEnabled;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I believe the main change is here: if allowPartialSearchResults is null, we fallback to CPS default.

SqlConfiguration cfg = new SqlConfiguration(
request.zoneId(),
request.catalog(),
Expand All @@ -172,8 +176,8 @@ public static void operation(
request.indexIncludeFrozen(),
new TaskId(clusterService.localNode().getId(), task.getId()),
task,
request.allowPartialSearchResults(),
new CrossProjectModeDecider(clusterService.getSettings()).crossProjectEnabled(),
allowPartialSearchResults,
crossProjectEnabled,
request.projectRouting()
);

Expand Down
Loading