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.

Sign up for GitHub

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

Jump to bottom

JDBC sources: Fix unexcepcted long Integer value failure #3846

Merged
merged 4 commits into from
Jun 4, 2021
Merged

JDBC sources: Fix unexcepcted long Integer value failure #3846

merged 4 commits into from
Jun 4, 2021

Conversation

DoNotPanicUA
Copy link
Contributor

@DoNotPanicUA DoNotPanicUA commented Jun 3, 2021 •
edited
Loading

Issue: #3840

What

Fix an error when JDBC source produces Integer value bigger than max Java Integer value.

How

Implement handler which tries to convert into Integer first. My assumption is that at most cases the value will be in the range.
If it fails, the handler tries to convert into the Long type. In all other negative cases it will replace value by null.

Pre-merge Checklist

  • Run integration tests
  • Publish Docker images

Recommended reading order

  1. JdbcUtils.java

@DoNotPanicUA
implement additional handler for the Integer source values in order t…
bfd05f6 
…o cover the case with too large value (possible for some sources)
@DoNotPanicUA
format
a218b46
@DoNotPanicUA DoNotPanicUA self-assigned this Jun 3, 2021
@DoNotPanicUA DoNotPanicUA linked an issue Jun 3, 2021 that may be closed by this pull request
Source MySQL: unsigned numeric values larger than signed integer size break connector #3840
Closed
@DoNotPanicUA DoNotPanicUA marked this pull request as ready for review June 3, 2021 18:49
sherifnada
sherifnada approved these changes Jun 3, 2021
View reviewed changes
Copy link
Contributor

@sherifnada sherifnada left a comment

Choose a reason for hiding this comment

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

LGTM -- please publish and merge. Also, can you add this to the data type tests?

BTW see the definition-of-done checklist here to help clarify what next steps would be: #3864

@sherifnada
Copy link
Contributor

@DoNotPanicUA also can you update the PR title to match best practices for PR titles?

@DoNotPanicUA DoNotPanicUA changed the title 3840 fix mysql JDBC sources: Fix unexcepcted long Integer value failure Jun 4, 2021
@DoNotPanicUA
Copy link
Contributor Author

DoNotPanicUA commented Jun 4, 2021 •
edited by github-actions bot
Loading

/test connector=source-mysql

🕑 source-mysql https://github.com/airbytehq/airbyte/actions/runs/905415719
✅ source-mysql https://github.com/airbytehq/airbyte/actions/runs/905415719

@DoNotPanicUA
Copy link
Contributor Author

DoNotPanicUA commented Jun 4, 2021 •
edited by github-actions bot
Loading

/test connector=source-jdbc

🕑 source-jdbc https://github.com/airbytehq/airbyte/actions/runs/905437299
✅ source-jdbc https://github.com/airbytehq/airbyte/actions/runs/905437299

@DoNotPanicUA
Copy link
Contributor Author

DoNotPanicUA commented Jun 4, 2021 •
edited by github-actions bot
Loading

/test connector=source-mssql

🕑 source-mssql https://github.com/airbytehq/airbyte/actions/runs/905438603
✅ source-mssql https://github.com/airbytehq/airbyte/actions/runs/905438603

@DoNotPanicUA
Copy link
Contributor Author

DoNotPanicUA commented Jun 4, 2021 •
edited by github-actions bot
Loading

/test connector=source-oracle

🕑 source-oracle https://github.com/airbytehq/airbyte/actions/runs/905439792
✅ source-oracle https://github.com/airbytehq/airbyte/actions/runs/905439792

@DoNotPanicUA
Copy link
Contributor Author

DoNotPanicUA commented Jun 4, 2021 •
edited by github-actions bot
Loading

/test connector=source-postgres

🕑 source-postgres https://github.com/airbytehq/airbyte/actions/runs/905440971
✅ source-postgres https://github.com/airbytehq/airbyte/actions/runs/905440971

@DoNotPanicUA
Copy link
Contributor Author

DoNotPanicUA commented Jun 4, 2021 •
edited by github-actions bot
Loading

/test connector=source-redshift

🕑 source-redshift https://github.com/airbytehq/airbyte/actions/runs/905442071
✅ source-redshift https://github.com/airbytehq/airbyte/actions/runs/905442071

@DoNotPanicUA
Copy link
Contributor Author

DoNotPanicUA commented Jun 4, 2021 •
edited by github-actions bot
Loading

/test connector=source-clickhouse

🕑 source-clickhouse https://github.com/airbytehq/airbyte/actions/runs/905443231
✅ source-clickhouse https://github.com/airbytehq/airbyte/actions/runs/905443231

@DoNotPanicUA
Copy link
Contributor Author

DoNotPanicUA commented Jun 4, 2021 •
edited by github-actions bot
Loading

/test connector=destination-snowflake

🕑 destination-snowflake https://github.com/airbytehq/airbyte/actions/runs/905488975
✅ destination-snowflake https://github.com/airbytehq/airbyte/actions/runs/905488975

@DoNotPanicUA
Copy link
Contributor Author

DoNotPanicUA commented Jun 4, 2021 •
edited by github-actions bot
Loading

/test connector=destination-jdbc

🕑 destination-jdbc https://github.com/airbytehq/airbyte/actions/runs/905491481
✅ destination-jdbc https://github.com/airbytehq/airbyte/actions/runs/905491481

@DoNotPanicUA
Copy link
Contributor Author

DoNotPanicUA commented Jun 4, 2021 •
edited by github-actions bot
Loading

/test connector=destination-mysql

🕑 destination-mysql https://github.com/airbytehq/airbyte/actions/runs/905493539
✅ destination-mysql https://github.com/airbytehq/airbyte/actions/runs/905493539

@DoNotPanicUA
Copy link
Contributor Author

DoNotPanicUA commented Jun 4, 2021 •
edited by github-actions bot
Loading

/test connector=destination-mssql

🕑 destination-mssql https://github.com/airbytehq/airbyte/actions/runs/905494132
✅ destination-mssql https://github.com/airbytehq/airbyte/actions/runs/905494132

@DoNotPanicUA
Copy link
Contributor Author

DoNotPanicUA commented Jun 4, 2021 •
edited by github-actions bot
Loading

/test connector=destination-postgres

🕑 destination-postgres https://github.com/airbytehq/airbyte/actions/runs/905495886
✅ destination-postgres https://github.com/airbytehq/airbyte/actions/runs/905495886

@DoNotPanicUA
Copy link
Contributor Author

DoNotPanicUA commented Jun 4, 2021 •
edited by github-actions bot
Loading

/test connector=destination-redshift

🕑 destination-redshift https://github.com/airbytehq/airbyte/actions/runs/905497914
✅ destination-redshift https://github.com/airbytehq/airbyte/actions/runs/905497914

@DoNotPanicUA
Copy link
Contributor Author

DoNotPanicUA commented Jun 4, 2021 •
edited by github-actions bot
Loading

/test connector=destination-snowflake

🕑 destination-snowflake https://github.com/airbytehq/airbyte/actions/runs/905498232
✅ destination-snowflake https://github.com/airbytehq/airbyte/actions/runs/905498232

@DoNotPanicUA
Copy link
Contributor Author

LGTM -- please publish and merge. Also, can you add this to the data type tests?

This case is added to the PR with tests. Corresponding commit.

@DoNotPanicUA
Copy link
Contributor Author

DoNotPanicUA commented Jun 4, 2021 •
edited by github-actions bot
Loading

/publish connector=connectors/source-mysql

🕑 connectors/source-mysql https://github.com/airbytehq/airbyte/actions/runs/905800611
✅ connectors/source-mysql https://github.com/airbytehq/airbyte/actions/runs/905800611

@DoNotPanicUA DoNotPanicUA merged commit 96bd949 into master Jun 4, 2021
@DoNotPanicUA DoNotPanicUA deleted the 3840-fix-mysql branch June 4, 2021 11:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sherifnada sherifnada sherifnada approved these changes

@masonwheeler masonwheeler masonwheeler approved these changes

@yaroslav-hrytsaienko yaroslav-hrytsaienko yaroslav-hrytsaienko approved these changes

@subodh1810 subodh1810 Awaiting requested review from subodh1810

@tuliren tuliren Awaiting requested review from tuliren

Assignees

@DoNotPanicUA DoNotPanicUA

Labels
connectors/destination/snowflake connectors/destinations-warehouse
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Source MySQL: unsigned numeric values larger than signed integer size break connector
5 participants
@DoNotPanicUA @sherifnada @masonwheeler @yaroslav-hrytsaienko @karinakuz