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

🐛Source-mssql: added support for missed data types #9094

Merged
merged 16 commits into from
Jan 7, 2022

Conversation

etsybaev
Copy link
Contributor

@etsybaev etsybaev commented Dec 23, 2021

What

We had lot's of types that were not handled properly by the Source-MsSql connector.

How

Updated to connector to properly process data type for a regular connection and for some of CDC connections.

The most of type are handled using different kind of native approaches.
But there is no support for the "hierarchyid" type even in the native SQL Server jdbc driver in DataTypes.
The DB stores it in it's own representation converted to HEX format.

Selection_306

The only way to get it as a text is to query it using internal function like this: "test_column.ToString() as test_column".
So in the final solution we make a pre-request for fetch a first row to get actual types and then wrap required fields with toString() function in eventual request: "test_column" -> "test_column.ToString() as test_column".

Also tested using UI for both full refresh and incremental sync types:
Full refresh:
Selection_346
![Selection_347](https://user-images.githubusercontent.com/4688354/148544339-e12fad8c-a792-403f-a5a7-94caa8a4f246.png
Selection_348
)
Selection_349

Incremental
Selection_351
:
Selection_352
Selection_353

Also checked for cases with empty tables:
Selection_354

So lots of different tests, no fails

Selection_355

Recommended reading order

  1. MssqlSource
  2. MssqlSourceOperations

🚨 User Impact 🚨

🚨🚨 As we not handle values properly, some of them may come represented in another format, ex. number<->string<->date\datetime\time.
So probably full refresh would be required.

Pre-merge Checklist

Expand the relevant checklist and delete the others.

New Connector

Community member or Airbyter

  • Community member? Grant edit access to maintainers (instructions)
  • Secrets in the connector's spec are annotated with airbyte_secret
  • Unit & integration tests added and passing. Community members, please provide proof of success locally e.g: screenshot or copy-paste unit, integration, and acceptance test output. To run acceptance tests for a Python connector, follow instructions in the README. For java connectors run ./gradlew :airbyte-integrations:connectors:<name>:integrationTest.
  • Code reviews completed
  • Documentation updated
    • Connector's README.md
    • Connector's bootstrap.md. See description and examples
    • docs/SUMMARY.md
    • docs/integrations/<source or destination>/<name>.md including changelog. See changelog example
    • docs/integrations/README.md
    • airbyte-integrations/builds.md
  • PR name follows PR naming conventions

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

  • Create a non-forked branch based on this PR and test the below items on it
  • Build is successful
  • Credentials added to Github CI. Instructions.
  • /test connector=connectors/<name> command is passing.
  • New Connector version released on Dockerhub by running the /publish command described here
  • After the connector is published, connector added to connector index as described here
  • Seed specs have been re-generated by building the platform and committing the changes to the seed spec files, as described here

Updating a connector

Community member or Airbyter

  • Grant edit access to maintainers (instructions)
  • Secrets in the connector's spec are annotated with airbyte_secret
  • Unit & integration tests added and passing. Community members, please provide proof of success locally e.g: screenshot or copy-paste unit, integration, and acceptance test output. To run acceptance tests for a Python connector, follow instructions in the README. For java connectors run ./gradlew :airbyte-integrations:connectors:<name>:integrationTest.
  • Code reviews completed
  • Documentation updated
    • Connector's README.md
    • Connector's bootstrap.md. See description and examples
    • Changelog updated in docs/integrations/<source or destination>/<name>.md including changelog. See changelog example
  • PR name follows PR naming conventions

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

  • Create a non-forked branch based on this PR and test the below items on it
  • Build is successful
  • Credentials added to Github CI. Instructions.
  • /test connector=connectors/<name> command is passing.
  • New Connector version released on Dockerhub by running the /publish command described here
  • After the new connector version is published, connector version bumped in the seed directory as described here
  • Seed specs have been re-generated by building the platform and committing the changes to the seed spec files, as described here

@github-actions github-actions bot added the area/connectors Connector related issues label Dec 23, 2021
@etsybaev etsybaev temporarily deployed to more-secrets December 23, 2021 20:59 Inactive
@etsybaev
Copy link
Contributor Author

etsybaev commented Dec 23, 2021

/test connector=connectors/source-mssql

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

@jrhizor jrhizor temporarily deployed to more-secrets December 23, 2021 21:01 Inactive
@etsybaev etsybaev temporarily deployed to more-secrets December 24, 2021 17:48 Inactive
@etsybaev etsybaev temporarily deployed to more-secrets December 24, 2021 18:12 Inactive
@etsybaev
Copy link
Contributor Author

etsybaev commented Dec 24, 2021

/test connector=connectors/source-mssql

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

@jrhizor jrhizor temporarily deployed to more-secrets December 24, 2021 18:41 Inactive
@etsybaev
Copy link
Contributor Author

etsybaev commented Dec 24, 2021

/test-performance connector=connectors/source-mssql

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

@jrhizor jrhizor temporarily deployed to more-secrets December 24, 2021 22:41 Inactive
@etsybaev etsybaev linked an issue Dec 26, 2021 that may be closed by this pull request
@alexandr-shegeda
Copy link
Contributor

kindly reminder that we also should bump the version for the source-mssql-strict-encrypt connector

@etsybaev
Copy link
Contributor Author

kindly reminder that we also should bump the version for the source-mssql-strict-encrypt connector

Sure, will do that along with changelog updating after got all approvals right before merging

@etsybaev etsybaev temporarily deployed to more-secrets December 27, 2021 14:06 Inactive
@etsybaev etsybaev changed the title [DRAFT_PR_IGNORE_IT] Source-mssql: added support for missed data types [DRAFT_PR] Source-mssql: added support for missed data types Dec 27, 2021
@etsybaev etsybaev changed the title [DRAFT_PR] Source-mssql: added support for missed data types Source-mssql: added support for missed data types Dec 27, 2021
@etsybaev
Copy link
Contributor Author

etsybaev commented Dec 27, 2021

/test connector=connectors/source-mssql

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

@jrhizor jrhizor temporarily deployed to more-secrets December 27, 2021 15:45 Inactive
@etsybaev
Copy link
Contributor Author

etsybaev commented Jan 6, 2022

/test connector=connectors/source-mssql

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

@etsybaev
Copy link
Contributor Author

etsybaev commented Jan 6, 2022

/test connector=connectors/source-mssql-strict-encrypt

🕑 connectors/source-mssql-strict-encrypt https://github.com/airbytehq/airbyte/actions/runs/1665186326
✅ connectors/source-mssql-strict-encrypt https://github.com/airbytehq/airbyte/actions/runs/1665186326
No Python unittests run

@etsybaev
Copy link
Contributor Author

etsybaev commented Jan 6, 2022

/test-performance connector=connectors/source-mssql

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

@jrhizor jrhizor temporarily deployed to more-secrets January 6, 2022 22:38 Inactive
@jrhizor jrhizor temporarily deployed to more-secrets January 6, 2022 22:38 Inactive
@jrhizor jrhizor temporarily deployed to more-secrets January 6, 2022 22:39 Inactive
@github-actions github-actions bot added the area/documentation Improvements or additions to documentation label Jan 7, 2022
@etsybaev etsybaev changed the title Source-mssql: added support for missed data types 🐛Source-mssql: added support for missed data types Jan 7, 2022
@etsybaev etsybaev temporarily deployed to more-secrets January 7, 2022 12:47 Inactive
@etsybaev etsybaev temporarily deployed to more-secrets January 7, 2022 14:02 Inactive
@etsybaev
Copy link
Contributor Author

etsybaev commented Jan 7, 2022

Currently, we have a known issue with build on CI, but locally all checks passed. So going to release new builds

@etsybaev
Copy link
Contributor Author

etsybaev commented Jan 7, 2022

/publish connector=connectors/source-mssql

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

@jrhizor jrhizor temporarily deployed to more-secrets January 7, 2022 15:20 Inactive
@jrhizor jrhizor temporarily deployed to more-secrets January 7, 2022 15:20 Inactive
@etsybaev
Copy link
Contributor Author

etsybaev commented Jan 7, 2022

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

🕑 connectors/source-mssql-strict-encrypt https://github.com/airbytehq/airbyte/actions/runs/1668352736
❌ connectors/source-mssql-strict-encrypt https://github.com/airbytehq/airbyte/actions/runs/1668352736

@jrhizor jrhizor temporarily deployed to more-secrets January 7, 2022 16:23 Inactive
@etsybaev
Copy link
Contributor Author

etsybaev commented Jan 7, 2022

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

🕑 connectors/source-mssql-strict-encrypt https://github.com/airbytehq/airbyte/actions/runs/1668377415
✅ connectors/source-mssql-strict-encrypt https://github.com/airbytehq/airbyte/actions/runs/1668377415

@etsybaev etsybaev temporarily deployed to more-secrets January 7, 2022 16:28 Inactive
@jrhizor jrhizor temporarily deployed to more-secrets January 7, 2022 16:31 Inactive
@etsybaev etsybaev temporarily deployed to more-secrets January 7, 2022 16:45 Inactive
@etsybaev etsybaev merged commit 1877e2a into master Jan 7, 2022
@etsybaev etsybaev deleted the etsybaev/7728-source-mssql-support-all-data-types branch January 7, 2022 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/connectors Connector related issues area/documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support all types in SQL Server source
5 participants