[common] Range-check the digit guard in DateTimeUtils - #9649
Conversation
parseDate and parseTime return null for unparseable input and guard every Integer.parseInt with isInteger, which only checked that the characters were digits. An eleven-digit component passed it and parseInt threw NumberFormatException out of a method whose other failure paths return null. Check the range in the guard, in front of all eleven parseInt calls.
JingsongLi
left a comment
There was a problem hiding this comment.
The overflow guard has a real caller through Hive's TIME conversion, and the 12 DateTimeUtils tests pass with the changed class. There is a narrow compatibility regression in the new length shortcut that should be addressed without changing accepted numeric values.
| */ | ||
| private static boolean isInteger(String s) { | ||
| boolean isInt = s.length() > 0; | ||
| if (s.isEmpty() || s.length() > 10) { |
There was a problem hiding this comment.
[P2] Range-check the value without rejecting leading zeros. This length limit rejects components whose numeric value still fits an int: parseDate("00000002024-01-01") and parseTime("00000000012:30:00") return 19723 and 45000000 on the base revision, but both return null with this change (confirmed in an isolated base/head probe). Through PaimonTimeObjectInspector.convert, previously accepted zero-padded TIME values are therefore silently replaced with NULL. Ignore leading zeros for the range check, or use an overflow-checked accumulator, and add regression coverage alongside the genuinely out-of-range inputs.
Purpose
close #9648
DateTimeUtils.parseDateandparseTimereturn null for input they cannot parse, and they guard everyInteger.parseIntwithisInteger. That guard only checked the characters:A component of eleven digits therefore passed it and
Integer.parseIntthrewNumberFormatException, escaping a method whose every other failure path returns null.parseDate("2147483648-01-01")andparseTime("2147483648:00:00")both do it.The guard now also checks the range, which is enough because it sits in front of each of the eleven
parseIntcalls in those two methods. Nothing else changes: no signature, no local variable type, no arithmetic.Callers see the difference as an exception type. Through the casts,
NumberFormatExceptionbecomes theDateTimeExceptionthatBinaryStringUtils.toDateraises for any unparseable string, which is what a caller already gets for"99999-01-01"(rejected byisIllegalDate) or"not-a-date". The HivePaimonTimeObjectInspector.convertpasses theIntegerthrough, so a TIME column holding such a value writes NULL rather than failing, matching what it already does for other invalid times.Tests
DateTimeUtilsTest.testParseDateAndTimeOverflowReturnsNullcovers a too-large year, month, day and hour, and the two boundary cases:2147483648is the smallest ten-digit value that does not fit anint, and2147483647does fit but is still not a valid year, so both have to come back null through different branches. Valid values are asserted alongside them.Against the unfixed guard the test errors with
NumberFormatException: For input string: "2147483648".mvn -pl paimon-common -Dtest=DateTimeUtilsTest teston JDK 8: 12 tests, 0 failures.spotless:checkandcheckstyle:checkon paimon-common are clean.