lriggs commented on code in PR #48867:
URL: https://github.com/apache/arrow/pull/48867#discussion_r2728940606


##########
cpp/src/gandiva/precompiled/time_test.cc:
##########
@@ -122,15 +122,26 @@ TEST(TestTime, TestCastTimestamp) {
             "Not a valid time for timestamp value 2000-01-01 00:00:100");
   context.Reset();
 
-  EXPECT_EQ(castTIMESTAMP_utf8(context_ptr, "2000-01-01 00:00:00.0001", 24), 
0);
-  EXPECT_EQ(context.get_error(),
-            "Invalid millis for timestamp value 2000-01-01 00:00:00.0001");
-  context.Reset();
-
-  EXPECT_EQ(castTIMESTAMP_utf8(context_ptr, "2000-01-01 00:00:00.1000", 24), 
0);
-  EXPECT_EQ(context.get_error(),
-            "Invalid millis for timestamp value 2000-01-01 00:00:00.1000");
-  context.Reset();
+  // Test truncation of subseconds to 3 digits (milliseconds)

Review Comment:
   Can you add some test cases with less than three digits?



##########
cpp/src/gandiva/precompiled/time.cc:
##########
@@ -566,6 +567,28 @@ bool is_valid_time(const int hours, const int minutes, 
const int seconds) {
          seconds < 60;
 }
 
+// Normalize sub-seconds value to milliseconds precision (3 digits).
+// Truncates if more than 3 digits are provided, pads with zeros if fewer than 
3 digits
+ARROW_FORCE_INLINE

Review Comment:
   I might use the regular 'inline'. I don't see ARROW_FORCE_INLINE really 
being used anywhere and I don't feel like I know enough that this function 
should be forced inline.



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

Reply via email to