findepi commented on code in PR #12227:
URL: https://github.com/apache/datafusion/pull/12227#discussion_r1738453116


##########
datafusion/functions/src/datetime/to_date.rs:
##########
@@ -118,3 +134,52 @@ impl ScalarUDFImpl for ToDateFunc {
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use arrow::{compute::kernels::cast_utils::Parser, datatypes::Date32Type};
+    use datafusion_common::ScalarValue;
+    use datafusion_expr::{ColumnarValue, ScalarUDFImpl};
+
+    use super::ToDateFunc;
+
+    #[test]
+    fn test_year_9999() {
+        let date_str = "9999-12-31";

Review Comment:
   i have no issue not supporting 5+-digit years, but interestingly they also 
can show in real data. imagine some data using 9999-12-31 as a placeholder and 
then this being converted to a timestamp with time zone and then converted to a 
different zone... this may produce 10000-01-01. And indeed, at least some query 
engines do support such dates, but you're right that 9999-12-31 is what they 
"are obliged" to support.
   
   just saying... it's fine to omit this here for now
   
   
   >  I added tests for (almost) all the examples you mentioned.
   
   
   thank you!
   
   



-- 
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: github-unsubscr...@datafusion.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org

Reply via email to