Flat index resolution #138557
Flat index resolution #138557
Conversation
idegtiarenko
added
>non-issue
Team:Analytics
|
Pinging @elastic/es-analytical-engine (Team:Analytics) |
luigidellaquila
left a comment
luigidellaquila
left a comment
There was a problem hiding this comment.
Thanks @idegtiarenko, LGTM
| result.fieldNames, | ||
| // Maybe if no indices are returned, retry without index mode and provide a clearer error message. | ||
| switch (indexMode) { | ||
| case IndexMode.TIME_SERIES -> { |
There was a problem hiding this comment.
nit: I see this logic and below is the same as for preAnalyzeMainIndices, maybe we can extract it in a method.
| ) | ||
| .build(); | ||
|
|
||
| private static final IndicesOptions FLAT_OPTIONS = IndicesOptions.builder(DEFAULT_OPTIONS) |
There was a problem hiding this comment.
Nit: can we put some comments somewhere about what "flat" means? Just a block of comment text someplace that explains what the difference between these two?
| public static final String UNMAPPED = "unmapped"; | ||
|
|
||
| public static final IndicesOptions FIELD_CAPS_INDICES_OPTIONS = IndicesOptions.builder() | ||
| public static final IndicesOptions DEFAULT_OPTIONS = IndicesOptions.builder() |
There was a problem hiding this comment.
I understand that you're trying to do here and I'm sure it works, but reads very visibly bi-modal. This is not necessarily for this PR, but maybe more for a follow up – can we split these two modalities through simple inheritance? I mean having the abstract base (IndexResolveBase) with protected final helper methods like createFieldCapsRequest, doResolveIndices and the abstract method resolveIndicesVersioned and have two classes deriving from it - DefaultIndexResolver and FlatWorldIndexResolver?
It will make for a better encapsulation and improved ability
There was a problem hiding this comment.
I will try this idea, however I am afraid this will shit complexity into service injection part.
While resolution remains somewhat similar, the usage will still be different around how EsqlExecutionInfo#clusterInfo is populated (eg before FC call in regular mode and after FC in CPS).
| */ | ||
| public void resolveIndicesVersioned( | ||
| String indexWildcard, | ||
| public void resolveFlatIndicesVersioned( |
There was a problem hiding this comment.
Nit: can we use "FlatWorld" as opposed to "Flat" please?
4 participants
Flat index resolution for an esql query. This includes: