Resolve main indices before lookup #134115
Conversation
Lookup indices must be resolved for every remote cluster that is going to be queried. Today list of remotes is derived from the index expression. With CPS/flat expressions remotes should be derived from the resolved main indices. This change moves lookup index resolution after main index resolution so that it will be possible to use main index resolution when deriving lookup indices remotes.
idegtiarenko
added
>non-issue
Team:Analytics
|
Pinging @elastic/es-analytical-engine (Team:Analytics) |
There was a problem hiding this comment.
The comment first resolve the lookup indices, then the main indices seems to be very on purpose, but the CI is happy, and checking the code I don't see any dependencies between the two resolutions, so it LGTM.
Just for reference, that comment was introduced by #117246, but there is no further explanation in the PR
| } | ||
| listener.<PreAnalysisResult>andThen((l, result) -> preAnalyzeMainIndices(preAnalysis, executionInfo, result, requestFilter, l)) | ||
| .<LogicalPlan>andThen((l, result) -> analyzeWithRetry(parsed, requestFilter, preAnalysis, executionInfo, result, l)) | ||
| .<PreAnalysisResult>andThen((l, r) -> resolveInferences(parsed, r, l)) |
There was a problem hiding this comment.
Is there a reason why inferences stay on top of main indices? I see that enrich adds fields so it has to be on top, but inference doesn't do that?
There was a problem hiding this comment.
I do not see a strong reason for that. Inference resolution result is not used in main or lookup index FC.
I believe we can move it if needed.
There was a problem hiding this comment.
I think it makes sense to move it so if we ever need the cluster context for it (which we may) we'd have it. And on general principle so it's not confusing why some things happen before the main lookup and some after.
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java
Show resolved
Hide resolved
4 participants
Lookup indices must be resolved for every remote cluster that is going to be queried. Today list of remotes is derived from the index expression. With CPS/flat expressions remotes should be derived from the resolved main indices. This change moves lookup index resolution after main index resolution so that it will be possible to use main index resolution when deriving lookup indices remotes.
Closes: es-12613