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

Destination snowflake: mostly done implementations for sqlgenerator+destinationhandler #28677

Merged
merged 108 commits into from
Aug 7, 2023

Conversation

edgao
Copy link
Contributor

@edgao edgao commented Jul 25, 2023

SnowflakeSqlGeneratorIntegrationTest is now fully passing. There's a few things that still need to happen before we can release to beta. I'll do those in followup PRs / submit more issues for these:

  • enabling the T+D tests + getting the e2e flow working
  • specific type mappings - compare these with legacy normalization (Destination Snowflake v2: doublecheck type mapping #29072)
  • adding support for more timezone formats when parsing timestamptz (and time_with_timezone?)
  • add a test class + GSM secret to verify that we can override the raw table schema
  • figure out snowflake clustering - should we even do this? (https://github.com/airbytehq/airbyte-internal-issues/issues/2022))
  • identifier transformations (handling reserved words, special characters, etc) - we might be OK since we're quoting everything, but we should confirm
  • disable 1s1t on gcs/s3 staging

@edgao edgao requested a review from a team as a code owner July 25, 2023 17:00
return switch (airbyteProtocolType) {
case STRING -> "VARCHAR";
case NUMBER -> "FLOAT";
case INTEGER -> "NUMBER(38,0)";
Copy link
Contributor

@evantahler evantahler Aug 4, 2023

Choose a reason for hiding this comment

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

Note for the future: INT in snowflake is the same as NUMBER, which is the same as NUMBER(38,0) - https://docs.snowflake.com/en/sql-reference/data-types-numeric#int-integer-bigint-smallint-tinyint-byteint

nit: It might be better to write BIGINT, and always pass this alais down to snowflake

Copy link
Contributor Author

Choose a reason for hiding this comment

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

looks like NUMBER is snowflake's internal name (i.e. what you get from querying information_schema.columns), using that everywhere 🤷

@edgao edgao changed the title Destination snowflake: add some basic implementations for sqlgenerator+destinationhandler Destination snowflake: mostly done implementations for sqlgenerator+destinationhandler Aug 4, 2023
@edgao edgao marked this pull request as ready for review August 4, 2023 23:23
@edgao edgao requested a review from a team as a code owner August 4, 2023 23:23
@octavia-squidington-iii octavia-squidington-iii added the area/documentation Improvements or additions to documentation label Aug 4, 2023
@edgao
Copy link
Contributor Author

edgao commented Aug 4, 2023

(feel free to rereview, otherwise I'll merge on monday)

@octavia-squidington-iii
Copy link
Collaborator

destination-bigquery test report (commit 95cb31008d) - ✅

⏲️ Total pipeline duration: 32mn48s

Step Result
Validate airbyte-integrations/connectors/destination-bigquery/metadata.yaml
Connector version semver check
QA checks
Build connector tar
Build destination-bigquery docker image for platform linux/x86_64
Build airbyte/normalization:dev
./gradlew :airbyte-integrations:connectors:destination-bigquery:integrationTest

🔗 View the logs here

☁️ View runs for commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=destination-bigquery test

@octavia-squidington-iii
Copy link
Collaborator

destination-snowflake test report (commit 95cb31008d) - ✅

⏲️ Total pipeline duration: 42mn54s

Step Result
Validate airbyte-integrations/connectors/destination-snowflake/metadata.yaml
Connector version semver check
Connector version increment check
QA checks
Build connector tar
Build destination-snowflake docker image for platform linux/x86_64
Build airbyte/normalization-snowflake:dev
./gradlew :airbyte-integrations:connectors:destination-snowflake:integrationTest

🔗 View the logs here

☁️ View runs for commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=destination-snowflake test

@octavia-squidington-iii
Copy link
Collaborator

destination-bigquery test report (commit 95cb31008d) - ✅

⏲️ Total pipeline duration: 04mn02s

Step Result
Validate airbyte-integrations/connectors/destination-bigquery/metadata.yaml
Connector version semver check
QA checks
Build connector tar
Build destination-bigquery docker image for platform linux/x86_64
Build airbyte/normalization:dev
./gradlew :airbyte-integrations:connectors:destination-bigquery:integrationTest

🔗 View the logs here

☁️ View runs for commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=destination-bigquery test

@octavia-squidington-iii
Copy link
Collaborator

destination-snowflake test report (commit 95cb31008d) - ✅

⏲️ Total pipeline duration: 42mn13s

Step Result
Validate airbyte-integrations/connectors/destination-snowflake/metadata.yaml
Connector version semver check
Connector version increment check
QA checks
Build connector tar
Build destination-snowflake docker image for platform linux/x86_64
Build airbyte/normalization-snowflake:dev
./gradlew :airbyte-integrations:connectors:destination-snowflake:integrationTest

🔗 View the logs here

☁️ View runs for commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=destination-snowflake test

@edgao edgao merged commit 2866ed6 into master Aug 7, 2023
22 of 25 checks passed
@edgao edgao deleted the edgao/1s1t/snowflake_impl branch August 7, 2023 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants