You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@paimon.apache.org by "Zouxxyy (via GitHub)" <gi...@apache.org> on 2024/07/03 05:48:19 UTC

[PR] [spark] Support timestamp_ntz [paimon]

Zouxxyy opened a new pull request, #3569:
URL: https://github.com/apache/paimon/pull/3569

   <!-- Please specify the module before the PR name: [core] ... or [flink] ... -->
   
   ### Purpose
   
   Spark support timestamp_ntz since spark3.4, paimon can support it. And there were problems with timestamp between paimon and spark types before, the following is the correct result after modification
   
   | Spark Data Type | Paimon Data Type | Atomic Type |
   |----------|----------|----------|
   | TimestampType    | LocalZonedTimestamp   | true   |
   | TimestampNTZType (spark3.4+)   | TimestampType   | true   |
   
   ### Tests
   
   - test("Paimon DDL: create table with timestamp/timestamp_ntz") with parquet, avro, orc. Note: paimon orc timestamp local time zone reader has a bug when change timezone, just skip this.
   
   - Write paimon and parquet, and read it by duckdb:
   ```
   D select * from 'test_hive.db/paimon_tbl/bucket-0/data-fc7c66db-0126-4709-b44a-3243ba06660e-0.parquet' limit 10;
   ┌───────┬──────────────────────────┬─────────────────────┐
   │  id   │           ts1            │         ts2         │
   │ int32 │ timestamp with time zone │      timestamp      │
   ├───────┼──────────────────────────┼─────────────────────┤
   │     1 │ 2023-01-02 00:00:00+08   │ 2023-01-02 00:00:00 │
   └───────┴──────────────────────────┴─────────────────────┘
   D select * from 'test_hive.db/parquet_tbl/part-00000-d5360ba2-6291-429f-9c0c-57f3ef8bfd82-c000.snappy.parquet' limit 10;
   ┌───────┬──────────────────────────┬─────────────────────┐
   │  id   │           ts1            │         ts2         │
   │ int32 │ timestamp with time zone │      timestamp      │
   ├───────┼──────────────────────────┼─────────────────────┤
   │     1 │ 2023-01-02 00:00:00+08   │ 2023-01-02 00:00:00 │
   └───────┴──────────────────────────┴─────────────────────┘
   
   D select name, type, converted_type, logical_type from parquet_schema('test_hive.db/paimon_tbl/bucket-0/data-fc7c66db-0126-4709-b44a-3243ba06660e-0.parquet');
   ┌───────────────┬─────────┬──────────────────┬─────────────────────────────────────────────────────────────────────────────────────────────────────┐
   │     name      │  type   │  converted_type  │                                            logical_type                                             │
   │    varchar    │ varchar │     varchar      │                                               varchar                                               │
   ├───────────────┼─────────┼──────────────────┼─────────────────────────────────────────────────────────────────────────────────────────────────────┤
   │ paimon_schema │         │                  │                                                                                                     │
   │ id            │ INT32   │                  │                                                                                                     │
   │ ts1           │ INT64   │ TIMESTAMP_MICROS │ TimestampType(isAdjustedToUTC=1, unit=TimeUnit(MILLIS=<null>, MICROS=MicroSeconds(), NANOS=<null>)) │
   │ ts2           │ INT64   │ TIMESTAMP_MICROS │ TimestampType(isAdjustedToUTC=0, unit=TimeUnit(MILLIS=<null>, MICROS=MicroSeconds(), NANOS=<null>)) │
   └───────────────┴─────────┴──────────────────┴─────────────────────────────────────────────────────────────────────────────────────────────────────┘
   D select name, type, converted_type, logical_type from parquet_schema('test_hive.db/parquet_tbl/part-00000-d5360ba2-6291-429f-9c0c-57f3ef8bfd82-c000.snappy.parquet');
   ┌──────────────┬─────────┬──────────────────┬─────────────────────────────────────────────────────────────────────────────────────────────────────┐
   │     name     │  type   │  converted_type  │                                            logical_type                                             │
   │   varchar    │ varchar │     varchar      │                                               varchar                                               │
   ├──────────────┼─────────┼──────────────────┼─────────────────────────────────────────────────────────────────────────────────────────────────────┤
   │ spark_schema │         │                  │                                                                                                     │
   │ id           │ INT32   │                  │                                                                                                     │
   │ ts1          │ INT64   │ TIMESTAMP_MICROS │ TimestampType(isAdjustedToUTC=1, unit=TimeUnit(MILLIS=<null>, MICROS=MicroSeconds(), NANOS=<null>)) │
   │ ts2          │ INT64   │ TIMESTAMP_MICROS │ TimestampType(isAdjustedToUTC=0, unit=TimeUnit(MILLIS=<null>, MICROS=MicroSeconds(), NANOS=<null>)) │
   └──────────────┴─────────┴──────────────────┴─────────────────────────────────────────────────────────────────────────────────────────────────────┘
   ```
   
   write paimon orc and read with orc-utils
   
   ```
   orc-tools meta data-84a39541-1165-47f9-adba-478e193a469e-0.orc
   Structure for data-84a39541-1165-47f9-adba-478e193a469e-0.orc
   File Version: 0.12 with ORC_14 by ORC Java 1.9.2
   Rows: 1
   Compression: ZSTD
   Compression size: 262144
   Calendar: Julian/Gregorian
   Type: struct<id:int,binary:binary,ts:timestamp with local time zone,ts_ntz:timestamp>
   ```
   
   ### Compatibility (WARNING!!)
   
   For the previous written paimon table, since its paimon schema is `TimestampType`, spark will interprete it as `TimestampNTZType` and read it correctly (I had copy `TimestampNTZType` to  paimon-spark3.2 and paimon-spark3.3).
   I tested read old paimon ts table manually, there is no compatibility issue and can be read and write correctly, for god sake...
   
   For the new table after this PR, everything goes well, timestamp will be correctly parsed as `TimestampType` while timestamp_ntz wil be parsed as `TimestampNTZType`.
   
   ### API and Format
   
   <!-- Does this change affect API or storage format -->
   
   ### Documentation
   
   
   
   


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [spark] Support timestamp_ntz [paimon]

Posted by "Zouxxyy (via GitHub)" <gi...@apache.org>.
Zouxxyy commented on PR #3569:
URL: https://github.com/apache/paimon/pull/3569#issuecomment-2231145281

   Thanks for the review! @JingsongLi,  may there be no timestamp in heaven


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [spark] Support timestamp_ntz [paimon]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #3569:
URL: https://github.com/apache/paimon/pull/3569#discussion_r1677595317


##########
paimon-spark/paimon-spark-common/src/main/java/org/apache/paimon/spark/SparkRow.java:
##########
@@ -170,7 +171,12 @@ private static int toPaimonDate(Object object) {
 
     private static Timestamp toPaimonTimestamp(Object object) {
         if (object instanceof java.sql.Timestamp) {
-            return Timestamp.fromSQLTimestamp((java.sql.Timestamp) object);
+            java.sql.Timestamp ts = (java.sql.Timestamp) object;
+            if (TypeUtils.treatPaimonTimestampTypeAsSparkTimestampType()) {
+                return Timestamp.fromSQLTimestamp(ts);
+            } else {
+                return DateTimeUtils.toInternalUTC(ts.getTime(), ts.getNanos());

Review Comment:
   Timestamp.fromInstant(timestamp.toInstant())



##########
paimon-spark/paimon-spark-common/src/main/java/org/apache/paimon/spark/SparkRow.java:
##########
@@ -170,7 +171,12 @@ private static int toPaimonDate(Object object) {
 
     private static Timestamp toPaimonTimestamp(Object object) {
         if (object instanceof java.sql.Timestamp) {
-            return Timestamp.fromSQLTimestamp((java.sql.Timestamp) object);
+            java.sql.Timestamp ts = (java.sql.Timestamp) object;
+            if (TypeUtils.treatPaimonTimestampTypeAsSparkTimestampType()) {
+                return Timestamp.fromSQLTimestamp(ts);
+            } else {
+                return DateTimeUtils.toInternalUTC(ts.getTime(), ts.getNanos());
+            }
         } else if (object instanceof java.time.Instant) {
             LocalDateTime localDateTime =
                     LocalDateTime.ofInstant((Instant) object, ZoneId.systemDefault());

Review Comment:
   need to handle this code path.



-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [spark] Support timestamp_ntz [paimon]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #3569:
URL: https://github.com/apache/paimon/pull/3569#discussion_r1677263286


##########
paimon-spark/paimon-spark-common/src/main/java/org/apache/paimon/spark/SparkTypeUtils.java:
##########
@@ -150,7 +151,12 @@ public DataType visit(TimeType timeType) {
 
         @Override
         public DataType visit(TimestampType timestampType) {
-            return DataTypes.TimestampType;
+            try {
+                return DataTypes.TimestampNTZType;
+            } catch (java.lang.NoSuchFieldError e) {
+                // For compatibility with old paimon table written by Spark3.3 and below
+                return TimestampNTZType$.MODULE$;

Review Comment:
   How to use it? No SQL functions support?



-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [spark] Support timestamp_ntz [paimon]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #3569:
URL: https://github.com/apache/paimon/pull/3569#discussion_r1677602414


##########
paimon-spark/paimon-spark-common/src/main/java/org/apache/paimon/spark/SparkInternalRow.java:
##########
@@ -283,7 +284,11 @@ public static org.apache.spark.sql.catalyst.InternalRow fromPaimon(
     }
 
     public static long fromPaimon(Timestamp timestamp) {
-        return DateTimeUtils.fromJavaTimestamp(timestamp.toSQLTimestamp());
+        if (TypeUtils.treatPaimonTimestampTypeAsSparkTimestampType()) {
+            return DateTimeUtils.fromJavaTimestamp(timestamp.toSQLTimestamp());

Review Comment:
   I think here is not correct, can you add tests:
   - Using Paimon API to create table, with timestamp without timezone type and timestamp with local zone type.
   - Using Spark 3.3 to write.
   - Using Spark 3.3 to read.



-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [spark] Support timestamp_ntz [paimon]

Posted by "Zouxxyy (via GitHub)" <gi...@apache.org>.
Zouxxyy commented on PR #3569:
URL: https://github.com/apache/paimon/pull/3569#issuecomment-2228337585

   will add more test cases of select with filter in the next 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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [spark] Support timestamp_ntz [paimon]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi merged PR #3569:
URL: https://github.com/apache/paimon/pull/3569


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [WIP][spark] Support timestamp_ntz [paimon]

Posted by "Zouxxyy (via GitHub)" <gi...@apache.org>.
Zouxxyy commented on code in PR #3569:
URL: https://github.com/apache/paimon/pull/3569#discussion_r1648489356


##########
paimon-spark/paimon-spark-common/src/test/java/org/apache/paimon/spark/SparkInternalRowTest.java:
##########
@@ -93,32 +98,40 @@ public void test() {
                         sparkConverter.apply(new SparkInternalRow(ALL_TYPES).replace(rowData));
 
         String expected =
-                "{"
-                        + "\"id\":1,"
-                        + "\"name\":\"jingsong\","
-                        + "\"char\":\"apache\","
-                        + "\"varchar\":\"paimon\","
-                        + "\"salary\":22.2,"
-                        + "\"locations\":{\"key1\":{\"posX\":1.2,\"posY\":2.3},\"key2\":{\"posX\":2.4,\"posY\":3.5}},"
-                        + "\"strArray\":[\"v1\",\"v5\"],"
-                        + "\"intArray\":[10,30],"
-                        + "\"boolean\":true,"
-                        + "\"tinyint\":22,"
-                        + "\"smallint\":356,"
-                        + "\"bigint\":23567222,"
-                        + "\"bytes\":\"dmFyYmluYXJ5X3Y=\","
-                        + "\"timestamp\":\"2007-12-03 10:15:30\","
-                        + "\"date\":\"2022-05-02\","
-                        + "\"decimal\":0.21,"
-                        + "\"decimal2\":65782123123.01,"
-                        + "\"decimal3\":62123123.5"
-                        + "}";
-        assertThat(sparkRow.json()).isEqualTo(expected);
+                "1,"
+                        + "jingsong,"
+                        + "apache,"
+                        + "paimon,"
+                        + "22.2,"
+                        + "Map(key2 -> [2.4,3.5], key1 -> [1.2,2.3]),"
+                        + "WrappedArray(v1, v5),"
+                        + "WrappedArray(10, 30),"
+                        + "true,"
+                        + "22,"
+                        + "356,"
+                        + "23567222,"
+                        + "[B@,"
+                        + "2007-12-03 18:15:30.0,"
+                        + "2007-12-03T10:15:30,"
+                        + "2022-05-02,"
+                        + "0.21,"
+                        + "65782123123.01,"
+                        + "62123123.5";
+        assertThat(sparkRowToString(sparkRow)).isEqualTo(expected);

Review Comment:
   spark tojson not support timestamp_ntz currently



##########
paimon-spark/paimon-spark-common/src/test/java/org/apache/paimon/spark/SparkInternalRowTest.java:
##########
@@ -93,32 +98,40 @@ public void test() {
                         sparkConverter.apply(new SparkInternalRow(ALL_TYPES).replace(rowData));
 
         String expected =
-                "{"
-                        + "\"id\":1,"
-                        + "\"name\":\"jingsong\","
-                        + "\"char\":\"apache\","
-                        + "\"varchar\":\"paimon\","
-                        + "\"salary\":22.2,"
-                        + "\"locations\":{\"key1\":{\"posX\":1.2,\"posY\":2.3},\"key2\":{\"posX\":2.4,\"posY\":3.5}},"
-                        + "\"strArray\":[\"v1\",\"v5\"],"
-                        + "\"intArray\":[10,30],"
-                        + "\"boolean\":true,"
-                        + "\"tinyint\":22,"
-                        + "\"smallint\":356,"
-                        + "\"bigint\":23567222,"
-                        + "\"bytes\":\"dmFyYmluYXJ5X3Y=\","
-                        + "\"timestamp\":\"2007-12-03 10:15:30\","
-                        + "\"date\":\"2022-05-02\","
-                        + "\"decimal\":0.21,"
-                        + "\"decimal2\":65782123123.01,"
-                        + "\"decimal3\":62123123.5"
-                        + "}";
-        assertThat(sparkRow.json()).isEqualTo(expected);
+                "1,"
+                        + "jingsong,"
+                        + "apache,"
+                        + "paimon,"
+                        + "22.2,"
+                        + "Map(key2 -> [2.4,3.5], key1 -> [1.2,2.3]),"
+                        + "WrappedArray(v1, v5),"
+                        + "WrappedArray(10, 30),"
+                        + "true,"
+                        + "22,"
+                        + "356,"
+                        + "23567222,"
+                        + "[B@,"
+                        + "2007-12-03 18:15:30.0,"
+                        + "2007-12-03T10:15:30,"
+                        + "2022-05-02,"
+                        + "0.21,"
+                        + "65782123123.01,"
+                        + "62123123.5";
+        assertThat(sparkRowToString(sparkRow)).isEqualTo(expected);

Review Comment:
   spark tojson not support timestamp_ntz currently



##########
paimon-spark/paimon-spark-common/src/test/java/org/apache/paimon/spark/SparkInternalRowTest.java:
##########
@@ -93,32 +98,40 @@ public void test() {
                         sparkConverter.apply(new SparkInternalRow(ALL_TYPES).replace(rowData));
 
         String expected =
-                "{"
-                        + "\"id\":1,"
-                        + "\"name\":\"jingsong\","
-                        + "\"char\":\"apache\","
-                        + "\"varchar\":\"paimon\","
-                        + "\"salary\":22.2,"
-                        + "\"locations\":{\"key1\":{\"posX\":1.2,\"posY\":2.3},\"key2\":{\"posX\":2.4,\"posY\":3.5}},"
-                        + "\"strArray\":[\"v1\",\"v5\"],"
-                        + "\"intArray\":[10,30],"
-                        + "\"boolean\":true,"
-                        + "\"tinyint\":22,"
-                        + "\"smallint\":356,"
-                        + "\"bigint\":23567222,"
-                        + "\"bytes\":\"dmFyYmluYXJ5X3Y=\","
-                        + "\"timestamp\":\"2007-12-03 10:15:30\","
-                        + "\"date\":\"2022-05-02\","
-                        + "\"decimal\":0.21,"
-                        + "\"decimal2\":65782123123.01,"
-                        + "\"decimal3\":62123123.5"
-                        + "}";
-        assertThat(sparkRow.json()).isEqualTo(expected);
+                "1,"
+                        + "jingsong,"
+                        + "apache,"
+                        + "paimon,"
+                        + "22.2,"
+                        + "Map(key2 -> [2.4,3.5], key1 -> [1.2,2.3]),"
+                        + "WrappedArray(v1, v5),"
+                        + "WrappedArray(10, 30),"
+                        + "true,"
+                        + "22,"
+                        + "356,"
+                        + "23567222,"
+                        + "[B@,"
+                        + "2007-12-03 18:15:30.0,"
+                        + "2007-12-03T10:15:30,"
+                        + "2022-05-02,"
+                        + "0.21,"
+                        + "65782123123.01,"
+                        + "62123123.5";
+        assertThat(sparkRowToString(sparkRow)).isEqualTo(expected);

Review Comment:
   spark  row's `tojson` not support timestamp_ntz currently



-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org