Skip to content

Conversation

@idegtiarenko
Copy link
Contributor

This change attempts to resolve lookup indices concurrently.

Opening as a draft to discuss feasibility and practicality of such approach.
In theory this could decrease analysis time for a queries using multiple lookup joins.

This change attempts to resolve lookup indices concurrently.
@idegtiarenko idegtiarenko added >non-issue Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) :Analytics/ES|QL AKA ESQL v9.2.0 labels Jul 31, 2025
Copy link
Contributor

@alex-spies alex-spies left a comment

Choose a reason for hiding this comment

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

I think the change makes sense, but I'm not familiar enough with our listener pipeline here to check for unintended consequences. I see you also seek review from @astefan , which is a good idea IMO as he's more familiar here.

}
)
// first resolve the lookup indices, then the main indices
.<PreAnalysisResult>andThen((l, preAnalysisResult) -> {
Copy link
Contributor

Choose a reason for hiding this comment

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

A comment wouldn't hurt to make more clear that this runs for all lookup indices in parallel.

Copy link
Contributor

@astefan astefan left a comment

Choose a reason for hiding this comment

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

I am no expert in listeners either, but by looking at the RefCountingListener javadoc and how we use it and for what exactly, it does make sense to me.
LGTM

@idegtiarenko
Copy link
Contributor Author

Outdated

@idegtiarenko idegtiarenko deleted the concurrent_lookup_index_resolution branch September 1, 2025 11:31
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 Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v9.2.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants