-
Notifications
You must be signed in to change notification settings - Fork 748
[GOBBLIN-1479] Add support to convert zeroDateTime to null when configured #3319
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| batchSize = (batchSize == 0 ? ConfigurationKeys.DEFAULT_SOURCE_FETCH_SIZE : batchSize); | ||
|
|
||
| String sourceConnProps = this.workUnitState.getProp(ConfigurationKeys.SOURCE_CONN_PROPERTIES); | ||
| boolean convertZeroDateTimeToNull = sourceConnProps != null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think here we can generalize this a bit more rather than keep it only to the zeroDateTime behavior, to support the other zeroDateTime properties if there are any users. We can check that sourceConnProps contains zeroDateTimeBehavior, then parse it as timestamp and return the result. This way it can handle the ROUND case
|
Thanks for the fix! LGTM |
autumnust
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for fixing, the change itself LGTM.
Since we have org.apache.gobblin.source.jdbc.JdbcExtractorTest, can we add a unit test for the change we made here?
Codecov Report
@@ Coverage Diff @@
## master #3319 +/- ##
============================================
+ Coverage 46.30% 46.40% +0.10%
- Complexity 9972 10001 +29
============================================
Files 2031 2031
Lines 78994 79002 +8
Branches 8831 8834 +3
============================================
+ Hits 36575 36661 +86
+ Misses 39010 38938 -72
+ Partials 3409 3403 -6
Continue to review full report at Codecov.
|
autumnust
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM !
|
Is the failing tests relevant ? |
In MySQL Connector 8, calling ResultSet.getString(int) on a record with a zero timestamp returns the string "0000-00-00 00:00:00", even when the connection property `zeroDateTimeBehavior=CONVERT_TO_NULL` is set. This change is a workaround to return null for these timestamps when the property is set.
Dear Gobblin maintainers,
Please accept this PR. I understand that it will not be reviewed until I have checked off all the steps below!
JIRA
Description
In MySQL Connector 8, calling ResultSet.getString(int) on a record with a zero timestamp
returns the string "0000-00-00 00:00:00", even when the connection property
zeroDateTimeBehavior=CONVERT_TO_NULLis set.This change is a workaround to return null for these timestamps when the property is set.
Tests
Commits