[GitHub] [incubator-seatunnel] ashulin commented on a diff in pull request #2637: [Bug][Translation] fix data,time,timestamp convert for spark

[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