SNOW-1032874: Fix an Overflow Issue for Integer-store Timestamp #673
Conversation
This file contains 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
| long val = Long.parseLong(input); | ||
|
|
||
| if (val > -SECONDS_LIMIT_FOR_EPOCH && val < SECONDS_LIMIT_FOR_EPOCH) { | ||
| epochNanos = BigInteger.valueOf(val).multiply(BigInteger.valueOf(Power10.intTable[9])); |
There was a problem hiding this comment.
nit, use Power10.sb16Table[9]
| Assert.assertEquals(123000000, wrapper.getFraction()); | ||
| Assert.assertEquals(new BigInteger("1609462800123000000"), wrapper.toBinary(false)); | ||
|
|
||
| // Test integer-stored time and scale guessing | ||
| SimpleDateFormat df = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss.SSS"); |
There was a problem hiding this comment.
I think we also need integration test coverage, which would test JDBC compatibility. We have a bunch of IT tests for scale guessing in DateTimeIT.java, just not for these edge cases. Could you add tests along the lines of these?
…-ingest-java into tzhang-si-overflow
sfc-gh-xhuang
approved these changes
Feb 3, 2024
|
Will merge this one first, we might need to update the logic again once we hear back from SQL folks about the behavior for negative timestamps. |
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.
According to https://docs.snowflake.com/en/sql-reference/functions/to_timestamp#usage-notes:
Due to the multiplication logic in the code, we run into an overflow issue when the input is bigger than a certain value (for example: 31535999999999) and it causes silent corrupted values in the table, this change updates the code to use BigInteger instead of long to resolve the overflow issue.