Skip to content

JDBC sources: Fix unexcepcted long Integer value failure #3846

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
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
@yaroslav-hrytsaienko yaroslav-hrytsaienko self-requested a review June 3, 2021 15:37
@DoNotPanicUA DoNotPanicUA marked this pull request as ready for review June 3, 2021 18:49
@auto-assign auto-assign bot requested review from masonwheeler and sherifnada 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

sherifnada commented Jun 3, 2021

@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

DoNotPanicUA commented Jun 4, 2021

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

Labels

connectors/destination/snowflake connectors/destinations-warehouse

6 participants

@DoNotPanicUA @sherifnada @masonwheeler @yaroslav-hrytsaienko @karinakuz