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

[pipeline-connector][mysql] fix timestamp with timezone format #2952

Merged
merged 4 commits into from
Jan 10, 2024

Conversation

whhe
Copy link
Member

@whhe whhe commented Jan 2, 2024

Try to fix #2950

@Jiabao-Sun
Copy link
Contributor

Thanks @whhe for this contribution. It makes sense.
I'm a little curious why this question only appears in MySQL8.
By the way, could you please add some tests in MySqlFullTypesITCase ?

@Jiabao-Sun Jiabao-Sun self-requested a review January 2, 2024 14:31
@bingxindan
Copy link

On which version will this bug be fixed? I will download one and test it out

@whhe
Copy link
Member Author

whhe commented Jan 3, 2024

Thanks @whhe for this contribution. It makes sense. I'm a little curious why this question only appears in MySQL8. By the way, could you please add some tests in MySqlFullTypesITCase ?

Sure, I will have a try to add a full types test case for it soon.

For the issue cause, now the pipeline connector converts TIMESTAMP to TIMESTAMP_LTZ in MySqlTypeUtils.java#L194, while the original mysql source connector does not deal with TIMESTAMP WITH TIME ZONE, so it I think should only occur in e2e case.

@Jiabao-Sun
Copy link
Contributor

Thanks @whhe for this contribution. It makes sense. I'm a little curious why this question only appears in MySQL8. By the way, could you please add some tests in MySqlFullTypesITCase ?

Sure, I will have a try to add a full types test case for it soon.

For the issue cause, now the pipeline connector converts TIMESTAMP to TIMESTAMP_LTZ in MySqlTypeUtils.java#L194, while the original mysql source connector does not deal with TIMESTAMP WITH TIME ZONE, so it I think should only occur in e2e case.

Thanks @whhe.

https://github.com/ververica/flink-cdc-connectors/blob/1b643026fe93ed477541dcd7e97001785840a296/flink-cdc-connect/flink-cdc-pipeline-connectors/flink-cdc-pipeline-connector-mysql/src/test/resources/ddl/column_type_test_mysql8.sql#L108

Change it to timestamp_c TIMESTAMP(0) NULL DEFAULT '2000-01-01 00:00:00' can reproduce this problem.

@whhe
Copy link
Member Author

whhe commented Jan 3, 2024

Because MySqlValueConverters always uses UTC, so using Instant.parse is OK for other cases, here I only changed the parsing related to default value.

@whhe whhe changed the title [source-connector][debezium] fix timestamp with timezone format [pipeline-connector][mysql] fix timestamp with timezone format Jan 3, 2024
Copy link
Contributor

@Jiabao-Sun Jiabao-Sun left a comment

Choose a reason for hiding this comment

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

LGTM

@whhe whhe merged commit f3762a1 into apache:master Jan 10, 2024
17 checks passed
lvyanquan pushed a commit that referenced this pull request Jan 18, 2024
* fix ts with tz parser

* test timestamp with default value

* fix related test

* use timestamp string in test cases

(cherry picked from commit f3762a1)
joyCurry30 pushed a commit to joyCurry30/flink-cdc-connectors that referenced this pull request Mar 22, 2024
…e#2952)

* fix ts with tz parser

* test timestamp with default value

* fix related test

* use timestamp string in test cases
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants