waitingkuo commented on code in PR #6832:
URL: https://github.com/apache/arrow-datafusion/pull/6832#discussion_r1268498518


##########
datafusion/core/tests/sql/timestamp.rs:
##########
@@ -1035,13 +1035,13 @@ async fn timestamp_sub_with_tz() -> Result<()> {
     let sql = "SELECT val, ts1 - ts2 AS ts_diff FROM table_a ORDER BY ts2 - 
ts1";
     let actual = execute_to_batches(&ctx, sql).await;
     let expected = vec![
-        "+-----+---------------------------------------------------+",
-        "| val | ts_diff                                           |",
-        "+-----+---------------------------------------------------+",
-        "| 3   | 0 years 0 mons 0 days 10 hours 0 mins 30.000 secs |",
-        "| 1   | 0 years 0 mons 0 days 10 hours 0 mins 20.000 secs |",
-        "| 2   | 0 years 0 mons 0 days 10 hours 0 mins 10.000 secs |",
-        "+-----+---------------------------------------------------+",
+        "+-----+---------+",
+        "| val | ts_diff |",
+        "+-----+---------+",
+        "| 3   | PT30S   |",

Review Comment:
   i think the coercion rules might not work well. 
   ```bash
   ❯ select ('2023-01-02'::timestamp - '2023-01-01'::timestamp)::interval;
   +--------------------------------------------------------+
   | Utf8("2023-01-02") - Utf8("2023-01-01")                |
   +--------------------------------------------------------+
   | 0 years 0 mons 0 days 24 hours 0 mins 0.000000000 secs |
   +--------------------------------------------------------+
   1 row in set. Query took 0.002 seconds.
   ```
   it's converted to (0 month, 0 day, 86400000000000 nanos) instead of (0 
month, 1 day, 0 nanos)
   
   which isn't consist with 
   ```bash
   ❯ select ('2023-01-02'::timestamp - '2023-01-01'::timestamp);
   +-----------------------------------------+
   | Utf8("2023-01-02") - Utf8("2023-01-01") |
   +-----------------------------------------+
   | P1D                                     |
   +-----------------------------------------+
   1 row in set. Query took 0.002 seconds.
   ``` 
   
   both formats are fine so we. in case we decide to align with interval 
format, we should discuss whether it outputs 
   `0 years 0 mons 1 days 0 hours 0 mins 0 secs` or 
   `0 years 0 mons 0 days 24 hours 0 mins 0 secs` in this case



##########
datafusion/core/tests/sql/timestamp.rs:
##########
@@ -1035,13 +1035,13 @@ async fn timestamp_sub_with_tz() -> Result<()> {
     let sql = "SELECT val, ts1 - ts2 AS ts_diff FROM table_a ORDER BY ts2 - 
ts1";
     let actual = execute_to_batches(&ctx, sql).await;
     let expected = vec![
-        "+-----+---------------------------------------------------+",
-        "| val | ts_diff                                           |",
-        "+-----+---------------------------------------------------+",
-        "| 3   | 0 years 0 mons 0 days 10 hours 0 mins 30.000 secs |",
-        "| 1   | 0 years 0 mons 0 days 10 hours 0 mins 20.000 secs |",
-        "| 2   | 0 years 0 mons 0 days 10 hours 0 mins 10.000 secs |",
-        "+-----+---------------------------------------------------+",
+        "+-----+---------+",
+        "| val | ts_diff |",
+        "+-----+---------+",
+        "| 3   | PT30S   |",

Review Comment:
   i think the coercion rules might not work well. 
   ```bash
   ❯ select ('2023-01-02'::timestamp - '2023-01-01'::timestamp)::interval;
   +--------------------------------------------------------+
   | Utf8("2023-01-02") - Utf8("2023-01-01")                |
   +--------------------------------------------------------+
   | 0 years 0 mons 0 days 24 hours 0 mins 0.000000000 secs |
   +--------------------------------------------------------+
   1 row in set. Query took 0.002 seconds.
   ```
   it's converted to (0 month, 0 day, 86400000000000 nanos) instead of (0 
month, 1 day, 0 nanos)
   
   which isn't consist with 
   ```bash
   ❯ select ('2023-01-02'::timestamp - '2023-01-01'::timestamp);
   +-----------------------------------------+
   | Utf8("2023-01-02") - Utf8("2023-01-01") |
   +-----------------------------------------+
   | P1D                                     |
   +-----------------------------------------+
   1 row in set. Query took 0.002 seconds.
   ``` 
   
   both formats are fine so we. in case we decide to align with interval 
format, we should discuss whether it outputs 
   `0 years 0 mons 1 days 0 hours 0 mins 0 secs` or 
   `0 years 0 mons 0 days 24 hours 0 mins 0 secs` in this case



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to