[GitHub] [arrow-datafusion] comphead commented on a diff in pull request #5517: fix: cast literal to timestamp

["comphead (via GitHub)" <gi...@apache.org>]

comphead commented on code in PR #5517:
URL: https://github.com/apache/arrow-datafusion/pull/5517#discussion_r1130228489


##########
datafusion/optimizer/src/unwrap_cast_in_comparison.rs:
##########
@@ -1070,4 +1119,19 @@ mod tests {
             }
         }
     }
+
+    #[test]
+    fn test_try_cast_literal_to_timestamp() {

Review Comment:
   please add more tests between timestamps



##########
datafusion/optimizer/src/unwrap_cast_in_comparison.rs:
##########
@@ -428,6 +452,31 @@ fn try_cast_literal_to_type(
     }
 }
 
+/// Cast a timestamp value from one unit to another
+fn cast_between_timestamp(from: DataType, to: DataType, value: i128) -> Option<i64> {
+    let seconds = match from {

Review Comment:
   I think it can be improved. if the cast Second to Second, then the code will do unneccessary mul by 1m and then div  by 1m



-- 
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-unsubscribe@arrow.apache.org

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