Skip to content

Conversation

@chrisparrinello
Copy link
Contributor

@chrisparrinello chrisparrinello commented Nov 4, 2025

Closes #136766.

The test creates a cycle in runtime fields that is broken if the doc with docId == 0's field 4 is accessed. The first part of the test attempts to only fetch the document with indexed_field first which should not fail because the runtime fields would not have a loop since first should have docId == 0. The second part fetches all of the documents which should fail since all of the documents are fetched and the runtime fields are calculated.

The random seed used in the failing test results in the document with first actually getting a docId of 1 causing the test to fail. Since we can't depend on docId to break the cycle in the runtime fields, changes were made to be able to use indexed_field == "first" to break the cycle in the first part of the test.

@elasticsearchmachine elasticsearchmachine added v9.3.0 needs:triage Requires assignment of a team area label labels Nov 4, 2025
@chrisparrinello chrisparrinello added >test Issues or PRs that are addressing/adding tests Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch :Search Foundations/Search Catch all for Search Foundations and removed needs:triage Requires assignment of a team area label labels Nov 4, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-search-foundations (Team:Search Foundations)

Copy link
Contributor

@benchaplin benchaplin left a comment

Choose a reason for hiding this comment

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

Nice find!

runtimeField("1", leafLookup -> leafLookup.doc().get("2").get(0).toString()),
runtimeField("2", leafLookup -> leafLookup.doc().get("3").get(0).toString()),
runtimeField("3", leafLookup -> leafLookup.doc().get("4").get(0).toString()),
runtimeField("4", (leafLookup, docId) -> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we remove docId?

try (Directory directory = newDirectory(); RandomIndexWriter indexWriter = new RandomIndexWriter(random(), directory)) {
indexWriter.addDocument(List.of(new StringField("indexed_field", "first", Field.Store.NO)));
indexWriter.addDocument(List.of(new StringField("indexed_field", "second", Field.Store.NO)));
indexWriter.addDocument(List.of(new KeywordField("indexed_field", "first", Field.Store.YES)));
Copy link
Contributor

Choose a reason for hiding this comment

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

You sure this doesn't mess with other tests that use this method? I would run @Repeat on the suite to be sure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Search Foundations/Search Catch all for Search Foundations Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch >test Issues or PRs that are addressing/adding tests v9.3.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CI] SearchExecutionContextTests testFielddataLookupSometimesLoop failing

3 participants