[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #2924: Add `from_unixtime` function

[GitBox <gi...@apache.org>]

alamb commented on code in PR #2924:
URL: https://github.com/apache/arrow-datafusion/pull/2924#discussion_r923642791


##########
datafusion/core/tests/sql/timestamp.rs:
##########
@@ -266,6 +266,37 @@ async fn query_cast_timestamp_micros_to_others() -> Result<()> {
     Ok(())
 }
 
+#[tokio::test]
+async fn query_cast_timestamp_from_unixtime() -> Result<()> {
+    let ctx = SessionContext::new();
+
+    let t1_schema = Arc::new(Schema::new(vec![Field::new("ts", DataType::Int64, true)]));
+    let t1_data = RecordBatch::try_new(
+        t1_schema.clone(),
+        vec![Arc::new(Int64Array::from(vec![
+            1235865600, 1235865660, 1238544000,
+        ]))],
+    )?;
+    let t1_table = MemTable::try_new(t1_schema, vec![vec![t1_data]])?;
+    ctx.register_table("t1", Arc::new(t1_table))?;
+
+    let sql = "SELECT from_unixtime(ts) FROM t1 LIMIT 3";
+    let actual = execute_to_batches(&ctx, sql).await;
+
+    let expected = vec![

Review Comment:
   For what it is worth `mysql` gives a different answer (defaults to local time)
   
   ```sql
   mysql> select from_unixtime(1238544000);
   +---------------------------+
   | from_unixtime(1238544000) |
   +---------------------------+
   | 2009-03-31 20:00:00       |
   +---------------------------+
   1 row in set (0.00 sec)
   ```
   
   However, when I set the session timezone to UTC things look good
   ```sql
   mysql> SET time_zone = '+0:00';
   Query OK, 0 rows affected (0.00 sec)
   
   mysql> select from_unixtime(1238544000);
   +---------------------------+
   | from_unixtime(1238544000) |
   +---------------------------+
   | 2009-04-01 00:00:00       |
   +---------------------------+
   1 row in set (0.00 sec)
   
   ```
   
   
   (This behavior is not unique to `from_unixtime` I think it affects all timestamp arithmetic in DataFusion)



##########
datafusion/physical-expr/src/datetime_expressions.rs:
##########
@@ -333,6 +333,8 @@ pub fn date_part(args: &[ColumnarValue]) -> Result<ColumnarValue> {
     }
     let (date_part, array) = (&args[0], &args[1]);
 
+    println!("date_part --> {:?}", date_part.data_type());

Review Comment:
   This seems like a left over from debugging



##########
datafusion/physical-expr/src/functions.rs:
##########
@@ -152,6 +152,23 @@ pub fn create_physical_expr(
                 }
             }
         }),
+        BuiltinScalarFunction::FromUnixtime => Arc::new({
+            match coerced_phy_exprs[0].data_type(input_schema) {
+                Ok(DataType::Int64) => |col_values: &[ColumnarValue]| {
+                    cast_column(

Review Comment:
   👍 



-- 
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: github-unsubscribe@arrow.apache.org

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