Skip to content

[sqlserver] Fix timestamp type parse of sqlserver#2708

Merged
GOODBOY008 merged 1 commit intoapache:masterfrom
loserwang1024:fix-timestamp
Nov 22, 2023
Merged

[sqlserver] Fix timestamp type parse of sqlserver#2708
GOODBOY008 merged 1 commit intoapache:masterfrom
loserwang1024:fix-timestamp

Conversation

@loserwang1024
Copy link
Copy Markdown
Contributor

@loserwang1024 loserwang1024 commented Nov 16, 2023

Fix #2637

@loserwang1024 loserwang1024 changed the title Fix timestamp [cdc-base] Fix wrong timestamp value in some cdc source Nov 17, 2023
@loserwang1024
Copy link
Copy Markdown
Contributor Author

loserwang1024 commented Nov 17, 2023

Hi, @GOODBOY008 , I have modified how to parse timestam_ltz value in testTemporalTypesWithTimeZone.
However, in com.ververica.cdc.connectors.sqlserver.table.SqlServerTimezoneITCase#testTemporalTypesWithTimeZone, I am wondering why you get different TIMESTAMP_LTZ values(UTC time) in different timezone ?

Do you mean that TO_TIMESTAMP(val_datetimeoffset) ? A TIMESTAMP_LTZ value can be parsed to different TIMESTAMP values in in different timezone.

@loserwang1024 loserwang1024 changed the title [cdc-base] Fix wrong timestamp value in some cdc source [sqlserver] Fix timestamp type parse of sqlserver Nov 17, 2023
GOODBOY008
GOODBOY008 reviewed Nov 17, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@GOODBOY008 GOODBOY008 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@loserwang1024 Can you fix code format?

GOODBOY008
GOODBOY008 reviewed Nov 17, 2023
View reviewed changes
Comment thread ...-cdc/src/test/java/com/ververica/cdc/connectors/sqlserver/table/SqlServerTimezoneITCase.java Outdated
}

@Test
public void testTimeZone_ltz2ntz() throws Exception {
Copy link
Copy Markdown
Member

@GOODBOY008 GOODBOY008 Nov 17, 2023

Choose a reason for hiding this comment

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

Merge unit tests.

@loserwang1024 loserwang1024 force-pushed the fix-timestamp branch 8 times, most recently from 9b3b6f9 to 60e681f Compare November 21, 2023 01:58
@loserwang1024
Copy link
Copy Markdown
Contributor Author

loserwang1024 commented Nov 21, 2023

@GOODBOY008 ，

TimestampData.fromLocalDateTime(
                            LocalDateTime.ofInstant(instant, serverTimeZone));

@GOODBOY008 , LocalDateTime.ofInstant(instant, serverTimeZone)) use serverTimeZone, while TimestampData.fromLocalDateTime use JVM's default timezone. If this two timezones are not same, time will change.

And I design a test with different timezones:

By the way, MSSQL_SERVER_CONTAINER can not run on my m2 Macbook, it's hard to develop server-sql cdc.

GOODBOY008
GOODBOY008 approved these changes Nov 22, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@GOODBOY008 GOODBOY008 left a comment

Choose a reason for hiding this comment

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

@loserwang1024 Great catch , Thanks for your contribution. LGTM

@GOODBOY008 GOODBOY008 merged commit 4770f36 into apache:master Nov 22, 2023
loserwang1024 added a commit to loserwang1024/flink-cdc-connectors that referenced this pull request Nov 24, 2023
@loserwang1024
[sqlserver] Fix timestamp type parse of sqlserver (apache#2708)
a769c6f
e-mhui pushed a commit to e-mhui/flink-cdc-connectors that referenced this pull request Dec 2, 2023
@loserwang1024 @e-mhui
[sqlserver] Fix timestamp type parse of sqlserver (apache#2708)
7659c7a
ChaomingZhangCN pushed a commit to ChaomingZhangCN/flink-cdc that referenced this pull request Jan 13, 2025
@loserwang1024 @ChaomingZhangCN
[sqlserver] Fix timestamp type parse of sqlserver (apache#2708)
5960201
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@GOODBOY008 GOODBOY008 GOODBOY008 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug] Get wrong timestamp and timestamp_ltz value in some cdc source

2 participants

@loserwang1024 @GOODBOY008