alamb commented on a change in pull request #10005:
URL: https://github.com/apache/arrow/pull/10005#discussion_r613966804
##########
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:
> 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?)
I think that sounds reasonable
> So the big question (and I'm not sure if this PR is the right forum for
this) is which direction should the project be going. There is a mismatch
between SQL support, the design to work with one type, and the Arrow support
for multiple time resolutions.
I don't think this PR is the right forum -- I think sending something to
[email protected] is probably the right place to have that discussion.
> Have most functions work on nano resolution timestamps, and have automatic
coercion of other resolutions to nanos when calling functions etc. This imposes
a perf penalty, but maybe an acceptable one.
this is my personal preferred solution , but I think we should gather input
from the larger community.
> Oh, there is also a easier solution. Expand return_type() method in
functions.rs to allow passing in the input arrays, so that we could return a
different type depending on the time granularity argument.
I think this idea is worth exploring some more
--
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:
[email protected]
