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