Skip to content

Conversation

@ioanatia
Copy link
Contributor

@ioanatia ioanatia commented May 21, 2025

should close #127209

Because the FORK output is a union of the subplans' outputs and we keep the same Attribute objects, we don't always apply the right optimizations.

Take this query for example:

FROM search-movies METADATA _id
| FORK ( EVAL a = 1 | WHERE _id == "1" )
             ( EVAL a = 2 | WHERE _id == "2" )
| KEEP _id, _fork, a
| WHERE a == 1

When we apply constant folding, we incorrectly determine that the a attribute from the last WHERE condition is a constant. In cases like these we either end up not returning any rows or we return the output of both FORK branches.

To fix this, the FORK output will no longer reuse the same Attribute objects from the fork subplans when we do the union of output attributes.
Instead FORK will output a new set of attributes that have the same name and type but with different IDs (in a way similar to STATS).

@ioanatia ioanatia changed the title ESFix constant folding when using FORK ES|QL: Fix constant folding when using FORK May 21, 2025
@ioanatia ioanatia added >non-issue :Analytics/ES|QL AKA ESQL :Search Relevance/Search Catch all for Search Relevance labels May 21, 2025
@ioanatia ioanatia marked this pull request as ready for review May 22, 2025 13:36
@ioanatia ioanatia requested review from ChrisHegarty and bpintea May 22, 2025 13:36
@elasticsearchmachine elasticsearchmachine added Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch labels May 22, 2025
@elasticsearchmachine
Copy link
Collaborator

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

@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-search-relevance (Team:Search Relevance)

@ioanatia ioanatia requested review from carlosdelest and tteofili May 22, 2025 13:37
Copy link
Member

@carlosdelest carlosdelest left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link
Contributor

@ChrisHegarty ChrisHegarty left a comment

Choose a reason for hiding this comment

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

LGTM

@ioanatia ioanatia merged commit 2bf6d54 into elastic:main May 22, 2025
18 checks passed
@ioanatia ioanatia deleted the fork_output branch May 22, 2025 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Analytics/ES|QL AKA ESQL >non-issue :Search Relevance/Search Catch all for Search Relevance Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch v9.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ES|QL: Fix column references for FORK

4 participants