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



##########
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:
       @alamb according to the DataFusion README, this shows up:
   ```
   TIME | Time64(TimeUnit::Millisecond)
   -- | --
   ```
   Is this only for time of the day?
   
   Basically, we have datasets in both nanos and millis, and don't really care 
for the extra precision of nanos, and a whole bunch of processing code tuned 
towards millis, so would rather convert everything to millis for convenience.  
What I really need is a convenient way to cast things to millis, or precisely 
`Timestamp(Milliseconds, None)`, that works with the SQL parser... what do you 
recommend?
   
   I'm not sure there is a SQL convention for precision of timestamps like in 
Arrow.  I see some places that have decimal places of precision, which doesn't 
really map well to our scheme either.  We could invent something different, 
like `TIMESTAMP(precision)` or allow casting to 
`TIMESTAMP(Nanos/Millis/Micros)` etc which would solve the problem... WDYT?
   




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


Reply via email to