Simplify index pattern #134215
Simplify index pattern #134215
Conversation
idegtiarenko
added
>non-issue
Team:Analytics
|
Pinging @elastic/es-analytical-engine (Team:Analytics) |
| ThreadPool.Names.SEARCH_COORDINATION, | ||
| ThreadPool.Names.SYSTEM_READ | ||
| ); | ||
| // TODO we plan to support joins in the future when possible, but for now we'll just fail early if we see one |
There was a problem hiding this comment.
We already support lookup joins and they require a separate data structure.
Once we support nested queries they would likely need their own structure (to distinguish top level vs nested query).
Today we are guarded by grammar and parser to never have more than a single item in the list so indices.size() > 1 is effectively a dead code.
| listener.onFailure(ex); | ||
| } | ||
| // occurs when dealing with local relations (row a = 1) | ||
| listener.onResponse(result.withIndexResolution(IndexResolution.invalid("[none specified]"))); |
There was a problem hiding this comment.
Nothing could be thrown here.
| plan.forEachUp(LogicalPlan::setPreAnalyzed); | ||
|
|
||
| return new PreAnalysis(indexMode.get(), indices.stream().toList(), unresolvedEnriches, lookupIndices); | ||
| return new PreAnalysis(indexMode.get(), index.get(), unresolvedEnriches, lookupIndices); |
There was a problem hiding this comment.
It looks like indices were copied to immutable list here. I wonder if it is worth doing the same for unresolvedEnriches and lookupIndices?
| plan.forEachUp(UnresolvedRelation.class, p -> { | ||
| if (p.indexMode() == IndexMode.LOOKUP) { | ||
| lookupIndices.add(p.indexPattern()); | ||
| } else if (indexMode.get() == null || indexMode.get() == p.indexMode()) { |
There was a problem hiding this comment.
leftover: In case that we somehow find 2 indices in the same mode, we're going to silently throw away the previous one.
There shouldn't be 2 indices. This is enforced in EsqlSession#preAnalyzeMainIndices.
| IndicesExpressionGrouper indicesGrouper, | ||
| XPackLicenseState licenseState, | ||
| List<IndexPattern> patterns, | ||
| IndexPattern index, |
There was a problem hiding this comment.
nit: we're moving the naming back to index, but we know this is generally a pattern. Also applies to PreAnalyzer.
There was a problem hiding this comment.
++ to what @alex-spies said, the name "index" can be confusing.
There was a problem hiding this comment.
Good point, updating
| IndicesExpressionGrouper indicesGrouper, | ||
| XPackLicenseState licenseState, | ||
| List<IndexPattern> patterns, | ||
| IndexPattern index, |
There was a problem hiding this comment.
++ to what @alex-spies said, the name "index" can be confusing.
BASE=c05c61d38cdac9c9608d6e8989f1fa7b9ca035cb HEAD=394aaef8034a4e09c6ef307a5ac9ac5d4f1e3f52 Branch=main
BASE=c05c61d38cdac9c9608d6e8989f1fa7b9ca035cb HEAD=394aaef8034a4e09c6ef307a5ac9ac5d4f1e3f52 Branch=main
BASE=c05c61d38cdac9c9608d6e8989f1fa7b9ca035cb HEAD=394aaef8034a4e09c6ef307a5ac9ac5d4f1e3f52 Branch=main
BASE=c05c61d38cdac9c9608d6e8989f1fa7b9ca035cb HEAD=394aaef8034a4e09c6ef307a5ac9ac5d4f1e3f52 Branch=main
BASE=c05c61d38cdac9c9608d6e8989f1fa7b9ca035cb HEAD=394aaef8034a4e09c6ef307a5ac9ac5d4f1e3f52 Branch=main
4 participants
We are using
IndexPatternclass to represent a set of patterns inFROM p1,p2or inLOOKUP JOIN p1.Today it is impossible to have more than a single
FROM, however we store main indices patterns in a list that leads to unnecessary complexity.This change replaces list with a nullable field.