You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@seatunnel.apache.org by GitBox <gi...@apache.org> on 2022/09/04 06:51:10 UTC
[GitHub] [incubator-seatunnel] laglangyue opened a new pull request, #2637: [hotfix] fix data,time,timestamp convert for spark
laglangyue opened a new pull request, #2637:
URL: https://github.com/apache/incubator-seatunnel/pull/2637
<!--
Thank you for contributing to SeaTunnel! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
## Contribution Checklist
- Make sure that the pull request corresponds to a [GITHUB issue](https://github.com/apache/incubator-seatunnel/issues).
- Name the pull request in the form "[Feature] [component] Title of the pull request", where *Feature* can be replaced by `Hotfix`, `Bug`, etc.
- Minor fixes should be named following this pattern: `[hotfix] [docs] Fix typo in README.md doc`.
-->
## Purpose of this pull request
fix data,time,timestamp convert for spark
I chose the UTC zone for timestamp，
time can be epochDay +time =timestamp,value is long
date is epochDay , value is int
time is timestamp , value is long
## Check list
* [ ] Code changed are covered with tests, or it does not need tests for reason:
* [ ] If any new Jar binary package adding in your PR, please add License Notice according
[New License Guide](https://github.com/apache/incubator-seatunnel/blob/dev/docs/en/contribution/new-license.md)
* [ ] If necessary, please update the documentation to describe the new feature. https://github.com/apache/incubator-seatunnel/tree/dev/docs
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] hailin0 commented on pull request #2637: [Bug][Translation] fix data,time,timestamp convert for spark
Posted by GitBox <gi...@apache.org>.
hailin0 commented on PR #2637:
URL: https://github.com/apache/incubator-seatunnel/pull/2637#issuecomment-1238841381
LGTM
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] laglangyue commented on a diff in pull request #2637: [Bug][Translation] fix data,time,timestamp convert for spark
Posted by GitBox <gi...@apache.org>.
laglangyue commented on code in PR #2637:
URL: https://github.com/apache/incubator-seatunnel/pull/2637#discussion_r962475410
##########
seatunnel-translation/seatunnel-translation-spark/seatunnel-translation-spark-common/src/main/java/org/apache/seatunnel/translation/spark/common/serialization/InternalRowConverter.java:
##########
@@ -164,11 +168,11 @@ private static Object reconvert(Object field, SeaTunnelDataType<?> dataType) {
case ROW:
return reconvert((InternalRow) field, (SeaTunnelRowType) dataType);
case DATE:
- return ((Date) field).toLocalDate();
+ return LocalDate.ofEpochDay((int) field);
Review Comment:
> how about like this? `(int) ((Date.valueOf(String.valueOf(val))).toLocalDate().toEpochDay());`
the date value in seatunal is LocalDate,we don't need to transform like `obj->string->date->localdate`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] ashulin commented on pull request #2637: [Bug][Translation] fix data,time,timestamp convert for spark
Posted by GitBox <gi...@apache.org>.
ashulin commented on PR #2637:
URL: https://github.com/apache/incubator-seatunnel/pull/2637#issuecomment-1238829363
ZonedDateTime should be solved by another PR as an improvement.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] laglangyue commented on pull request #2637: [Bug][Translation] fix data,time,timestamp convert for spark
Posted by GitBox <gi...@apache.org>.
laglangyue commented on PR #2637:
URL: https://github.com/apache/incubator-seatunnel/pull/2637#issuecomment-1242633993
@ashulin Please review again, I don't konw why CI failed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] laglangyue commented on pull request #2637: [Bug][Translation] fix data,time,timestamp convert for spark
Posted by GitBox <gi...@apache.org>.
laglangyue commented on PR #2637:
URL: https://github.com/apache/incubator-seatunnel/pull/2637#issuecomment-1243289526
I accidentally deleted my branch, but it was restored through PR
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] ashulin merged pull request #2637: [Bug][Translation] fix data,time,timestamp convert for spark
Posted by GitBox <gi...@apache.org>.
ashulin merged PR #2637:
URL: https://github.com/apache/incubator-seatunnel/pull/2637
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] laglangyue closed pull request #2637: [Bug][Translation] fix data,time,timestamp convert for spark
Posted by GitBox <gi...@apache.org>.
laglangyue closed pull request #2637: [Bug][Translation] fix data,time,timestamp convert for spark
URL: https://github.com/apache/incubator-seatunnel/pull/2637
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] laglangyue commented on pull request #2637: [Bug][Translation] fix data,time,timestamp convert for spark
Posted by GitBox <gi...@apache.org>.
laglangyue commented on PR #2637:
URL: https://github.com/apache/incubator-seatunnel/pull/2637#issuecomment-1236801818
I will fix it so that we can get 1/1000_000 of secend
---Original---
From: ***@***.***>
Date: Mon, Sep 5, 2022 10:59 AM
To: ***@***.***>;
Cc: ***@***.******@***.***>;
Subject: Re: [apache/incubator-seatunnel] [Bug][Translation] fixdata,time,timestamp convert for spark (PR #2637)
@liugddx commented on this pull request.
In seatunnel-translation/seatunnel-translation-spark/seatunnel-translation-spark-common/src/main/java/org/apache/seatunnel/translation/spark/common/serialization/InternalRowConverter.java:
> case TIMESTAMP: - return ((Timestamp) field).toLocalDateTime(); + return LocalDateTime.ofInstant(Instant.ofEpochMilli((long) field), ZONE_UTC);
Can the accuracy of TimesStamp accurately be precise?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you authored the thread.Message ID: ***@***.***>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] laglangyue commented on a diff in pull request #2637: [Bug][Translation] fix data,time,timestamp convert for spark
Posted by GitBox <gi...@apache.org>.
laglangyue commented on code in PR #2637:
URL: https://github.com/apache/incubator-seatunnel/pull/2637#discussion_r962466539
##########
seatunnel-translation/seatunnel-translation-spark/seatunnel-translation-spark-common/src/main/java/org/apache/seatunnel/translation/spark/common/serialization/InternalRowConverter.java:
##########
@@ -164,11 +168,11 @@ private static Object reconvert(Object field, SeaTunnelDataType<?> dataType) {
case ROW:
return reconvert((InternalRow) field, (SeaTunnelRowType) dataType);
case DATE:
- return ((Date) field).toLocalDate();
+ return LocalDate.ofEpochDay((int) field);
Review Comment:
> (int) ((Date.valueOf(String.valueOf(val))).toLocalDate().toEpochDay());
the value is int in sparkRow, and transfrom it to LocalDate.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] liugddx commented on a diff in pull request #2637: [Bug][Translation] fix data,time,timestamp convert for spark
Posted by GitBox <gi...@apache.org>.
liugddx commented on code in PR #2637:
URL: https://github.com/apache/incubator-seatunnel/pull/2637#discussion_r962440330
##########
seatunnel-translation/seatunnel-translation-spark/seatunnel-translation-spark-common/src/main/java/org/apache/seatunnel/translation/spark/common/serialization/InternalRowConverter.java:
##########
@@ -164,11 +168,11 @@ private static Object reconvert(Object field, SeaTunnelDataType<?> dataType) {
case ROW:
return reconvert((InternalRow) field, (SeaTunnelRowType) dataType);
case DATE:
- return ((Date) field).toLocalDate();
+ return LocalDate.ofEpochDay((int) field);
case TIME:
- return ((Timestamp) field).toLocalDateTime().toLocalTime();
+ return LocalDateTime.ofInstant(Instant.ofEpochMilli((long) field), ZONE_UTC).toLocalTime();
case TIMESTAMP:
- return ((Timestamp) field).toLocalDateTime();
+ return LocalDateTime.ofInstant(Instant.ofEpochMilli((long) field), ZONE_UTC);
Review Comment:
Can the accuracy of TimesStamp accurately be precise?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] laglangyue commented on pull request #2637: [Bug][Translation] fix data,time,timestamp convert for spark
Posted by GitBox <gi...@apache.org>.
laglangyue commented on PR #2637:
URL: https://github.com/apache/incubator-seatunnel/pull/2637#issuecomment-1237225808
> LGTM
wait a time
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] TyrantLucifer commented on a diff in pull request #2637: [Bug][Translation] fix data,time,timestamp convert for spark
Posted by GitBox <gi...@apache.org>.
TyrantLucifer commented on code in PR #2637:
URL: https://github.com/apache/incubator-seatunnel/pull/2637#discussion_r962324425
##########
seatunnel-translation/seatunnel-translation-spark/seatunnel-translation-spark-common/src/main/java/org/apache/seatunnel/translation/spark/common/serialization/InternalRowConverter.java:
##########
@@ -74,13 +76,12 @@ private static Object convert(Object field, SeaTunnelDataType<?> dataType) {
SeaTunnelRowType rowType = (SeaTunnelRowType) dataType;
return convert(seaTunnelRow, rowType);
case DATE:
- return Date.valueOf((LocalDate) field);
+ return (int) ((LocalDate) field).toEpochDay();
case TIME:
- //TODO: how reconvert?
LocalTime localTime = (LocalTime) field;
- return Timestamp.valueOf(LocalDateTime.of(LocalDate.ofEpochDay(0), localTime));
+ return (LocalDateTime.of(LocalDate.ofEpochDay(0), localTime).toInstant(ZONE_UTC)).toEpochMilli();
Review Comment:
The same as above.
##########
seatunnel-translation/seatunnel-translation-spark/seatunnel-translation-spark-common/src/main/java/org/apache/seatunnel/translation/spark/common/serialization/InternalRowConverter.java:
##########
@@ -164,11 +168,11 @@ private static Object reconvert(Object field, SeaTunnelDataType<?> dataType) {
case ROW:
return reconvert((InternalRow) field, (SeaTunnelRowType) dataType);
case DATE:
- return ((Date) field).toLocalDate();
+ return LocalDate.ofEpochDay((int) field);
case TIME:
- return ((Timestamp) field).toLocalDateTime().toLocalTime();
+ return LocalDateTime.ofInstant(Instant.ofEpochMilli((long) field), ZONE_UTC).toLocalTime();
case TIMESTAMP:
- return ((Timestamp) field).toLocalDateTime();
+ return LocalDateTime.ofInstant(Instant.ofEpochMilli((long) field), ZONE_UTC);
Review Comment:
The same as above.
##########
seatunnel-translation/seatunnel-translation-spark/seatunnel-translation-spark-common/src/main/java/org/apache/seatunnel/translation/spark/common/serialization/InternalRowConverter.java:
##########
@@ -164,11 +168,11 @@ private static Object reconvert(Object field, SeaTunnelDataType<?> dataType) {
case ROW:
return reconvert((InternalRow) field, (SeaTunnelRowType) dataType);
case DATE:
- return ((Date) field).toLocalDate();
+ return LocalDate.ofEpochDay((int) field);
Review Comment:
The same as above.
##########
seatunnel-translation/seatunnel-translation-spark/seatunnel-translation-spark-common/src/main/java/org/apache/seatunnel/translation/spark/common/serialization/InternalRowConverter.java:
##########
@@ -74,13 +76,12 @@ private static Object convert(Object field, SeaTunnelDataType<?> dataType) {
SeaTunnelRowType rowType = (SeaTunnelRowType) dataType;
return convert(seaTunnelRow, rowType);
case DATE:
- return Date.valueOf((LocalDate) field);
+ return (int) ((LocalDate) field).toEpochDay();
case TIME:
- //TODO: how reconvert?
LocalTime localTime = (LocalTime) field;
- return Timestamp.valueOf(LocalDateTime.of(LocalDate.ofEpochDay(0), localTime));
+ return (LocalDateTime.of(LocalDate.ofEpochDay(0), localTime).toInstant(ZONE_UTC)).toEpochMilli();
case TIMESTAMP:
- return Timestamp.valueOf((LocalDateTime) field);
+ return ((LocalDateTime) field).toInstant(ZONE_UTC).toEpochMilli();
Review Comment:
The same as above.
##########
seatunnel-translation/seatunnel-translation-spark/seatunnel-translation-spark-common/src/main/java/org/apache/seatunnel/translation/spark/common/serialization/InternalRowConverter.java:
##########
@@ -164,11 +168,11 @@ private static Object reconvert(Object field, SeaTunnelDataType<?> dataType) {
case ROW:
return reconvert((InternalRow) field, (SeaTunnelRowType) dataType);
case DATE:
- return ((Date) field).toLocalDate();
+ return LocalDate.ofEpochDay((int) field);
case TIME:
- return ((Timestamp) field).toLocalDateTime().toLocalTime();
+ return LocalDateTime.ofInstant(Instant.ofEpochMilli((long) field), ZONE_UTC).toLocalTime();
Review Comment:
The same as above.
##########
seatunnel-translation/seatunnel-translation-spark/seatunnel-translation-spark-common/src/main/java/org/apache/seatunnel/translation/spark/common/serialization/InternalRowConverter.java:
##########
@@ -74,13 +76,12 @@ private static Object convert(Object field, SeaTunnelDataType<?> dataType) {
SeaTunnelRowType rowType = (SeaTunnelRowType) dataType;
return convert(seaTunnelRow, rowType);
case DATE:
- return Date.valueOf((LocalDate) field);
+ return (int) ((LocalDate) field).toEpochDay();
Review Comment:
What's the meaning of using `UTC` zone? I think the data should be consistent.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] laglangyue commented on pull request #2637: [Bug][Translation] fix data,time,timestamp convert for spark
Posted by GitBox <gi...@apache.org>.
laglangyue commented on PR #2637:
URL: https://github.com/apache/incubator-seatunnel/pull/2637#issuecomment-1237312826
> when I check again, I find a bug. It's time type convert:
>
> localTime (source) ->Timestamp(spark)->LocalDatetime(sink), sink only know timestamp ,and can't distinguish between time and timestamp. maybe we need a new Type for time org.apache.seatunnel.translation.spark.common.utils.TypeConverterUtils
and I get a idea, we can transform time and timestamp to timestamp, and value of `int `is time, value of `long `is timestamp, time use the EpochOf Day
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] liugddx commented on a diff in pull request #2637: [Bug][Translation] fix data,time,timestamp convert for spark
Posted by GitBox <gi...@apache.org>.
liugddx commented on code in PR #2637:
URL: https://github.com/apache/incubator-seatunnel/pull/2637#discussion_r962432658
##########
seatunnel-translation/seatunnel-translation-spark/seatunnel-translation-spark-common/src/main/java/org/apache/seatunnel/translation/spark/common/serialization/InternalRowConverter.java:
##########
@@ -164,11 +168,11 @@ private static Object reconvert(Object field, SeaTunnelDataType<?> dataType) {
case ROW:
return reconvert((InternalRow) field, (SeaTunnelRowType) dataType);
case DATE:
- return ((Date) field).toLocalDate();
+ return LocalDate.ofEpochDay((int) field);
Review Comment:
how about like this? `(int) ((Date.valueOf(String.valueOf(val))).toLocalDate().toEpochDay());`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] laglangyue commented on pull request #2637: [hotfix] fix data,time,timestamp convert for spark
Posted by GitBox <gi...@apache.org>.
laglangyue commented on PR #2637:
URL: https://github.com/apache/incubator-seatunnel/pull/2637#issuecomment-1236273225
the type convert line : java type -> sql type -> sparkType -> sql type-> spark
the value convert line : java lue -> mutable value (spark) -> java value
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] ashulin commented on a diff in pull request #2637: [Bug][Translation] fix data,time,timestamp convert for spark
Posted by GitBox <gi...@apache.org>.
ashulin commented on code in PR #2637:
URL: https://github.com/apache/incubator-seatunnel/pull/2637#discussion_r970713003
##########
seatunnel-translation/seatunnel-translation-spark/seatunnel-translation-spark-common/src/main/java/org/apache/seatunnel/translation/spark/common/serialization/InternalRowConverter.java:
##########
@@ -74,13 +73,12 @@ private static Object convert(Object field, SeaTunnelDataType<?> dataType) {
SeaTunnelRowType rowType = (SeaTunnelRowType) dataType;
return convert(seaTunnelRow, rowType);
case DATE:
- return Date.valueOf((LocalDate) field);
+ return (int) ((LocalDate) field).toEpochDay();
case TIME:
- //TODO: how reconvert?
- LocalTime localTime = (LocalTime) field;
- return Timestamp.valueOf(LocalDateTime.of(LocalDate.ofEpochDay(0), localTime));
+ // TODO: how reconvert? there are not Support for Spark Type,we must define the Type in spark
+ throw new RuntimeException("we not support for time type now");
Review Comment:
```suggestion
// TODO: Support TIME Type
throw new RuntimeException("time type is not supported now, but will be supported in the future.");
```
##########
seatunnel-translation/seatunnel-translation-spark/seatunnel-translation-spark-common/src/main/java/org/apache/seatunnel/translation/spark/common/serialization/InternalRowConverter.java:
##########
@@ -167,11 +167,12 @@ private static Object reconvert(Object field, SeaTunnelDataType<?> dataType) {
case ROW:
return reconvert((InternalRow) field, (SeaTunnelRowType) dataType);
case DATE:
- return ((Date) field).toLocalDate();
+ return LocalDate.ofEpochDay((int) field);
case TIME:
- return ((Timestamp) field).toLocalDateTime().toLocalTime();
+ // todo: support this Type
+ throw new RuntimeException("time type not support now but will");
Review Comment:
```suggestion
// TODO: Support TIME Type
throw new RuntimeException("time type is not supported now, but will be supported in the future.");
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] laglangyue commented on pull request #2637: [Bug][Translation] fix data,time,timestamp convert for spark
Posted by GitBox <gi...@apache.org>.
laglangyue commented on PR #2637:
URL: https://github.com/apache/incubator-seatunnel/pull/2637#issuecomment-1237560312
and I update the time convert, what do you think, sink will lost the type of LocalTime
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] laglangyue commented on a diff in pull request #2637: [Bug][Translation] fix data,time,timestamp convert for spark
Posted by GitBox <gi...@apache.org>.
laglangyue commented on code in PR #2637:
URL: https://github.com/apache/incubator-seatunnel/pull/2637#discussion_r964571696
##########
seatunnel-translation/seatunnel-translation-spark/seatunnel-translation-spark-common/src/main/java/org/apache/seatunnel/translation/spark/common/utils/TypeConverterUtils.java:
##########
@@ -85,8 +86,6 @@ public static DataType convert(SeaTunnelDataType<?> dataType) {
return DataTypes.BinaryType;
case DATE:
return DataTypes.DateType;
- case TIME:
- // TODO: how reconvert?
Review Comment:
Or else,we don't suport time Type and throw Exception now. What do you think.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] ashulin commented on a diff in pull request #2637: [Bug][Translation] fix data,time,timestamp convert for spark
Posted by GitBox <gi...@apache.org>.
ashulin commented on code in PR #2637:
URL: https://github.com/apache/incubator-seatunnel/pull/2637#discussion_r964783059
##########
seatunnel-examples/seatunnel-spark-connector-v2-example/src/main/resources/examples/spark.batch.conf:
##########
@@ -72,7 +72,7 @@ transform {
# you can also use other transform plugins, such as sql
sql {
- sql = "select c_array,c_string,c_boolean,c_tinyint,c_smallint,c_int,c_bigint,c_float,c_double,c_null,c_bytes from fake"
+ sql = "select c_array,c_string,c_boolean,c_tinyint,c_smallint,c_int,c_bigint,c_float,c_double,c_null,c_bytes,c_date,c_time,c_timestamp from fake"
Review Comment:
Now we change the time to not supported, why no exception is thrown here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] Hisoka-X commented on a diff in pull request #2637: [Bug][Translation] fix data,time,timestamp convert for spark
Posted by GitBox <gi...@apache.org>.
Hisoka-X commented on code in PR #2637:
URL: https://github.com/apache/incubator-seatunnel/pull/2637#discussion_r969185187
##########
seatunnel-translation/seatunnel-translation-spark/seatunnel-translation-spark-common/src/main/java/org/apache/seatunnel/translation/spark/common/utils/InstantConverterUtils.java:
##########
@@ -0,0 +1,51 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.seatunnel.translation.spark.common.utils;
+
+import java.time.Instant;
+
+public class InstantConverterUtils {
+
+ private static final int MICRO_OF_SECOND = 1000_000;
+ private static final int MICRO_OF_NANOS = 1000;
+
+ /**
+ * @see Instant#toEpochMilli()
+ */
+ public static Long toEpochMicro(Instant instant) {
+ long seconds = instant.getEpochSecond();
+ int nanos = instant.getNano();
+ if (seconds < 0 && nanos > 0) {
Review Comment:
Under what circumstances will second be less than 0？
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] laglangyue commented on pull request #2637: [Bug][Translation] fix data,time,timestamp convert for spark
Posted by GitBox <gi...@apache.org>.
laglangyue commented on PR #2637:
URL: https://github.com/apache/incubator-seatunnel/pull/2637#issuecomment-1237292793
when I check again, I find a bug.
It's time type convert:
localTime (reader) ->Timestamp(spark)->LocalDatetime(sink)
we need a new Type for time
org.apache.seatunnel.translation.spark.common.utils.TypeConverterUtils
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] laglangyue commented on pull request #2637: [Bug][Translation] fix data,time,timestamp convert for spark
Posted by GitBox <gi...@apache.org>.
laglangyue commented on PR #2637:
URL: https://github.com/apache/incubator-seatunnel/pull/2637#issuecomment-1237317077
the type can't convert ,yet.
> > when I check again, I find a bug. It's time type convert:
> > localTime (source) ->Timestamp(spark)->LocalDatetime(sink), sink only know timestamp ,and can't distinguish between time and timestamp. maybe we need a new Type for time org.apache.seatunnel.translation.spark.common.utils.TypeConverterUtils
>
> and I get a idea, we can transform time and timestamp to timestamp, and value of `int `is time, value of `long `is timestamp, time use the EpochOf Day
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] laglangyue commented on pull request #2637: [hotfix] fix data,time,timestamp convert for spark
Posted by GitBox <gi...@apache.org>.
laglangyue commented on PR #2637:
URL: https://github.com/apache/incubator-seatunnel/pull/2637#issuecomment-1236273791
ZoneOffset which is UTC is not import，convert and reconvert ，Keep consistent
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] liugddx commented on a diff in pull request #2637: [Bug][Translation] fix data,time,timestamp convert for spark
Posted by GitBox <gi...@apache.org>.
liugddx commented on code in PR #2637:
URL: https://github.com/apache/incubator-seatunnel/pull/2637#discussion_r962432985
##########
seatunnel-translation/seatunnel-translation-spark/seatunnel-translation-spark-common/src/main/java/org/apache/seatunnel/translation/spark/common/serialization/InternalRowConverter.java:
##########
@@ -164,11 +168,11 @@ private static Object reconvert(Object field, SeaTunnelDataType<?> dataType) {
case ROW:
return reconvert((InternalRow) field, (SeaTunnelRowType) dataType);
case DATE:
- return ((Date) field).toLocalDate();
+ return LocalDate.ofEpochDay((int) field);
case TIME:
- return ((Timestamp) field).toLocalDateTime().toLocalTime();
+ return LocalDateTime.ofInstant(Instant.ofEpochMilli((long) field), ZONE_UTC).toLocalTime();
Review Comment:
like this ?` (int)
((Time.valueOf(String.valueOf(val))).toLocalTime().toNanoOfDay()
/ 1_000_000L)`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] ashulin commented on a diff in pull request #2637: [Bug][Translation] fix data,time,timestamp convert for spark
Posted by GitBox <gi...@apache.org>.
ashulin commented on code in PR #2637:
URL: https://github.com/apache/incubator-seatunnel/pull/2637#discussion_r964326811
##########
seatunnel-translation/seatunnel-translation-spark/seatunnel-translation-spark-common/src/main/java/org/apache/seatunnel/translation/spark/common/serialization/InternalRowConverter.java:
##########
@@ -196,6 +195,7 @@ private static Object reconvert(Object field, SeaTunnelDataType<?> dataType) {
default:
return arrayData.array();
}
+ case TIME:
Review Comment:
You need to `#reconvert` it to `LocalTime`
##########
seatunnel-translation/seatunnel-translation-spark/seatunnel-translation-spark-common/src/main/java/org/apache/seatunnel/translation/spark/common/serialization/InternalRowConverter.java:
##########
@@ -74,13 +74,11 @@ private static Object convert(Object field, SeaTunnelDataType<?> dataType) {
SeaTunnelRowType rowType = (SeaTunnelRowType) dataType;
return convert(seaTunnelRow, rowType);
case DATE:
- return Date.valueOf((LocalDate) field);
+ return (int) ((LocalDate) field).toEpochDay();
case TIME:
- //TODO: how reconvert?
- LocalTime localTime = (LocalTime) field;
- return Timestamp.valueOf(LocalDateTime.of(LocalDate.ofEpochDay(0), localTime));
+ return UTF8String.fromString(((LocalTime) field).toString());
Review Comment:
I don't think it's a good idea to use `String`. It will be difficult to filter on `Time` type field.
##########
seatunnel-translation/seatunnel-translation-spark/seatunnel-translation-spark-common/src/main/java/org/apache/seatunnel/translation/spark/common/utils/TypeConverterUtils.java:
##########
@@ -85,8 +86,6 @@ public static DataType convert(SeaTunnelDataType<?> dataType) {
return DataTypes.BinaryType;
case DATE:
return DataTypes.DateType;
- case TIME:
- // TODO: how reconvert?
Review Comment:
I think either modify it to not support first, or don't change it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] laglangyue commented on a diff in pull request #2637: [Bug][Translation] fix data,time,timestamp convert for spark
Posted by GitBox <gi...@apache.org>.
laglangyue commented on code in PR #2637:
URL: https://github.com/apache/incubator-seatunnel/pull/2637#discussion_r964568987
##########
seatunnel-translation/seatunnel-translation-spark/seatunnel-translation-spark-common/src/main/java/org/apache/seatunnel/translation/spark/common/serialization/InternalRowConverter.java:
##########
@@ -196,6 +195,7 @@ private static Object reconvert(Object field, SeaTunnelDataType<?> dataType) {
default:
return arrayData.array();
}
+ case TIME:
Review Comment:
LocalTime will cause the data Exception, we convert time->timestamp, writer to Sink, is timestamp, not time.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] laglangyue commented on a diff in pull request #2637: [Bug][Translation] fix data,time,timestamp convert for spark
Posted by GitBox <gi...@apache.org>.
laglangyue commented on code in PR #2637:
URL: https://github.com/apache/incubator-seatunnel/pull/2637#discussion_r962466539
##########
seatunnel-translation/seatunnel-translation-spark/seatunnel-translation-spark-common/src/main/java/org/apache/seatunnel/translation/spark/common/serialization/InternalRowConverter.java:
##########
@@ -164,11 +168,11 @@ private static Object reconvert(Object field, SeaTunnelDataType<?> dataType) {
case ROW:
return reconvert((InternalRow) field, (SeaTunnelRowType) dataType);
case DATE:
- return ((Date) field).toLocalDate();
+ return LocalDate.ofEpochDay((int) field);
Review Comment:
> (int) ((Date.valueOf(String.valueOf(val))).toLocalDate().toEpochDay());
the value is int in sparkRow, and transfrom it to LocalDate.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] ashulin commented on a diff in pull request #2637: [Bug][Translation] fix data,time,timestamp convert for spark
Posted by GitBox <gi...@apache.org>.
ashulin commented on code in PR #2637:
URL: https://github.com/apache/incubator-seatunnel/pull/2637#discussion_r967640496
##########
seatunnel-translation/seatunnel-translation-spark/seatunnel-translation-spark-common/src/main/java/org/apache/seatunnel/translation/spark/common/serialization/InternalRowConverter.java:
##########
@@ -102,10 +100,7 @@ private static InternalRow convert(SeaTunnelRow seaTunnelRow, SeaTunnelRowType r
if (TypeConverterUtils.ROW_KIND_FIELD.equals(rowType.getFieldName(i))) {
values[i].update(seaTunnelRow.getRowKind().toByteValue());
} else {
- Object fieldValue = convert(seaTunnelRow.getField(i), rowType.getFieldType(i));
- if (fieldValue != null) {
- values[i].update(fieldValue);
- }
+ values[i].update(convert(seaTunnelRow.getField(i), rowType.getFieldType(i)));
Review Comment:
```suggestion
Object fieldValue = convert(seaTunnelRow.getField(i), rowType.getFieldType(i));
if (fieldValue != null) {
values[i].update(fieldValue);
}
```
you can see [Bug] [seatunnel-translation-spark] Fix NPE caused by incorrect setting of field nullable ([#2670](https://github.com/apache/incubator-seatunnel/issues/2670))
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] EricJoy2048 commented on a diff in pull request #2637: [Bug][Translation] fix data,time,timestamp convert for spark
Posted by GitBox <gi...@apache.org>.
EricJoy2048 commented on code in PR #2637:
URL: https://github.com/apache/incubator-seatunnel/pull/2637#discussion_r970666553
##########
seatunnel-translation/seatunnel-translation-spark/seatunnel-translation-spark-common/src/main/java/org/apache/seatunnel/translation/spark/common/serialization/InternalRowConverter.java:
##########
@@ -74,13 +73,12 @@ private static Object convert(Object field, SeaTunnelDataType<?> dataType) {
SeaTunnelRowType rowType = (SeaTunnelRowType) dataType;
return convert(seaTunnelRow, rowType);
case DATE:
- return Date.valueOf((LocalDate) field);
+ return (int) ((LocalDate) field).toEpochDay();
case TIME:
- //TODO: how reconvert?
- LocalTime localTime = (LocalTime) field;
- return Timestamp.valueOf(LocalDateTime.of(LocalDate.ofEpochDay(0), localTime));
+ // TODO: how reconvert? there are not Support for Spark Type,we must define the Type in spark
+ throw new RuntimeException("we not support for time type now");
Review Comment:
Please replace `we not support for time type now` with `we do not support for time type now`
##########
seatunnel-translation/seatunnel-translation-spark/seatunnel-translation-spark-common/src/main/java/org/apache/seatunnel/translation/spark/common/serialization/InternalRowConverter.java:
##########
@@ -167,11 +167,12 @@ private static Object reconvert(Object field, SeaTunnelDataType<?> dataType) {
case ROW:
return reconvert((InternalRow) field, (SeaTunnelRowType) dataType);
case DATE:
- return ((Date) field).toLocalDate();
+ return LocalDate.ofEpochDay((int) field);
case TIME:
- return ((Timestamp) field).toLocalDateTime().toLocalTime();
+ // todo: support this Type
+ throw new RuntimeException("time type not support now but will");
Review Comment:
Replace with `time type is not supported now, but will be supported in the future`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] laglangyue commented on pull request #2637: [Bug][Translation] fix data,time,timestamp convert for spark
Posted by GitBox <gi...@apache.org>.
laglangyue commented on PR #2637:
URL: https://github.com/apache/incubator-seatunnel/pull/2637#issuecomment-1238089380
@ashulin @TyrantLucifer Please review again
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] laglangyue commented on pull request #2637: [hotfix] fix data,time,timestamp convert for spark
Posted by GitBox <gi...@apache.org>.
laglangyue commented on PR #2637:
URL: https://github.com/apache/incubator-seatunnel/pull/2637#issuecomment-1236274222
@hailin0
@TyrantLucifer
Please review,I need to finish the e2e for DM
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] laglangyue commented on a diff in pull request #2637: [Bug][Translation] fix data,time,timestamp convert for spark
Posted by GitBox <gi...@apache.org>.
laglangyue commented on code in PR #2637:
URL: https://github.com/apache/incubator-seatunnel/pull/2637#discussion_r962474537
##########
seatunnel-translation/seatunnel-translation-spark/seatunnel-translation-spark-common/src/main/java/org/apache/seatunnel/translation/spark/common/serialization/InternalRowConverter.java:
##########
@@ -74,13 +76,12 @@ private static Object convert(Object field, SeaTunnelDataType<?> dataType) {
SeaTunnelRowType rowType = (SeaTunnelRowType) dataType;
return convert(seaTunnelRow, rowType);
case DATE:
- return Date.valueOf((LocalDate) field);
+ return (int) ((LocalDate) field).toEpochDay();
Review Comment:
> What's the meaning of using `UTC` zone? I think the data should be consistent.
It's not important param in Tansform Processing, I choose the one casually ,and I change to Timestamp，it's GMT zone
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] laglangyue commented on pull request #2637: [Bug][Translation] fix data,time,timestamp convert for spark
Posted by GitBox <gi...@apache.org>.
laglangyue commented on PR #2637:
URL: https://github.com/apache/incubator-seatunnel/pull/2637#issuecomment-1237227073
> LGTM
I will improve the accuracy of timestamp
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] ashulin commented on pull request #2637: [Bug][Translation] fix data,time,timestamp convert for spark
Posted by GitBox <gi...@apache.org>.
ashulin commented on PR #2637:
URL: https://github.com/apache/incubator-seatunnel/pull/2637#issuecomment-1238830093
For the `TIME` type, we need to improve the `starter` module to support it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] TyrantLucifer commented on pull request #2637: [Bug][Translation] fix data,time,timestamp convert for spark
Posted by GitBox <gi...@apache.org>.
TyrantLucifer commented on PR #2637:
URL: https://github.com/apache/incubator-seatunnel/pull/2637#issuecomment-1246723638
> +1
Let's waiting CI.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org