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 temporal type default value bug mysql #15917

Merged
merged 6 commits into from
Aug 25, 2022

Conversation

subodh1810
Copy link
Contributor

Issue : #15840
The bug is happening for datetime, time and date data types with default values. While building the internal schema of the columns, debezium is failing with the error.

@subodh1810 subodh1810 requested a review from a team as a code owner August 24, 2022 07:36
@subodh1810 subodh1810 self-assigned this Aug 24, 2022
@github-actions github-actions bot added the area/connectors Connector related issues label Aug 24, 2022
@subodh1810
Copy link
Contributor Author

subodh1810 commented Aug 24, 2022

/test connector=connectors/source-mysql

🕑 connectors/source-mysql https://github.com/airbytehq/airbyte/actions/runs/2917299699
✅ connectors/source-mysql https://github.com/airbytehq/airbyte/actions/runs/2917299699
No Python unittests run

Build Passed

Test summary info:

All Passed

return DateTimeConverter.convertToTime(LocalTime.ofNanoOfDay(l));
}
return DateTimeConverter.convertToTime(x);
case "TIMESTAMP":
Copy link
Contributor

Choose a reason for hiding this comment

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

just curious: did you test with different precisions? It seems kind of weird that debezium would have special handling for datetime but not timestamp (and that timestamp isn't actually in their table >.> )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup. They have listed it in the docs as well

Excluding the TIMESTAMP data type, MySQL temporal types depend on the value of the time.precision.mode connector configuration property.

};
switch (fieldType.toUpperCase(Locale.ROOT)) {
case "DATETIME":
if (x instanceof Long) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: x instanceof Long l

@github-actions
Copy link
Contributor

NOTE ⚠️ Changes in this PR affect the following connectors. Make sure to run corresponding integration tests:

  • source-mysql
  • source-postgres

@github-actions github-actions bot added the area/documentation Improvements or additions to documentation label Aug 25, 2022
@subodh1810
Copy link
Contributor Author

subodh1810 commented Aug 25, 2022

/publish connector=connectors/source-mysql

🕑 Publishing the following connectors:
connectors/source-mysql
https://github.com/airbytehq/airbyte/actions/runs/2924387002


Connector Did it publish? Were definitions generated?
connectors/source-mysql

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

@github-actions
Copy link
Contributor

NOTE ⚠️ Changes in this PR affect the following connectors. Make sure to run corresponding integration tests:

  • source-postgres
  • source-mysql
  • source-mysql-strict-encrypt

@subodh1810
Copy link
Contributor Author

subodh1810 commented Aug 25, 2022

/publish connector=connectors/source-mysql-strict-encrypt

🕑 Publishing the following connectors:
connectors/source-mysql-strict-encrypt
https://github.com/airbytehq/airbyte/actions/runs/2924389059


Connector Did it publish? Were definitions generated?
connectors/source-mysql-strict-encrypt

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

@subodh1810 subodh1810 merged commit 264b72f into master Aug 25, 2022
@subodh1810 subodh1810 deleted the fix-mysql-temporal-datetime-failure-bug branch August 25, 2022 08:00
@danieldiamond
Copy link
Contributor

this worked great, thanks so much for the fast turnaround @subodh1810 !

rodireich pushed a commit that referenced this pull request Aug 25, 2022
* fix handling for temporal data type columns with default values

* add tests

* minor NIT comment

* bump version

* auto-bump connector version [ci skip]

Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com>
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.

5 participants