Repository navigation
[Ready] SOLR-16673: The Parse{Int,Long,Float,Double}FieldUpdateProcessorFactory classes no longer fail with a NullPointerException on an empty string value - #5094
Conversation
…; add empty-string test and changelog
|
Posted an independent vetting pass on this PR (reproduced the bug on Verdict: approve, with one note — worth explicitly calling out in the description why this takes a different approach than the |
Thanks for the review, I made the changes to the PR description and added the text to the reference guide. |
|
I've tagged @HoustonPutman since he opened up the orgininal ticket. This all seems more or less reasonable! |
🤖 AI text below 🤖 (posted on behalf of Nick Shanin)
Ready for review, but held as a draft due to the project's open-PR cap; I'll mark it open on request.
https://issues.apache.org/jira/browse/SOLR-16673
What happens today
An empty string in a Parse{Int,Long,Float,Double}FieldUpdateProcessorFactory field throws a NullPointerException.
NumberFormat.parsereturns null for an empty string. The bounds check still passes, because the parse position and the string length are both 0. The code then calls a method on the null result, for examplenumber.intValue()in ParseIntFieldUpdateProcessorFactory.java.What this change does
An empty string that does not parse is skipped, so the value stays as it was.
Each of the four factories treats a null parse result as unparseable. The check is now
number == null || pos.getIndex() != stringVal.length()in Int, Long, Float, and Double. The value is then kept. The Long and Double changes also reach Schema Designer's type guess, in DefaultSchemaSuggester.isIntOrLong and in the float and double check. In that path an empty value no longer throws. It counts as not a number, and the guess moves on to the other type checks.This is a behavior change, not only a crash fix. An empty string used to fail the request with the NullPointerException above. Now the request goes through and the value is left unchanged.
On the ticket, Houston Putman suggested a narrower fix: check whether the string is empty and return null in that case. This change takes the broader route. A null parse result counts as unparseable, like a value that only partly parses, so an empty string is treated like any other value that does not fully parse, and the field value is left unchanged rather than dropped.
Proof
The full test class passes at the current head, and the new test fails without the fix.
2adeae3bf8on 2026-10-12, the changelog parses, tidy passes, the Error Prone compile passes, and the module checks pass.ParsingFieldUpdateProcessorsTestruns 43 tests with 0 failures. The new test fails on the code without the fix.d5c19e64ba1bchange no production code: one narrows the changelog fragment title, and one adds a ref guide sentence stating that non-numeric values, including empty strings, are left unmodified.Limits
The other errors in the ticket sample are not fixed, and no test runs the Schema Designer path.
_root_type errors. This change does not touch them. A follow-up submission is planned to cover them.Changelog: changelog/unreleased/SOLR-16673.yml (type: fixed).
AI assistance
AI agents assisted with research, implementation, review, and drafting. Nick Shanin directed the work and takes responsibility for this contribution.