Move resolve aliases to IndexAbstractionOptions#143953
Conversation
There has been several discussions on this topic since I opened #141050 and @idegtiarenko also mentioned this in #143561 (comment). The issue is that both `resolveViews` and `resolveAliases` are in `WildcardOptions` but are used when resolving both concrete index pattern targets and wildcard targets. This PR moves `resolveViews` from `IndicesOptions.WildcardOptions` into a new `IndicesOptions.IndexAbstractionOptions` record to address this confusing inconsistency. This is in preparation for sending `resolveViews` as a parameter to field caps for remote view resolving where more work will be done to serialize `resolveViews` and not having this in place would make for confusing code. See #143384 for more information on the upcoming serialization of `resolveViews`. **_Note_**: `resolveAliases` is moved in a follow up PR since it's decoupled from the resolveViews changes. #143953 **_Note_**: `IndicesRequest.includeDataStreams()` is the same type of flag as `resolveAliases` and `resolveViews`, it controls whether data streams participate in index resolution. It's a candidate for `IndexAbstractionOptions` but currently lives on `IndicesRequest` (not `IndicesOptions`) and is threaded separately through `IndexNameExpressionResolver.Context` as a constructor parameter. Moving it would touch 60+ files and change the `IndicesRequest` interface. Because of how big that change would be, I have not considered doing that in this PR (or at all).
6010006 to
015e98e
Compare
30caf92 to
1402f33
Compare
|
Pinging @elastic/es-security (Team:Security) |
|
|
||
| public static Builder builder(IndexAbstractionOptions indexAbstractionOptions) { | ||
| return new Builder(indexAbstractionOptions); | ||
| } |
There was a problem hiding this comment.
NIT: we have 2 static builder methods today: one for starting from scratch and one to start editing other abstraction. I wonder if we should change it to be instance method:
IndicesOptions.builder(options.indexAbstractionOptions())...build() -> options.indexAbstractionOptions().builder()...build().
It is different from existing approach but it looks a bit cleaner to me.
There was a problem hiding this comment.
I agree that is a lot nicer, but the other IndicesOptions are using that style of builder so I think it would be inconsistent and require a full refactor of all of them, which I think is out of scope for this PR.
There was a problem hiding this comment.
I agree, this is not blocking this PR
* Move resolveAliases to IndexAbstractionOptions * fixup! Remove explicit default
…elocations * upstream/main: (33 commits) Unmute InferenceRestIT and DefaultEndPointsIT (elastic#144217) feat: add keep_alive to async task status (elastic#144010) Add explicit isNoOpUpdate() method to MapperService (elastic#144113) Always attach APM Agent (elastic#144120) Fix random_score nightly tests (elastic#144176) Add nested query checks for disabled sequence numbers (elastic#144185) Return sentinel values from Fetch when sequence numbers are disabled (elastic#144212) [Test] Test peer-recovery with sequence numbers pruning (elastic#144116) Remove `scaled-*` field assertions from mixed cluster downsampling test (elastic#144295) Refactor: Use range syntax in ES|QL exponential histogram tests (elastic#144110) Move resolve aliases to IndexAbstractionOptions (elastic#143953) unmute test (elastic#144299) Fix approximation csvtests (elastic#144233) fix test (elastic#144171) Add int4 vector scoring benchmarks (elastic#144105) Mute org.elasticsearch.xpack.esql.qa.single_node.GenerativeIT test elastic#143023 Mute org.elasticsearch.test.apmintegration.MetricsApmIT testApmIntegration {withOTel=false} elastic#144282 Native cli launcher (elastic#143712) Mute org.elasticsearch.xpack.esql.qa.multi_node.GenerativeIT test elastic#143023 Mute org.elasticsearch.xpack.esql.heap_attack.HeapAttackSubqueryIT testManyRandomKeywordFieldsInSubqueryIntermediateResults elastic#144274 ...
* Move resolveAliases to IndexAbstractionOptions * fixup! Remove explicit default
This is a follow up from #143741 to also move resolve aliases to
IndexAbstractionOptions.