Skip to content
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

Fix non-nullable datetime when zeroDateTimeBehavior is CONVERT_TO_NULL. #489

Merged
merged 1 commit into from
May 15, 2024

Conversation

damjad
Copy link
Contributor

@damjad damjad commented Mar 15, 2024

It's for the issue #488

The fix contains a check if the zeroDateTimeBehavior is to convert to null, make all date time like columns nullable.

@damjad
Copy link
Contributor Author

damjad commented Apr 1, 2024

Hello @itsankit-google,

Is there anything more that needs to be done?

@damjad
Copy link
Contributor Author

damjad commented May 1, 2024

Hello @itsankit-google,

Could you please have a look?
Could you please guide me if there's anything more that should be done?

We need this patch when we upgrade to the latest version of data fusion. Thanks!

Copy link
Member

@itsankit-google itsankit-google left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add unit tests.

@damjad damjad force-pushed the develop branch 3 times, most recently from b2dfd0b to 8418f8e Compare May 4, 2024 05:32
@damjad
Copy link
Contributor Author

damjad commented May 4, 2024

I have added the tests and run them.

I ran the following command:

mvn test -Dtest="MysqlSchemaReaderUnitTest,MysqlUtilUnitTest"

The output is as follows:

[INFO] -------------------------------------------------------
[INFO]  T E S T S
[INFO] -------------------------------------------------------
[INFO] Running io.cdap.plugin.mysql.MysqlSchemaReaderUnitTest
[INFO] Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.814 s - in io.cdap.plugin.mysql.MysqlSchemaReaderUnitTest
[INFO] validateZeroDateTimeBehavior(io.cdap.plugin.mysql.MysqlSchemaReaderUnitTest)  Time elapsed: 0.741 s
[INFO] validateYearTypeToStringTypeConversion(io.cdap.plugin.mysql.MysqlSchemaReaderUnitTest)  Time elapsed: 0.007 s
[INFO] Running io.cdap.plugin.mysql.MysqlUtilUnitTest
[INFO] Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.064 s - in io.cdap.plugin.mysql.MysqlUtilUnitTest
[INFO] testIsZeroDateTimeToNull(io.cdap.plugin.mysql.MysqlUtilUnitTest)  Time elapsed: 0.004 s
[INFO] testIsDateTimeLikeType(io.cdap.plugin.mysql.MysqlUtilUnitTest)  Time elapsed: 0 s
[INFO] 
[INFO] Results:
[INFO] 
[INFO] Tests run: 4, Failures: 0, Errors: 0, Skipped: 0

Copy link
Member

@itsankit-google itsankit-google left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also make the similar changes in CloudSQLMySQL plugin for consistency.

@itsankit-google
Copy link
Member

Please squash commits before we merge the PR, you can refer to steps here: data-integrations/firestore-plugins#18 (comment)

@damjad
Copy link
Contributor Author

damjad commented May 15, 2024

@itsankit-google squashed 🎊

@itsankit-google itsankit-google merged commit 3ab084b into data-integrations:develop May 15, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants