[Fix][Connector-v2] Add DateMilliConvertor to Convert DateMilliVector into Default Timezone by xiaochen-zhou · Pull Request #8736 · apache/seatunnel

xiaochen-zhou · 2025-02-14T21:03:44Z

Purpose of this pull request

Does this PR introduce any user-facing change?

The DateMilliVector type in Apache Arrow stores timestamps in UTC，Add DateMilliConvertor to Convert DateMilliVector into Default Timezone.

How was this patch tested?

exist tests

Check list

If any new Jar binary package adding in your PR, please add License Notice according
New License Guide
If necessary, please update the documentation to describe the new feature. https://github.com/apache/seatunnel/tree/dev/docs
If you are contributing the connector code, please check that the following files are updated:
1. Update plugin-mapping.properties and add new connector information in it
2. Update the pom file of seatunnel-dist
3. Add ci label in label-scope-conf
4. Add e2e testcase in seatunnel-e2e
5. Update connector plugin_config
Update the release-note.

Hisoka-X

Could you add a test case to cover it?

xiaochen-zhou · 2025-02-17T03:03:08Z

Could you add a test case to cover it?

There is already this test case: ArrowToSeatunnelRowReaderTest#testSeatunnelRow(). If this conversion is not added, it may cause this test case to fail, for example, before 8 a.m.

xiaochen-zhou · 2025-02-17T03:04:25Z

Could you add a test case to cover it?

Store Timestamp：1739564483396

Hisoka-X · 2025-02-17T03:09:33Z

Could you add a test case to cover it?

There is already this test case: ArrowToSeatunnelRowReaderTest#testSeatunnelRow(). If this conversion is not added, it may cause this test case to fail, for example, before 8 a.m.

I think we can use a fixed datetime (not use system time now()) to reproduce this bug.

Hisoka-X · 2025-02-17T03:10:37Z

If this conversion is not added, it may cause this test case to fail, for example, before 8 a.m.

Any test case failed link?

xiaochen-zhou · 2025-02-17T03:18:49Z

If this conversion is not added, it may cause this test case to fail, for example, before 8 a.m.

Any test case failed link?

No, I discovered this during local debugging. This test case is likely to reproduce the issue if run before 8 a.m.

xiaochen-zhou · 2025-02-17T03:19:03Z

Could you add a test case to cover it?

Store Timestamp：1739564483396

@Hisoka-X

Hisoka-X · 2025-02-17T03:32:41Z

If this conversion is not added, it may cause this test case to fail, for example, before 8 a.m.

Any test case failed link?

No, I discovered this during local debugging. This test case is likely to reproduce the issue if run before 8 a.m.

OK, please add test case.

xiaochen-zhou · 2025-02-17T03:57:03Z

Could you add a test case to cover it?

There is already this test case: ArrowToSeatunnelRowReaderTest#testSeatunnelRow(). If this conversion is not added, it may cause this test case to fail, for example, before 8 a.m.

I think we can use a fixed datetime (not use system time now()) to reproduce this bug.

Ok, I will refer to this to add the test case. @Hisoka-X

hawk9821 · 2025-02-17T07:15:24Z

     * on windows The test case uses TimeStampMicroVector to test the timestamp, thus truncating the
     * timestamp accuracy to ChronoUnit.MILLIS
     */
    private static final LocalDateTime localDateTime =


Suggested change

private static final LocalDateTime localDateTime =

private static final LocalDateTime localDateTime =

LocalDateTime.parse(

"2025-02-15 02:21:23", DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss"));

hawk9821 · 2025-02-17T07:18:33Z

@@ -172,13 +178,7 @@ private static VectorSchemaRoot buildVectorSchemaRoot(
        }
        // allocate storage
        vectors.forEach(FieldVector::allocateNew);


Suggested change

vectors.forEach(FieldVector::allocateNew);

vectors.forEach(FieldVector::allocateNew);

long epochMilli = localDateTime.atZone(zoneId).toInstant().toEpochMilli();

hawk9821 · 2025-02-17T07:19:41Z

                                    .setSafe(i, (stringValue).getBytes(StandardCharsets.UTF_8));
                        } else if (vector instanceof TimeStampMicroVector) {
-                            ((TimeStampMicroVector) vector).setSafe(i, epochMilli * 1000);
+                            ((TimeStampMicroVector) vector).setSafe(i, testEpochMilli);


Suggested change

((TimeStampMicroVector) vector).setSafe(i, testEpochMilli);

((TimeStampMicroVector) vector).setSafe(i, epochMilli * 1000);

hawk9821 · 2025-02-17T07:19:55Z

                                    .setSafe(i, (stringValue).getBytes(StandardCharsets.UTF_8));
                        } else if (vector instanceof TimeStampMilliTZVector) {
-                            ((TimeStampMilliTZVector) vector).setSafe(i, epochMilli);
+                            ((TimeStampMilliTZVector) vector).setSafe(i, testEpochMilli);


Suggested change

((TimeStampMilliTZVector) vector).setSafe(i, testEpochMilli);

((TimeStampMilliTZVector) vector).setSafe(i, epochMilli);

hawk9821 · 2025-02-17T07:21:33Z

+                            ((TimeStampMilliTZVector) vector).setSafe(i, testEpochMilli);
                        } else if (vector instanceof TimeMicroVector) {
-                            ((TimeMicroVector) vector).setSafe(i, epochMilli);
+                            ((TimeMicroVector) vector).setSafe(i, testEpochMilli);


hawk9821 · 2025-02-17T07:21:39Z

+                            ((TimeMicroVector) vector).setSafe(i, testEpochMilli);
                        } else if (vector instanceof DateMilliVector) {
-                            ((DateMilliVector) vector).setSafe(i, epochMilli);
+                            ((DateMilliVector) vector).setSafe(i, testEpochMilli);


hawk9821 · 2025-02-17T07:21:45Z

                                    List<LocalDateTime> dateTimeList = new ArrayList<>();
                                    for (int j = 0; j < 5; j++) {
-                                        writer.writeTimeStampMilliTZ(epochMilli);
+                                        writer.writeTimeStampMilliTZ(testEpochMilli);


xiaochen-zhou · 2025-02-18T02:15:05Z

@Hisoka-X @hawk9821 done.

hawk9821

LGTM. Thanks @xiaochen-zhou

… into Default Timezone (apache#8736)

xiaochen-zhou added 2 commits February 15, 2025 04:03

Fix ArrowToSeatunnelRowReader Date SqlType

a1965bf

add DateMilliConvertor

e1d01fa

github-actions Bot added the connectors-v2 label Feb 14, 2025

Fix code style

66ae659

xiaochen-zhou changed the title ~~[Fix][Zeta] Add DateMilliConvertor to Convert DateMilliVector into Default Timezone~~ [Fix][Connector-v2] Add DateMilliConvertor to Convert DateMilliVector into Default Timezone Feb 14, 2025

Hisoka-X reviewed Feb 17, 2025

View reviewed changes

update ArrowToSeatunnelRowReaderTest

360262b

hawk9821 reviewed Feb 17, 2025

View reviewed changes

update ArrowToSeatunnelRowReaderTest

cde927e

Hisoka-X requested a review from hawk9821 February 18, 2025 06:09

hawk9821 approved these changes Feb 18, 2025

View reviewed changes

github-actions Bot added the reviewed label Feb 18, 2025

Hisoka-X approved these changes Feb 18, 2025

View reviewed changes

github-actions Bot added the approved label Feb 18, 2025

hailin0 approved these changes Feb 18, 2025

View reviewed changes

hailin0 merged commit 7b8298a into apache:dev Feb 18, 2025

xiaochen-zhou deleted the arrow_localdatetime branch February 22, 2025 16:53

zax1314157 pushed a commit to zax1314157/seatunnel that referenced this pull request Feb 24, 2025

[Fix][Connector-v2] Add DateMilliConvertor to Convert DateMilliVector…

b410eb2

… into Default Timezone (apache#8736)

	vectors.forEach(FieldVector::allocateNew);
	vectors.forEach(FieldVector::allocateNew);
	long epochMilli = localDateTime.atZone(zoneId).toInstant().toEpochMilli();

	((TimeStampMicroVector) vector).setSafe(i, testEpochMilli);
	((TimeStampMicroVector) vector).setSafe(i, epochMilli * 1000);

	((TimeStampMilliTZVector) vector).setSafe(i, testEpochMilli);
	((TimeStampMilliTZVector) vector).setSafe(i, epochMilli);

Uh oh!

Conversation

xiaochen-zhou commented Feb 14, 2025

Purpose of this pull request

Does this PR introduce any user-facing change?

How was this patch tested?

Check list

Uh oh!

Hisoka-X left a comment

Choose a reason for hiding this comment

Uh oh!

xiaochen-zhou commented Feb 17, 2025

Uh oh!

xiaochen-zhou commented Feb 17, 2025

Uh oh!

Hisoka-X commented Feb 17, 2025

Uh oh!

Hisoka-X commented Feb 17, 2025

Uh oh!

xiaochen-zhou commented Feb 17, 2025

Uh oh!

xiaochen-zhou commented Feb 17, 2025

Uh oh!

Hisoka-X commented Feb 17, 2025

Uh oh!

xiaochen-zhou commented Feb 17, 2025

Uh oh!

hawk9821 Feb 17, 2025

Choose a reason for hiding this comment

Uh oh!

hawk9821 Feb 17, 2025

Choose a reason for hiding this comment

Uh oh!

hawk9821 Feb 17, 2025

Choose a reason for hiding this comment

Uh oh!

hawk9821 Feb 17, 2025

Choose a reason for hiding this comment

Uh oh!

hawk9821 Feb 17, 2025

Choose a reason for hiding this comment

Uh oh!

hawk9821 Feb 17, 2025

Choose a reason for hiding this comment

Uh oh!

hawk9821 Feb 17, 2025

Choose a reason for hiding this comment

Uh oh!

xiaochen-zhou commented Feb 18, 2025

Uh oh!

hawk9821 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants