ESQL: Revert "Allow partial results by default in ES|QL (#125060)"#126286
Merged
elasticsearchmachine merged 3 commits intoelastic:mainfrom Apr 4, 2025
Merged
Conversation
This reverts commit 81555cc.
Collaborator
|
Pinging @elastic/es-analytical-engine (Team:Analytics) |
elasticsearchmachine
added
the
Team:Analytics
Collaborator
|
Hi @alex-spies, I've created a changelog YAML for you. |
idegtiarenko
approved these changes
Apr 4, 2025
alex-spies
added
the
auto-merge-without-approval
Contributor
Author
|
I got a bwc test failure on the yaml tests in Serverless - I guess because in bwc scenarios, the default setting is initially to allow partial results, but the test specifically expects to fail. The original PR made the test stricter to force strict handling of partial failures here; to make the test pass, I specifically won't revert this part, which is fine as it only makes the test more specific. |
Reverting it results in bwc test failures on Serverless. That's probably because in bwc scenarios, the default setting is initially to allow partial results, but the test specifically expects failure. The original PR made the test stricter to force strict handling of partial failures here; to make the test pass, I specifically won't revert this part, which is fine as it only makes the test more specific.
andreidan
pushed a commit
to andreidan/elasticsearch
that referenced
this pull request
Apr 9, 2025
…)" (elastic#126286) This reverts commit 81555cc from elastic#125060. Fix elastic#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.
smalyshev
added a commit
to smalyshev/elasticsearch
that referenced
this pull request
Apr 24, 2025
…tic#125060)" (elastic#126286)" This reverts commit 8f38b13. Restore changes from elastic#125060 now that the breakage should be fixed.
smalyshev
added a commit
that referenced
this pull request
Apr 28, 2025
smalyshev
added a commit
to smalyshev/elasticsearch
that referenced
this pull request
Apr 28, 2025
* Revert "ESQL: Revert "Allow partial results by default in ES|QL (elastic#125060)" (elastic#126286)" This reverts commit 8f38b13. Restore changes from elastic#125060 now that the breakage should be fixed. (cherry picked from commit eb479e5) # Conflicts: # docs/release-notes/breaking-changes.md # x-pack/plugin/build.gradle # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/ClusterComputeHandler.java
elasticsearchmachine
pushed a commit
that referenced
this pull request
Apr 28, 2025
…127474) * Allow partial results by default in ES|QL - Take 2 (#127351) * Revert "ESQL: Revert "Allow partial results by default in ES|QL (#125060)" (#126286)" This reverts commit 8f38b13. Restore changes from #125060 now that the breakage should be fixed. (cherry picked from commit eb479e5) # Conflicts: # docs/release-notes/breaking-changes.md # x-pack/plugin/build.gradle # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/ClusterComputeHandler.java * skip test * fix test * fix test
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.