NIFI-15305 Fix Date Time conversion for floating point as string #10697
+78
−25
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Added check for integral numbers greater than expected number of seconds for year 10,000 and handled as milliseconds
11 tasks
pvillard31
reviewed
Dec 30, 2025
Contributor
pvillard31
left a comment
pvillard31
left a comment
There was a problem hiding this comment.
LGTM, left some minor comments, not blockers.
Comment on lines
43
to
44
| private static final long MICROSECOND_MULTIPLIER = 1_000; | ||
| private static final long MILLISECOND_MULTIPLIER = 1_000_000; |
Contributor
There was a problem hiding this comment.
Thoughts about using something like MICROS_TO_NANOS and FRACTION_TO_MICROS_SECONDS to be more self-explanatory?
|
|
||
| private LocalDateTime toLocalDateTime(final long epochSeconds, final long nanosPastSecond) { | ||
| final Instant instant = Instant.ofEpochSecond(epochSeconds).plusNanos(nanosPastSecond); | ||
| private LocalDateTime convertIntegralFractional(final long integral, final double fractional) { |
Contributor
There was a problem hiding this comment.
I'd add a comment to explain the dual case logic. Could also be an in-line comment.
Suggested change
| private LocalDateTime convertIntegralFractional(final long integral, final double fractional) { | |
| /** | |
| * Convert integral and fractional components to LocalDateTime. | |
| * | |
| * Large integral values (beyond year 10,000 in seconds) indicate the input | |
| * is in milliseconds, with the fractional part representing microseconds. | |
| * Smaller integral values are treated as seconds, with the fractional part | |
| * representing the fraction of a second (converted to microseconds). | |
| * | |
| * Example: 1765056655230.746 → 1765056655230 ms + 746 µs | |
| * Example: 1707238288.351567 → 1707238288 s + 351567 µs | |
| */ | |
| private LocalDateTime convertIntegralFractional(final long integral, final double fractional) { |
Contributor
Author
|
Thanks for the review @pvillard31, I pushed an update adjusting the variable naming and adding some clarifying comments on the implementation. |
pvillard31
approved these changes
Dec 30, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
NIFI-15305 Corrects
StringtoLocalDateTimeconversion for floating point numbers represented as strings.The current implementation checks
longnumbers for values larger than the year 10,000, but does not implement a similar check for floating point numbers represented as strings.The corrected approach checks the integral part of a floating point number for values larger than the year 10,000 in seconds, and then handles the number as epoch milliseconds. The parsing strategy also handles the fractional part as either the number of microseconds or the number of milliseconds.
Tracking
Please complete the following tracking steps prior to pull request creation.
Issue Tracking
Pull Request Tracking
NIFI-00000NIFI-00000Pull Request Formatting
mainbranchVerification
Please indicate the verification steps performed prior to pull request creation.
Build
./mvnw clean install -P contrib-checkLicensing
LICENSEandNOTICEfilesDocumentation