source-mysql: datatype fixes #49918

postamar · 2024-12-18T16:43:57Z

What

This PR tries to get source-mysql's datatypes correct by leveraging the improved MySqlDatatypeIntegrationTest.

How

n/a

Review guide

n/a

User Impact

Source-mysql will handle some datatypes differently:

very large DECIMAL values are now rendered properly without loss of precision
BIT(n) where n > 1 are now properly handled as byte arrays instead of as integers
FLOAT(?), FLOAT(?,?), DOUBLE(?), DOUBLE(?,?) might render very slightly differently

Can this PR be safely reverted and rolled back?

YES 💚
NO ❌

vercel · 2024-12-18T16:44:03Z

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

Name	Status	Preview	Comments	Updated (UTC)
airbyte-docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Dec 20, 2024 5:35pm

airbyte-cdk/bulk/core/base/src/main/kotlin/io/airbyte/cdk/util/Jsons.kt

.../bulk/core/extract/src/testFixtures/kotlin/io/airbyte/cdk/read/DynamicDatatypeTestFactory.kt

postamar · 2024-12-18T16:55:00Z

...lk/toolkits/extract-cdc/src/main/kotlin/io/airbyte/cdk/read/cdc/DebeziumPropertiesBuilder.kt

+                with("${key}.type", converterClass.canonicalName)
+            }
+        }
+    }


syntactic sugar for registering converters

airbyte-integrations/connectors/source-mysql/metadata.yaml

postamar · 2024-12-18T16:58:03Z

...s/source-mysql/src/main/kotlin/io/airbyte/integrations/source/mysql/MysqlSourceOperations.kt

            MysqlType.ENUM,
            MysqlType.SET -> StringFieldType
+            MysqlType.JSON -> JsonStringFieldType


were json values always rendered as strings? or is this a recent regression?

in old mysql's MySqlSourceOperations.copyToJsonField:

case … JSON, … -> putString(json, columnName, resultSet, colIndex);

I'm thinking that we should fix this in a different PR

postamar · 2024-12-18T16:58:15Z

...s/source-mysql/src/main/kotlin/io/airbyte/integrations/source/mysql/MysqlSourceOperations.kt

-            }
+            MysqlType.VECTOR,
+            MysqlType.UNKNOWN,
+            null -> PokemonFieldType


might as well list all values explicitly for safety

postamar · 2024-12-18T16:58:53Z

...ce-mysql/src/main/kotlin/io/airbyte/integrations/source/mysql/cdc/MySqlDebeziumOperations.kt

+                // This is to make sure that numbers are represented as strings.
+                .with("decimal.handling.mode", "string")
+                // This is to make sure that temporal data is represented without loss of precision.
+                .with("time.precision.mode", "adaptive_time_microseconds")


this is the default value for this setting, but I prefer to declare it explicitly

postamar · 2024-12-18T16:59:32Z

...ce-mysql/src/main/kotlin/io/airbyte/integrations/source/mysql/cdc/MySqlDebeziumOperations.kt

@@ -373,6 +391,10 @@ class MySqlDebeziumOperations(
                // This to make sure that binary data represented as a base64-encoded String.
                // https://debezium.io/documentation/reference/2.2/connectors/mysql.html#mysql-property-binary-handling-mode
                .with("binary.handling.mode", "base64")
+                // This is to make sure that numbers are represented as strings.
+                .with("decimal.handling.mode", "string")


this wraps large numbers in quotes, which will get turned into BigDecimal again by the snippet I added further above

postamar · 2024-12-18T17:02:49Z

...e-mysql/src/test/kotlin/io/airbyte/integrations/source/mysql/MySqlDatatypeIntegrationTest.kt

@@ -175,7 +174,8 @@ object MySqlDatatypeTestOperations :
    val dateTimeValues =
        mapOf(
            "'2024-09-13 14:30:00'" to """"2024-09-13T14:30:00.000000"""",
-            "'2024-09-13T14:40:00+00:00'" to """"2024-09-13T14:40:00.000000""""
+            "'2024-09-13T14:40:00+00:00'" to """"2024-09-13T14:40:00.000000"""",
+            "'1752-09-01 14:30:00'" to """"1752-09-01T14:30:00.000000"""",


I don't think I have enough edge cases in these temporal test cases.
In any case mysql doesn't do BC DATEs, and doesn't do pre-year-1000 DATETIMEs and doesn't do TIMESTAMPs before unix epoch 0.

stephane-airbyte · 2024-12-20T00:43:48Z

...-mysql/src/main/kotlin/io/airbyte/integrations/source/mysql/MySqlSourceDebeziumOperations.kt

+                    data.set<JsonNode>(field.id, Jsons.readTree(textNode.textValue()))
+                }
+                else -> continue
+            }


this feels like a bad abstraction here, the fact that we (the implementor of DebeziumOperations) have to iterate through the schema types. Just some food for thoughts

Yeah, it's not great. Perhaps Stream should have pre-computed groupings of Fields per Airbyte type. Or per FieldType. Not sure.

postamar · 2024-12-20T14:39:18Z

@rodireich I looked into the debezium-connector-mysql code and all of the weird edge cases in the legacy converter logic apply only to debezium snapshots, as far as I can tell, which we just don't do. When using debezium in regular, non-snapshot mode the Debezium engine stays on the happy path with no weird java types.

postamar · 2024-12-20T15:30:17Z

@rodireich actually there is one edge case which this PR doesn't handle correctly: default values! I'm working on adding test cases for these.

postamar · 2024-12-20T17:33:26Z

I added test cases for default values, which proved very useful.

octavia-squidington-iii added area/connectors Connector related issues CDK Connector Development Kit connectors/source/mysql area/documentation Improvements or additions to documentation labels Dec 18, 2024

postamar commented Dec 18, 2024

View reviewed changes

vercel bot deployed to Preview December 18, 2024 17:04 View deployment

postamar force-pushed the postamar/cute-debezium-converters branch from b464f37 to 2e13b9a Compare December 18, 2024 17:13

postamar changed the base branch from master to postamar/fix-json-deserialization-bug-in-cdk December 18, 2024 17:13

vercel bot deployed to Preview December 18, 2024 17:19 View deployment

postamar force-pushed the postamar/fix-json-deserialization-bug-in-cdk branch 2 times, most recently from 66a505d to 233cabf Compare December 18, 2024 18:32

postamar force-pushed the postamar/cute-debezium-converters branch 2 times, most recently from e1a82df to 4576060 Compare December 18, 2024 18:38

postamar changed the base branch from postamar/fix-json-deserialization-bug-in-cdk to master December 18, 2024 18:40

vercel bot deployed to Preview December 18, 2024 18:46 View deployment

postamar force-pushed the postamar/cute-debezium-converters branch from 4576060 to 7a9c03f Compare December 18, 2024 19:48

postamar changed the base branch from master to postamar/fix-json-deserialization-bug-in-cdk December 18, 2024 19:48

vercel bot deployed to Preview December 18, 2024 19:54 View deployment

postamar changed the base branch from postamar/fix-json-deserialization-bug-in-cdk to postamar/cdk-cute-debezium-converters December 18, 2024 19:54

postamar changed the title ~~bulk-cdk-extract,source-mysql: better CustomConverter implementations~~ source-mysql: datatype fixes Dec 18, 2024

postamar mentioned this pull request Dec 18, 2024

bulk-cdk-toolkits-extract-cdc: add CustomConverter utils #49923

Merged

2 tasks

postamar force-pushed the postamar/cute-debezium-converters branch from 7a9c03f to ebbbb9d Compare December 18, 2024 22:57

postamar force-pushed the postamar/cdk-cute-debezium-converters branch from fd09283 to 1805e08 Compare December 18, 2024 22:58

octavia-squidington-iii removed the area/documentation Improvements or additions to documentation label Dec 18, 2024

vercel bot deployed to Preview December 18, 2024 23:04 View deployment

postamar marked this pull request as ready for review December 19, 2024 12:09

postamar requested a review from a team as a code owner December 19, 2024 12:09

postamar mentioned this pull request Dec 19, 2024

source-mysql: normalize class names #49939

Merged

2 tasks

postamar marked this pull request as draft December 19, 2024 21:01

postamar force-pushed the postamar/cdk-cute-debezium-converters branch from 1805e08 to 3f30b8c Compare December 19, 2024 21:14

postamar force-pushed the postamar/cute-debezium-converters branch from ebbbb9d to 0aa2030 Compare December 19, 2024 21:21

octavia-squidington-iii added area/documentation Improvements or additions to documentation and removed CDK Connector Development Kit labels Dec 19, 2024

postamar force-pushed the postamar/cdk-cute-debezium-converters branch from 3f30b8c to 9818c9a Compare December 19, 2024 21:25

vercel bot deployed to Preview December 19, 2024 21:27 View deployment

octavia-squidington-iii added the CDK Connector Development Kit label Dec 19, 2024

vercel bot deployed to Preview December 19, 2024 21:39 View deployment

postamar marked this pull request as ready for review December 19, 2024 22:09

Base automatically changed from postamar/cdk-cute-debezium-converters to master December 20, 2024 00:27

vercel bot deployed to Preview December 20, 2024 00:29 View deployment

stephane-airbyte reviewed Dec 20, 2024

View reviewed changes

stephane-airbyte approved these changes Dec 20, 2024

View reviewed changes

postamar force-pushed the postamar/cute-debezium-converters branch from e81d3dc to 38639e5 Compare December 20, 2024 14:45

octavia-squidington-iii removed the CDK Connector Development Kit label Dec 20, 2024

vercel bot deployed to Preview December 20, 2024 14:51 View deployment

postamar mentioned this pull request Dec 20, 2024

bulk-cdk-toolkit-extract-cdc: fix default value handling in PartialConverter #49970

Merged

2 tasks

postamar added 3 commits December 20, 2024 12:06

source-mysql: fix datatype bugs

2a9ce44

add default value test cases

c4fa967

use CDK version 234

1ec2728

postamar force-pushed the postamar/cute-debezium-converters branch from 38639e5 to 1ec2728 Compare December 20, 2024 17:29

postamar enabled auto-merge (squash) December 20, 2024 17:33

vercel bot deployed to Preview December 20, 2024 17:35 View deployment

postamar merged commit 1d6b34c into master Dec 20, 2024
30 checks passed

postamar deleted the postamar/cute-debezium-converters branch December 20, 2024 17:44

rodireich mentioned this pull request Dec 30, 2024

Make sample query compatible with views #50437

Open

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

source-mysql: datatype fixes #49918

source-mysql: datatype fixes #49918

postamar commented Dec 18, 2024 •

edited

Loading

vercel bot commented Dec 18, 2024 •

edited

Loading

postamar Dec 18, 2024

postamar Dec 18, 2024

rodireich Dec 19, 2024 •

edited

Loading

postamar Dec 20, 2024

postamar Dec 20, 2024

postamar Dec 18, 2024

postamar Dec 18, 2024

postamar Dec 18, 2024

postamar Dec 18, 2024

stephane-airbyte Dec 20, 2024

postamar Dec 20, 2024

postamar commented Dec 20, 2024

postamar commented Dec 20, 2024

postamar commented Dec 20, 2024

source-mysql: datatype fixes #49918

source-mysql: datatype fixes #49918

Conversation

postamar commented Dec 18, 2024 • edited Loading

What

How

Review guide

User Impact

Can this PR be safely reverted and rolled back?

vercel bot commented Dec 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rodireich Dec 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

postamar commented Dec 20, 2024

postamar commented Dec 20, 2024

postamar commented Dec 20, 2024

postamar commented Dec 18, 2024 •

edited

Loading

vercel bot commented Dec 18, 2024 •

edited

Loading

rodireich Dec 19, 2024 •

edited

Loading