[Spark] Fix CDC Commit Timestamp value under different Timezones #3347
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
longvu-db
commented
Jul 9, 2024
| modifyDeltaTimestamp(deltaLog, 1, currentTime) | ||
| modifyDeltaTimestamp(deltaLog, 2, currentTime + 100000) | ||
|
|
||
| val readDf = sql(s"SELECT * FROM table_changes('$tbl', 0, now())") |
There was a problem hiding this comment.
now() and current_date() aren't affected by SQLConf.SESSION_LOCAL_TIMEZONE, while we are using the Java's new Date().getTime to generate time under the JVM timezone by the testing environment, in this case the JVM timezone in Delta testing is Pacific Timezone I believe (they set it not UTC on purpose to catch any timezone test failures).
Thus, I use the withDefaultTimeZone to modify the JVM timezone to UTC in this test so that we can test the interaction with timestamp expressions.
longvu-db
commented
Jul 9, 2024
| @@ -250,13 +251,11 @@ abstract class DeltaCDCSuiteBase | |||
| // modify timestamps | |||
| // version 0 | |||
| modifyDeltaTimestamp(deltaLog, 0, 0) | |||
| val tsAfterV0 = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss") | |||
| .format(new Date(1)) | |||
| val tsAfterV0 = dateFormat.format(new Date(1)) | |||
There was a problem hiding this comment.
In this PR, we change so that we take into account timezone, these test failed because they didn't take into account timezones, so I'm modifying them to take into account timezone.
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.
Which Delta project/connector is this regarding?
Description
The CDC's
_commit_timestampis incorrect when we try to read/display it under a different Spark Session's timezonespark.sql.session.timeZone(e.g.America/Chicago,Asia/Ho_Chi_Minh, ...).In this PR, we address this issue by taking into account timezone to capture the precise point in time when we convert
CDCDataSpec's Java Timestamp field to Spark's Timestamp for the_commit_timestampcolumn for all CDC's file indexes (CDCAddFileIndex,TahoeRemoveFileIndex,TahoeChangeFileIndex).This is needed in order for CDF to work properly under a different timezone than
UTC.How was this patch tested?
Added UT, some minor UTs fix to take into account timezone.
Does this PR introduce any user-facing changes?
Yes.
_commit_timestampshould now be correct when we try to read/display it under a different Spark Session's timezonespark.sql.session.timeZone(e.g.America/Chicago,Asia/Ho_Chi_Minh, ...).