fix temporal data type handling in mysql source #15504

subodh1810 · 2022-08-10T10:13:56Z

🚨 User Impact 🚨

Are there any breaking changes? What is the end result perceived by the user? If yes, please merge this PR with the 🚨🚨 emoji so changelog authors can further highlight this if needed.

Pre-merge Checklist

Expand the relevant checklist and delete the others.

New Connector

Community member or Airbyter

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
If new credentials are required for use in CI, add them to GSM. 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
If new credentials are required for use in CI, add them to GSM. Instructions.
/test connector=connectors/<name> command is passing
New Connector version released on Dockerhub and connector version bumped by running the /publish command described here

Connector Generator

Issue acceptance criteria met
PR name follows PR naming conventions
If adding a new generator, add it to the list of scaffold modules being tested
The generator test modules (all connectors with -scaffold in their name) have been updated with the latest scaffold by running ./gradlew :airbyte-integrations:connector-templates:generator:testScaffoldTemplates then checking in your changes
Documentation which references the generator is updated as needed

Tests

Unit

Put your unit tests output here.

Integration

Put your integration tests output here.

Acceptance

Put your acceptance tests output here.

…logs

…x-temporal-type-handling-mysql

subodh1810 · 2022-08-10T15:11:42Z

/test connector=connectors/source-mysql

🕑 connectors/source-mysql https://github.com/airbytehq/airbyte/actions/runs/2833702737
❌ connectors/source-mysql https://github.com/airbytehq/airbyte/actions/runs/2833702737
🐛 https://gradle.com/s/z66bdvmtjyabg

Build Failed

Test summary info:

Could not find result summary

# Conflicts: # airbyte-db/db-lib/src/main/java/io/airbyte/db/jdbc/DateTimeConverter.java

subodh1810 · 2022-08-10T16:53:40Z

/test connector=connectors/source-mysql

subodh1810 · 2022-08-10T17:14:17Z

/test connector=connectors/source-mysql

🕑 connectors/source-mysql https://github.com/airbytehq/airbyte/actions/runs/2834441705
❌ connectors/source-mysql https://github.com/airbytehq/airbyte/actions/runs/2834441705
🐛

Build Failed

Test summary info:

Could not find result summary

subodh1810 · 2022-08-10T17:42:26Z

/test connector=connectors/source-mysql

subodh1810 · 2022-08-10T19:24:15Z

/test connector=connectors/source-mysql

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

Build Passed

Test summary info:

All Passed

subodh1810 · 2022-08-10T19:52:07Z

@edgao can you confirm this will also cover the scope of the issue (I think it would but would be nice to get your approval as well)

edgao

yep, it covers all the data types for that issue (date/datetime/timestamp)

added a few comments, but nothing relevant to the actual code change

edgao · 2022-08-11T03:41:52Z

...s/source-mysql/src/main/java/io/airbyte/integrations/source/mysql/MySqlSourceOperations.java

+    try {
+      preparedStatement.setObject(parameterIndex, LocalDateTime.parse(value));
+    } catch (final DateTimeParseException e) {
+      // attempt to parse the datetime with timezone. This can be caused by schema created with an older


are there test cases for this behavior?

added tests for the behaviour

edgao · 2022-08-11T03:43:06Z

...t-integration/java/io/airbyte/integrations/source/mysql/AbstractMySqlSourceDatatypeTest.java

+            .sourceType("timestamp")
+            .airbyteType(JsonSchemaType.STRING_TIMESTAMP_WITH_TIMEZONE)
+            .addInsertValues("null", "'2021-01-00'", "'2021-00-00'", "'0000-00-00'", "'2022-08-09T10:17:16.161342Z'")
+            .addExpectedValues(null, null, null, null, "2022-08-09T10:17:16.000000Z")


why are these returning null? is it because the insert values aren't valid timestamps?

edgao · 2022-08-11T03:45:39Z

...integration/java/io/airbyte/integrations/source/mysql/CdcBinlogsMySqlSourceDatatypeTest.java

+    container.start();
+
+    config = Jsons.jsonNode(ImmutableMap.builder()
+        .put(JdbcUtils.HOST_KEY, container.getHost())


prefer HostPortResolver.resolveHost() and resolvePort() for the host/portname - otherwise this breaks on m1 macs (maybe? at the very least, george and I had issues with postgres tests which were fixed by switching to HostPortResolver)

(iirc the DSLContextFactory still needs to use container.getHost/getPort because that's running directly on the host laptop, not inside a container)

edgao · 2022-08-11T03:47:56Z

...integration/java/io/airbyte/integrations/source/mysql/CdcBinlogsMySqlSourceDatatypeTest.java

+            .sourceType("timestamp")
+            .airbyteType(JsonSchemaType.STRING_TIMESTAMP_WITH_TIMEZONE)
+            .addInsertValues("null", "'2021-01-00'", "'2021-00-00'", "'0000-00-00'", "'2022-08-09T10:17:16.161342Z'")
+            .addExpectedValues(null, "1970-01-01T00:00:00.000000Z", "1970-01-01T00:00:00.000000Z", "1970-01-01T00:00:00.000000Z",


is 1970-01-01 the equivalent of the null entries in the default test cases? (adding this as a comment would be helpful)

subodh1810 · 2022-08-12T10:25:23Z

/test connector=connectors/source-mysql

🕑 connectors/source-mysql https://github.com/airbytehq/airbyte/actions/runs/2846108958
❌ connectors/source-mysql https://github.com/airbytehq/airbyte/actions/runs/2846108958
🐛 https://gradle.com/s/palwm6uq3fxva

Build Failed

Test summary info:

Could not find result summary

subodh1810 · 2022-08-12T13:09:56Z

/test connector=connectors/source-mysql

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

Build Passed

Test summary info:

All Passed

subodh1810 and others added 4 commits August 10, 2022 01:41

fix postgres data handling from WAL logs in CDC mode

2ec6694

format

98ad23b

fix temporal type data handling from in mysql source

8c00ce2

use formatter for dates also (#15485)

49ae556

subodh1810 self-assigned this Aug 10, 2022

github-actions bot added the area/connectors Connector related issues label Aug 10, 2022

subodh1810 changed the base branch from master to fix-postgres-data-handling-in-transaction-logs August 10, 2022 10:14

subodh1810 added 3 commits August 10, 2022 15:46

Merge branch 'master' into fix-postgres-data-handling-in-transaction-…

c830796

…logs

Merge branch 'fix-postgres-data-handling-in-transaction-logs' into fi…

93be124

…x-temporal-type-handling-mysql

format

f445d27

subodh1810 temporarily deployed to more-secrets August 10, 2022 10:32 Inactive

Merge branch 'fix-postgres-data-handling-in-transaction-logs' into fi…

2f3d32d

…x-temporal-type-handling-mysql

subodh1810 temporarily deployed to more-secrets August 10, 2022 10:34 Inactive

subodh1810 added 4 commits August 10, 2022 19:58

tweak tests

047f35b

change test structure

6e94507

Merge branch 'fix-postgres-data-handling-in-transaction-logs' into fi…

a1e557f

…x-temporal-type-handling-mysql

add test for data read from binlogs

79e6806

subodh1810 temporarily deployed to more-secrets August 10, 2022 14:46 Inactive

fix json schema types for data types

0a5f0c9

subodh1810 changed the title ~~Fix temporal type handling mysql~~ fix temporal data type handling in mysql source Aug 10, 2022

subodh1810 temporarily deployed to more-secrets August 10, 2022 15:13 Inactive

Base automatically changed from fix-postgres-data-handling-in-transaction-logs to master August 10, 2022 16:18

subodh1810 added 2 commits August 10, 2022 21:53

Merge branch 'master' into fix-temporal-type-handling-mysql

41aa283

# Conflicts: # airbyte-db/db-lib/src/main/java/io/airbyte/db/jdbc/DateTimeConverter.java

handle cursor parsing

e7d618d

octavia-squidington-iii added the connectors/source/mysql label Aug 10, 2022

subodh1810 marked this pull request as ready for review August 10, 2022 19:46

subodh1810 requested a review from a team as a code owner August 10, 2022 19:46

subodh1810 requested review from edgao and tuliren August 10, 2022 19:48

subodh1810 linked an issue Aug 10, 2022 that may be closed by this pull request

Source MySQL should correctly map temporal types to JsonSchema #15381

Closed

edgao approved these changes Aug 11, 2022

View reviewed changes

subodh1810 added 2 commits August 12, 2022 13:19

Merge branch 'master' into fix-temporal-type-handling-mysql

4184aa5

add more tests

07d7e76

remove assertion cause its not useful

2b5bc72

subodh1810 merged commit 7a7e78f into master Aug 12, 2022

subodh1810 deleted the fix-temporal-type-handling-mysql branch August 12, 2022 14:14

andriikorotkov mentioned this pull request Aug 15, 2022

🎉 Source MySql: Added SSL certificates to MySql Source #15044

Merged

37 tasks

tuliren mentioned this pull request Aug 17, 2022

🐛DATETIME from MySQL not returned as TIMESTAMP on postgres. It returns as text #3865

Closed

octavia-squidington-iii mentioned this pull request Aug 19, 2022

Bump Airbyte version from 0.40.0-alpha to 0.40.1-alpha #15775

Closed

ryankfu mentioned this pull request Aug 22, 2022

java.lang.RuntimeException: java.time.format.DateTimeParseException: Text '0' could not be parsed at index 0 #15840

Closed

evantahler mentioned this pull request Aug 22, 2022

Bump Airbyte version from 0.40.0-alpha to 0.40.1 #15857

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix temporal data type handling in mysql source #15504

fix temporal data type handling in mysql source #15504

subodh1810 commented Aug 10, 2022 •

edited

Loading

subodh1810 commented Aug 10, 2022 •

edited by github-actions bot

Loading

subodh1810 commented Aug 10, 2022

subodh1810 commented Aug 10, 2022 •

edited by github-actions bot

Loading

subodh1810 commented Aug 10, 2022

subodh1810 commented Aug 10, 2022 •

edited by github-actions bot

Loading

subodh1810 commented Aug 10, 2022

edgao left a comment

edgao Aug 11, 2022

subodh1810 Aug 12, 2022

edgao Aug 11, 2022

subodh1810 Aug 12, 2022

edgao Aug 11, 2022

edgao Aug 11, 2022

subodh1810 commented Aug 12, 2022 •

edited by github-actions bot

Loading

subodh1810 commented Aug 12, 2022 •

edited by github-actions bot

Loading

fix temporal data type handling in mysql source #15504

fix temporal data type handling in mysql source #15504

Conversation

subodh1810 commented Aug 10, 2022 • edited Loading

🚨 User Impact 🚨

Pre-merge Checklist

Community member or Airbyter

Airbyter

Community member or Airbyter

Airbyter

Tests

subodh1810 commented Aug 10, 2022 • edited by github-actions bot Loading

Build Failed

subodh1810 commented Aug 10, 2022

subodh1810 commented Aug 10, 2022 • edited by github-actions bot Loading

Build Failed

subodh1810 commented Aug 10, 2022

subodh1810 commented Aug 10, 2022 • edited by github-actions bot Loading

Build Passed

subodh1810 commented Aug 10, 2022

edgao left a comment

Choose a reason for hiding this comment

edgao Aug 11, 2022

Choose a reason for hiding this comment

subodh1810 Aug 12, 2022

Choose a reason for hiding this comment

edgao Aug 11, 2022

Choose a reason for hiding this comment

subodh1810 Aug 12, 2022

Choose a reason for hiding this comment

edgao Aug 11, 2022

Choose a reason for hiding this comment

edgao Aug 11, 2022

Choose a reason for hiding this comment

subodh1810 commented Aug 12, 2022 • edited by github-actions bot Loading

Build Failed

subodh1810 commented Aug 12, 2022 • edited by github-actions bot Loading

Build Passed

subodh1810 commented Aug 10, 2022 •

edited

Loading

subodh1810 commented Aug 10, 2022 •

edited by github-actions bot

Loading

subodh1810 commented Aug 10, 2022 •

edited by github-actions bot

Loading

subodh1810 commented Aug 10, 2022 •

edited by github-actions bot

Loading

subodh1810 commented Aug 12, 2022 •

edited by github-actions bot

Loading

subodh1810 commented Aug 12, 2022 •

edited by github-actions bot

Loading