-
Notifications
You must be signed in to change notification settings - Fork 25.4k
test: fix SparseVectorFieldMapperTests edge case #132937
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
test: fix SparseVectorFieldMapperTests edge case #132937
Conversation
Pinging @elastic/search-eng (Team:SearchOrg) |
Pinging @elastic/search-relevance (Team:Search - Relevance) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks investigating and implementing the fix! I left one material comment about how we're getting index versions, other than that we're good
server/src/test/java/org/elasticsearch/index/mapper/vectors/SparseVectorFieldMapperTests.java
Outdated
Show resolved
Hide resolved
IndexVersion indexVersion = ESTestCase.randomFrom( | ||
IndexVersionUtils.allReleasedVersions().subSet(SPARSE_VECTOR_PRUNING_INDEX_OPTIONS_SUPPORT_BACKPORT_8_X, IndexVersion.current()) | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a TODO here about why we are using randomFrom
and that we should go back to IndexVersionUtils.randomVersionBetween
once issues have been resolved with that?
The other thing we would want to improve here is making it equally likely we get one of these cases:
- 8.x index version prior to index options support
- 8.x index version with index options support
- 9.x index version prior to index options support
- 9.x index version with index options support
As written, the likelihood that we get an index version prior to index options support will shrink as the number of versions between SPARSE_VECTOR_PRUNING_INDEX_OPTIONS_SUPPORT_BACKPORT_8_X
and IndexVersion.current()
increases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure! Updated in 1b60a47
…maios/elasticsearch into 132810_fix_sparse_vector_field_tests
Closes #132810
Follow up from: #132264
This PR: