kumarUjjawal commented on code in PR #27316:
URL: https://github.com/apache/datafusion/pull/27316#discussion_r4533709191


##########
datafusion/sql/src/planner.rs:
##########
@@ -806,11 +806,20 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
                 ))
             }
             SQLDataType::Date => Ok(DataType::Date32),
-            SQLDataType::Time(None, tz_info) => {
+            SQLDataType::Time(precision, tz_info)
+                if precision.is_none() || [0, 3, 6, 
9].contains(&precision.unwrap()) =>
+            {
                 if *tz_info == TimezoneInfo::None
                     || *tz_info == TimezoneInfo::WithoutTimeZone
                 {
-                    Ok(DataType::Time64(TimeUnit::Nanosecond))
+                    let data_type = match precision {
+                        Some(0) => DataType::Time32(TimeUnit::Second),

Review Comment:
   Correct me if I am wrong but TIME(0), TIME(3) and TIME(6) now give 
Time32(s), Time32(ms) and Time64(us), but DataFusion has no rule to compare two 
time values with different units, so these columns cannot be compared, joined 
or IN-listed against plain TIME values?



##########
datafusion/sqllogictest/test_files/datetime/timestamps.slt:
##########
@@ -749,6 +749,85 @@ select date '1994-01-01' - interval '1' day as date;
 ----
 1993-12-31
 
+# TIME precision selects the Arrow unit; the default remains nanoseconds.
+query TTTTT
+SELECT arrow_typeof(CAST(NULL AS TIME)) AS t,
+       arrow_typeof(CAST(NULL AS TIME(0))) AS s,
+       arrow_typeof(CAST(NULL AS TIME(3))) AS ms,
+       arrow_typeof(CAST(NULL AS TIME(6))) AS us,
+       arrow_typeof(CAST(NULL AS TIME(9))) AS ns;
+----
+Time64(ns) Time32(s) Time32(ms) Time64(µs) Time64(ns)
+
+# String casts truncate fractional seconds to the requested precision.
+query DDDD
+SELECT CAST('12:34:56.123456789' AS TIME(0)) AS s,
+       CAST('12:34:56.123456789' AS TIME(3)) AS ms,
+       CAST('12:34:56.123456789' AS TIME(6)) AS us,
+       CAST('12:34:56.123456789' AS TIME(9)) AS ns;
+----
+12:34:56 12:34:56.123 12:34:56.123456 12:34:56.123456789
+
+# Column declarations use the same mapping, including WITHOUT TIME ZONE.
+statement ok
+CREATE TABLE times (

Review Comment:
   The new `times` table is created but never dropped, so it stays in the 
session for the rest of this long test file.**



##########
datafusion/sql/src/planner.rs:
##########
@@ -806,11 +806,20 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
                 ))
             }
             SQLDataType::Date => Ok(DataType::Date32),
-            SQLDataType::Time(None, tz_info) => {
+            SQLDataType::Time(precision, tz_info)

Review Comment:
   The precision guard and the precision-to-TimeUnit match, including `_ => 
unreachable!()`, are copied from the TIMESTAMP arm instead of shared.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to