Skip to content
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

Prevent unbounded latency converting decimals to time #87

Merged
merged 3 commits into from
Oct 24, 2018

Conversation

toddjonker
Copy link
Contributor

This change prevents latency explosions when working with high-magnitude BigDecimal values, while preserving the current behavior on the edge cases.

It's fixes the original bug reported in FasterXML/jackson-databind#2141 but not the several related issues discussed therein. It builds atop my earlier PR #85

One challenge here is that the current two-step conversion process, using BigDecimal.longValue() and DecimalUtils. extractNanosecondDecimal() separately, made it hard to control the edge cases because information is lost during the former. So I combined the two into a single helper method.

(IMO these helpers shouldn't be public API, since they are bespoke semantics needed by this package, and unlikely to be usable by other contexts. Frankly I'd prefer to make the new helper method package-protected, if that's okay with the maintainers.)

@cowtowncoder
Copy link
Member

Ok, yes, excellent thank you for the fix!

+1 for hiding functionality (I often use protected as compromise to allow sub-classing but not advertise it). I also think that deprecation here makes sense, and then dropping method off in 3.0.

I can make that change after merge.

Let's see if I can continue discussion on remaining challenge after I merge this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants