Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ESQL: Revert "Allow partial results by default in ES|QL (#125060)" #126286

Conversation

alex-spies
Copy link
Contributor

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.

@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-analytical-engine (Team:Analytics)

@elasticsearchmachine elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Apr 4, 2025
@elasticsearchmachine
Copy link
Collaborator

Hi @alex-spies, I've created a changelog YAML for you.

@alex-spies alex-spies added the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Apr 4, 2025
@alex-spies
Copy link
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/ES|QL AKA ESQL auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) >bug Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v9.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ESQL: silently empty result in case of missing index instead of ValidationException
3 participants