SQL: better management of allowPartialSearchResults defaults for CPS#143995
SQL: better management of allowPartialSearchResults defaults for CPS#143995luigidellaquila merged 6 commits intoelastic:mainfrom
Conversation
elasticsearchmachine
added
the
Team:Analytics
|
Pinging @elastic/es-analytical-engine (Team:Analytics) |
| boolean crossProjectEnabled = new CrossProjectModeDecider(clusterService.getSettings()).crossProjectEnabled(); | ||
| boolean allowPartialSearchResults = request.allowPartialSearchResults() != null | ||
| ? request.allowPartialSearchResults() | ||
| : crossProjectEnabled; |
There was a problem hiding this comment.
I believe the main change is here: if allowPartialSearchResults is null, we fallback to CPS default.
| boolean keepOnCompletion, | ||
| TimeValue keepAlive, | ||
| boolean allowPartialSearchResults | ||
| Boolean allowPartialSearchResults |
There was a problem hiding this comment.
It seems the request is only initialized here:
Unless I miss something it is actually always supplying it with non-null allowPartialSearchResults
There was a problem hiding this comment.
I think you're right, that's overprotective. Let me simplify it (we can also avoid the transport version bump)
There was a problem hiding this comment.
No, sorry, my bad, the implementation is correct.
XContent parser uses the other constructor https://github.com/elastic/elasticsearch/blob/main/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/SqlQueryRequest.java#L50
richard-dennehy
left a comment
richard-dennehy
left a comment
There was a problem hiding this comment.
I've confirmed this fixes the failing SQL test I wrote
… into sql-cps-error-handling
4 participants
Fix the way SQL manages
allowPartialSearchResultsdefaults in CPS, unifying the logic with what EQL and ES|QL do.