🐛 Fixing rounding of numeric values for async destinations #31083

cynthiaxyin · 2023-10-04T22:10:49Z

Platform change (airbytehq/airbyte-platform-internal#9158)
Async destinations change (this PR)
Non-async destinations change (will be done separately)

The order (1) and (2) are merged doesn't matter. But both of them need to be merged before the problem is fixed (for async destinations).

Problem

Previously, numeric values with many digits after the decimal point (ex: 1884518522.1684267241) would get truncated in the _airbyte_data column in the raw table (ex: 1.884518522168427e+09). We should preserve full values in the raw table (after which numeric-typed columns may have precision restrictions per destination, ex: Snowflake has ~15 digits of precision so 1884518522.16843).

Solution

Deserialize the Airbyte record message string with DeserializationFeature.USE_BIG_DECIMAL_FOR_FLOATS to preserve all the decimals.

Testing

Tested with source Postgres to destination Snowflake. See old raw table, new raw table, and final table.

Reading

The main change is in AsyncStreamConsumer.java and Jsons.java, with an added unit test in AsyncStreamConsumerTest.java.

github-actions · 2023-10-04T22:11:20Z

Before Merging a Connector Pull Request

Wow! What a great pull request you have here! 🎉

To merge this PR, ensure the following has been done/considered for each connector added or updated:

PR name follows PR naming conventions
Breaking changes are considered. If a Breaking Change is being introduced, ensure an Airbyte engineer has created a Breaking Change Plan.
Connector version has been incremented in the Dockerfile and metadata.yaml according to our Semantic Versioning for Connectors guidelines
You've updated the connector's metadata.yaml file any other relevant changes, including a breakingChanges entry for major version bumps. See metadata.yaml docs
Secrets in the connector's spec are annotated with airbyte_secret
All documentation files are up to date. (README.md, bootstrap.md, docs.md, etc...)
Changelog updated in docs/integrations/<source or destination>/<name>.md with an entry for the new version. See changelog example
Migration guide updated in docs/integrations/<source or destination>/<name>-migrations.md with an entry for the new version, if the version is a breaking change. See migration guide example
If set, you've ensured the icon is present in the platform-internal repo. (Docs)

If the checklist is complete, but the CI check is failing,

Check for hidden checklists in your PR description
Toggle the github label checklist-action-run on/off to re-run the checklist CI.

vercel · 2023-10-04T22:11:55Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment

Name	Status	Preview	Comments	Updated (UTC)
airbyte-docs	⬜️ Ignored (Inspect)	Visit Preview		Oct 10, 2023 11:42pm

cynthiaxyin · 2023-10-09T23:38:03Z

airbyte-integrations/connectors/destination-bigquery/build.gradle

    features = ['db-destinations']
-    useLocalCdk = false
+    useLocalCdk = true


Please CMIIW, my understanding of the steps are:

Wait for PR approval

Slash publish the CDK

Toggle this line (and the same for the other 2 dests) back to false

Merge the PR like usual

looks right to me!

edgao

master cdk is already on 0.1.5 so you'll need to fix that conflict :(

we should keep an eye on this dashboard in case this causes a noticeable drop? but I'm not expecting anything egregious -

edgao · 2023-10-10T16:03:59Z

airbyte-integrations/connectors/destination-bigquery/build.gradle

    features = ['db-destinations']
-    useLocalCdk = false
+    useLocalCdk = true


looks right to me!

edgao · 2023-10-10T16:05:05Z

...ation-bigquery/src/test/java/io/airbyte/integrations/destination/bigquery/CdkImportTest.java

@@ -17,7 +17,7 @@ class CdkImportTest {
   */
  @Test
  void cdkVersionShouldMatch() {
-    assertEquals("0.1.4", CDKConstants.VERSION.replace("-SNAPSHOT", ""));
+    assertEquals("0.1.5", CDKConstants.VERSION.replace("-SNAPSHOT", ""));


@aaronsteers can we delete this test class? looks like it was something for cdk dev work

airbyte-oss-build-runner · 2023-10-10T23:15:05Z

destination-redshift test report (commit `645070c69d`) - ✅

⏲️ Total pipeline duration: 20mn37s

Step	Result
Build connector tar	✅
Build destination-redshift docker image for platform(s) linux/x86_64	✅
Java Connector Unit Tests	✅
Build airbyte/normalization-redshift:dev	✅
Java Connector Integration Tests	✅
Validate metadata for destination-redshift	✅
Connector version semver check	✅
QA checks	✅

🔗 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-redshift test

airbyte-oss-build-runner · 2023-10-10T23:18:41Z

destination-snowflake test report (commit `645070c69d`) - ✅

⏲️ Total pipeline duration: 03mn22s

Step	Result
Build connector tar	✅
Build destination-snowflake docker image for platform(s) linux/x86_64	✅
Java Connector Unit Tests	✅
Java Connector Integration Tests	✅
Validate metadata for destination-snowflake	✅
Connector version semver check	✅
Connector version increment check	✅
QA checks	✅

🔗 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

airbyte-oss-build-runner · 2023-10-10T23:22:15Z

destination-bigquery test report (commit `645070c69d`) - ✅

⏲️ Total pipeline duration: 03mn20s

Step	Result
Build connector tar	✅
Build destination-bigquery docker image for platform(s) linux/x86_64	✅
Java Connector Unit Tests	✅
Java Connector Integration Tests	✅
Validate metadata for destination-bigquery	✅
Connector version semver check	✅
Connector version increment check	✅
QA checks	✅

🔗 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

cynthiaxyin · 2023-10-10T23:23:12Z

/publish-java-cdk

🕑 https://github.com/airbytehq/airbyte/actions/runs/6475890644
✅ Successfully published Java CDK version=0.1.6!

airbyte-oss-build-runner · 2023-10-11T01:58:38Z

destination-redshift test report (commit `1059004942`) - ✅

⏲️ Total pipeline duration: 03mn32s

Step	Result
Build connector tar	✅
Build destination-redshift docker image for platform(s) linux/x86_64	✅
Java Connector Unit Tests	✅
Build airbyte/normalization-redshift:dev	✅
Java Connector Integration Tests	✅
Validate metadata for destination-redshift	✅
Connector version semver check	✅
QA checks	✅

🔗 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-redshift test

airbyte-oss-build-runner · 2023-10-11T02:02:07Z

destination-bigquery test report (commit `1059004942`) - ✅

⏲️ Total pipeline duration: 03mn10s

Step	Result
Build connector tar	✅
Build destination-bigquery docker image for platform(s) linux/x86_64	✅
Java Connector Unit Tests	✅
Java Connector Integration Tests	✅
Validate metadata for destination-bigquery	✅
Connector version semver check	✅
Connector version increment check	✅
QA checks	✅

🔗 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

airbyte-oss-build-runner · 2023-10-11T02:13:24Z

destination-snowflake test report (commit `1059004942`) - ✅

⏲️ Total pipeline duration: 11mn06s

Step	Result
Build connector tar	✅
Build destination-snowflake docker image for platform(s) linux/x86_64	✅
Java Connector Unit Tests	✅
Java Connector Integration Tests	✅
Validate metadata for destination-snowflake	✅
Connector version semver check	✅
Connector version increment check	✅
QA checks	✅

🔗 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

…#31083)

octavia-squidington-iii added area/connectors Connector related issues CDK Connector Development Kit connectors/destination/bigquery connectors/destination/snowflake connectors/destination/redshift labels Oct 4, 2023

octavia-squidington-iii added the area/documentation Improvements or additions to documentation label Oct 9, 2023

vercel bot deployed to Preview October 9, 2023 23:24 View deployment

cynthiaxyin force-pushed the cynthia/numeric_rounding branch from bef8169 to 27ac2ba Compare October 9, 2023 23:30

vercel bot deployed to Preview October 9, 2023 23:33 View deployment

cynthiaxyin force-pushed the cynthia/numeric_rounding branch from 27ac2ba to 89475c9 Compare October 9, 2023 23:35

cynthiaxyin marked this pull request as ready for review October 9, 2023 23:35

cynthiaxyin requested a review from a team as a code owner October 9, 2023 23:35

cynthiaxyin requested a review from benmoriceau October 9, 2023 23:36

vercel bot deployed to Preview October 9, 2023 23:36 View deployment

cynthiaxyin commented Oct 9, 2023

View reviewed changes

This comment was marked as outdated.

Sign in to view

edgao approved these changes Oct 10, 2023

View reviewed changes

cynthiaxyin added 3 commits October 10, 2023 14:12

fixing numeric precision

660ce71

changelog

e0ea429

fix BQ CdkImportTest

645070c

cynthiaxyin force-pushed the cynthia/numeric_rounding branch from 3c7bc10 to 645070c Compare October 10, 2023 21:20

This comment was marked as outdated.

Sign in to view

turn off useLocalCdk

1059004

cynthiaxyin enabled auto-merge (squash) October 10, 2023 23:43

This comment was marked as outdated.

Sign in to view

cynthiaxyin merged commit e3d126b into master Oct 11, 2023
23 checks passed

cynthiaxyin deleted the cynthia/numeric_rounding branch October 11, 2023 02:13

edgao pushed a commit that referenced this pull request Oct 11, 2023

🐛 Fixing rounding of numeric values for async destinations (#31083)

6641e3e

ariesgun pushed a commit to ariesgun/airbyte that referenced this pull request Oct 20, 2023

🐛 Fixing rounding of numeric values for async destinations (airbytehq…

65b7df0

…#31083)

ariesgun pushed a commit to ariesgun/airbyte that referenced this pull request Oct 23, 2023

🐛 Fixing rounding of numeric values for async destinations (airbytehq…

e58a113

…#31083)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🐛 Fixing rounding of numeric values for async destinations #31083

🐛 Fixing rounding of numeric values for async destinations #31083

cynthiaxyin commented Oct 4, 2023 •

edited

github-actions bot commented Oct 4, 2023 •

edited by cynthiaxyin

vercel bot commented Oct 4, 2023 •

edited

cynthiaxyin Oct 9, 2023

edgao Oct 10, 2023

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

edgao left a comment

edgao Oct 10, 2023

edgao Oct 10, 2023

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

airbyte-oss-build-runner commented Oct 10, 2023

airbyte-oss-build-runner commented Oct 10, 2023

airbyte-oss-build-runner commented Oct 10, 2023

cynthiaxyin commented Oct 10, 2023 •

edited by github-actions bot

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

airbyte-oss-build-runner commented Oct 11, 2023

airbyte-oss-build-runner commented Oct 11, 2023

airbyte-oss-build-runner commented Oct 11, 2023

🐛 Fixing rounding of numeric values for async destinations #31083

🐛 Fixing rounding of numeric values for async destinations #31083

Conversation

cynthiaxyin commented Oct 4, 2023 • edited

Problem

Solution

Testing

Reading

github-actions bot commented Oct 4, 2023 • edited by cynthiaxyin

Before Merging a Connector Pull Request

vercel bot commented Oct 4, 2023 • edited

cynthiaxyin Oct 9, 2023

Choose a reason for hiding this comment

edgao Oct 10, 2023

Choose a reason for hiding this comment

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

edgao left a comment

Choose a reason for hiding this comment

edgao Oct 10, 2023

Choose a reason for hiding this comment

edgao Oct 10, 2023

Choose a reason for hiding this comment

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

airbyte-oss-build-runner commented Oct 10, 2023

destination-redshift test report (commit 645070c69d) - ✅

airbyte-oss-build-runner commented Oct 10, 2023

destination-snowflake test report (commit 645070c69d) - ✅

airbyte-oss-build-runner commented Oct 10, 2023

destination-bigquery test report (commit 645070c69d) - ✅

cynthiaxyin commented Oct 10, 2023 • edited by github-actions bot

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

airbyte-oss-build-runner commented Oct 11, 2023

destination-redshift test report (commit 1059004942) - ✅

airbyte-oss-build-runner commented Oct 11, 2023

destination-bigquery test report (commit 1059004942) - ✅

airbyte-oss-build-runner commented Oct 11, 2023

destination-snowflake test report (commit 1059004942) - ✅

cynthiaxyin commented Oct 4, 2023 •

edited

github-actions bot commented Oct 4, 2023 •

edited by cynthiaxyin

vercel bot commented Oct 4, 2023 •

edited

destination-redshift test report (commit `645070c69d`) - ✅

destination-snowflake test report (commit `645070c69d`) - ✅

destination-bigquery test report (commit `645070c69d`) - ✅

cynthiaxyin commented Oct 10, 2023 •

edited by github-actions bot

destination-redshift test report (commit `1059004942`) - ✅

destination-bigquery test report (commit `1059004942`) - ✅

destination-snowflake test report (commit `1059004942`) - ✅