-
Notifications
You must be signed in to change notification settings - Fork 2.9k
NIFI-15305 Fix PutDatabaseRecord Timestamp Parsing #10613
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
Conversation
exceptionfactory
left a comment
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 for proposing this improvement @taylorobyen.
On initial read, this appears to break expected behavior. As indicated in the changes for NIFI-12710, the parsing logic expects that a floating point number contains seconds before the period, and nanoseconds after the period.
The proposed changes appear to alter the behavior, expecting milliseconds instead of seconds before the period.
Can you provide more detail on the expected use case where the input field contains milliseconds.nanoseconds?
There may be options to support a change based on maximum expected numbers of seconds versus milliseconds, but current behavior should be preserved.
| // Less precise timestamp than other tests as double is less precise than BigDecimal | ||
| final double timestamp = 1764673335503.607; | ||
|
|
||
| final BigDecimal bd = new BigDecimal(Double.toString(timestamp)); |
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.
Is there a particular reason for converting the timestamp double to a String, as opposed to just passing it to the BigDecimal constructor?
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.
Passing a double directly to the BigDecimal constructor preserves the binary floating-point approximation rather than the intended decimal value. I’ve updated this to use BigDecimal.valueOf() instead.
|
If it is intended behavior to parse the timestamp differently based off the input, I think the reader controller services should have their descriptions updated. In NIFI-12710, it modifies the parsing under the hood, but nothing was updated description wise to indicate to the end user that this behavior has changed. When I upgraded my production instances of NiFi from 1.20.0 to NiFi 2.5.0 and noticed that my timestamps were now being parsed incorrectly the first place I checked was the JsonTreeReader timestamp field to see if parsing had changed. It wasn't until I checked out the project and viewed the source code that I found that I need to provide my milliseconds as integers to have my timestamps parsed correctly. The current description from JsonTreeReader only mentions milliseconds (other readers such as CSV also have the same description):
My use case for these timestamps is that I load JSON payloads into my PostgreSQL database which have timestamp fields generated in Python i.e: ms = int(time.time() * 1000)Which will generate something like Instead of inferring the epoch unit from the numeric value, it may be better to let the user explicitly specify the unit, defaulting to milliseconds to preserve existing behavior. For example, the timestamp format could accept |
2ae4f01 to
9bc128e
Compare
|
Thanks for the reply @taylorobyen. After evaluating the details, the core of the current problem is that the conversion process does not handle floating point numbers, represented as strings, with the same level of sanity checking as It is important to preserve current parsing behavior in a way that also addresses the current issue, if at all possible. With that goal in mind, I put together an alternative pull request that addresses the core issue in #10697. If you are able to evaluate that approach and test it in your environment, that would be helpful for comparison. |
|
Thanks again for raising this issue and proposing an initial approach @taylorobyen. I'm closing this in favor the alternative in #10697 that preserves existing behavior, but handles this scenario. If you observe additional issues, feel free to raise a new Jira issue for evaluation. |

Summary
NIFI-15305 Fixes PutDatabaseRecord's inconsistent timestamp parsing when handling epoch timestamps.
Epoch timestamps are supposed to be in milliseconds, but when fractional milliseconds are included in the timestamp it is incorrectly handled as seconds resulting in dates very far in the future.
Behavior prior to my patch:
Correct handling of a timestamp with only whole milliseconds:
{"ts": 1765056655230}Timestamp loaded into PostgreSQL:
2025-12-06 16:30:55.230 -0500✅Incorrect handling of a timestamp that includes fractional milliseconds:
{"ts": 1765056655230.746}Timestamp loaded into PostgreSQL:
57902-06-03 07:20:30746. -0400❌This bug was introduced in #8332, which was trying to allow for microsecond precision for epoch timestamps. Before #8332, the fractional milliseconds were truncated as the timestamp was always converted from
Stringtolong.My implementation allows for microsecond precision while handling all epoch timestamps as 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