velvia commented on a change in pull request #10005:
URL: https://github.com/apache/arrow/pull/10005#discussion_r613406501



##########
File path: rust/datafusion/src/sql/planner.rs
##########
@@ -1498,6 +1498,7 @@ pub fn convert_data_type(sql: &SQLDataType) -> 
Result<DataType> {
         SQLDataType::Double => Ok(DataType::Float64),
         SQLDataType::Char(_) | SQLDataType::Varchar(_) => Ok(DataType::Utf8),
         SQLDataType::Timestamp => Ok(DataType::Timestamp(TimeUnit::Nanosecond, 
None)),
+        SQLDataType::Time => Ok(DataType::Timestamp(TimeUnit::Millisecond, 
None)),

Review comment:
       Hmmm, I'm def in favour of implementing PostGres compatible support, 
though there is a lot there, especially with intervals and timestamps, and many 
operators including math operators to add.
   
   As for `date_trunc()`, my read of the PostGres functionality is that it is 
used for date _rounding_, like I can round something to the nearest hour or 
minute.   This is something definitely useful to add, though I'm more 
interested in casting to a specific precision.
   
   Here's my proposal:
   * add `date_trunc('milliseconds', _)` -> returns `Timestamp(Milliseconds, 
None)`
   * add `date_trunc('microseconds', _)` -> returns `Timestamp(Microseconds, 
None)`
   * Leave implementation of other time periods to a future PR (there is also a 
question of what type to return)
   * I would also add automatic coercion of Timestamp millis (and micros, and 
maybe nanos?) to Int64 for fast and easy Int64 comparison, and coercion to 
nanos for when comparison with nano timestamps matters.
   
   The issue with return types would need to be solved, but the above proposal 
would work for our needs.
   I can also update the bug once we decide on the solution.
   
   thanks,
   Evan




-- 
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.

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


Reply via email to