Skip to content

Conversation

@romseygeek
Copy link
Contributor

Relates to #15480

Placeholder values for SortFields are set via setMissingValue(), after construction, which means
that they are inherently mutable. This commit deprecates setMissingValue() and instead
adds constructor parameters for missing values.

@github-actions github-actions bot added this to the 10.4.0 milestone Dec 8, 2025
Copy link
Member

@benwtrent benwtrent left a comment

Choose a reason for hiding this comment

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

Its weird that its mutable to begin with...seems like a mistake. I wonder if it was mutable because certain sort fields just don't support setting missing value?

I did a quick look through the code base and through Solr & ES, there weren't obvious places where it was actually treated as mutable. Generally the pattern was 'construct...some logic...immediately set the value once on the new instance'

@romseygeek
Copy link
Contributor Author

I wonder if it was mutable because certain sort fields just don't support setting missing value?

TBH I think it was immutable because it was 2002 and that's how Java was written then. And as you say, it hasn't really been an issue with actual usage. Even now, it's only really a problem for index sorts, although I suppose there could be some weird concurrency issues if a SortField was shared across multiple threads.

@romseygeek romseygeek merged commit 75108f4 into apache:main Dec 8, 2025
12 checks passed
@romseygeek romseygeek deleted the sort/missing-values branch December 8, 2025 16:00
romseygeek added a commit that referenced this pull request Dec 8, 2025
Placeholder values for SortFields are set via setMissingValue(), after construction, which means
that they are inherently mutable. This commit deprecates setMissingValue() and instead
adds constructor parameters for missing values.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants