[common] Support all TIMESTAMP precisions in the numeric string cast - #9622
Open
LuciferYang wants to merge 1 commit into
Open
[common] Support all TIMESTAMP precisions in the numeric string cast#9622LuciferYang wants to merge 1 commit into
LuciferYang wants to merge 1 commit into
Conversation
fromMillisToTimestamp handled only precisions 0, 3, 6 and 9 and threw "Unsupported precision" for the rest, so a numeric string could not be cast to TIMESTAMP(1/2/4/5/7/8) even though TimestampType allows 0..9. Those four cases are one formula sampled at four points: the value counts units of 10^-precision seconds, so a unit is 10^(3 - precision) milliseconds. Keep the formula and drop the cases.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Purpose
close #9621
BinaryStringUtils.fromMillisToTimestampimplemented the numeric-string to TIMESTAMP conversion as aswitchover precisions 0, 3, 6 and 9, and threwRuntimeException("Unsupported precision: N")for everything else.TimestampTypeallows 0 through 9, soCAST('1700000000' AS TIMESTAMP(4))failed on a perfectly legal column type while the same string worked at precision 3.The four implemented cases are one formula sampled at four points: the value counts units of 10^-precision seconds, so a unit is 10^(3 - precision) milliseconds. Writing that formula covers all ten precisions, and it produces exactly what the four cases produced. At precision 6 the old code computed
epoch / 1000and(epoch % 1000) * 1000; the formula givesdivisor = 1000,epoch / 1000and(epoch % 1000) * 1_000_000 / 1000, which is the same value. Precisions 0, 3 and 9 line up the same way, so the four cases are gone rather than left beside the general path.The remainder is scaled before it is divided, so no digit the string carried is lost:
"170000000012"at precision 8 gives 1700000 ms plus 120 ns. The existing negative-nanos correction below the branch is unchanged and still does the borrow, so"-1700000001"at precision 7 gives -170001 ms plus 999900 ns rather than a negative offset. Out-of-range precisions still throw the same message, now checked once against 0..9 instead of falling out of aswitch.Tests
BinaryStringUtilsTest.testToTimestampgains nine rows: one hour expressed in each of the six previously rejected units, two cases where the remainder lands below a millisecond and has to come back as nanos-of-millisecond (precision 5 and precision 8), and a negative epoch at precision 7.testInvalidPrecisionsdrops 1, 2, 4, 5, 7 and 8, keeping 10 and -1.Against the unfixed code nine of those rows error with
RuntimeException: Unsupported precision.mvn -pl paimon-common -Dtest=BinaryStringUtilsTest teston JDK 8: 31 tests, 0 failures.spotless:checkandcheckstyle:checkon paimon-common are clean.