rootvector2 commented on code in PR #433:
URL: https://github.com/apache/commons-beanutils/pull/433#discussion_r3744264015
##########
src/main/java/org/apache/commons/beanutils2/converters/DateTimeConverter.java:
##########
@@ -200,7 +200,7 @@ protected <T> T convertToType(final Class<T> targetType,
final Object value) thr
// didn't include the milliseconds. The following code
// ensures it works consistently across JDK versions
final java.sql.Timestamp timestamp = (java.sql.Timestamp) value;
- long timeInMillis = timestamp.getTime() / 1000 * 1000;
+ long timeInMillis = Math.floorDiv(timestamp.getTime(), 1000) *
1000;
timeInMillis += timestamp.getNanos() / 1000000;
return toDate(targetType, timeInMillis);
Review Comment:
verified. the multiply does wrap for `getTime()` in `[Long.MIN_VALUE,
Long.MIN_VALUE + 807]`, but adding the non-negative `getNanos() / 1000000`
wraps it back: the two terms reconstruct `getTime()` exactly in
two's-complement arithmetic, so the final millis are correct and no guard is
needed. the old truncating expression avoided the intermediate wrap but still
returned a value off by +1000 in that same range. added
`testConvertExtremePreEpochSqlTimestamp` pinning `Long.MIN_VALUE` and
`Long.MIN_VALUE + 807` (the last value whose whole-second term wraps); full
`mvn` build is green.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
