kou commented on code in PR #50112:
URL: https://github.com/apache/arrow/pull/50112#discussion_r3409284549
##########
cpp/src/gandiva/precompiled/time_test.cc:
##########
@@ -1172,7 +1242,7 @@ TEST(TestTime, TestCastNullableInterval) {
EXPECT_EQ(castNULLABLEINTERVALYEAR_int64(context_ptr, 1201), 1201);
// validate overflow error when using bigint as input
castNULLABLEINTERVALYEAR_int64(context_ptr, INT64_MAX);
- EXPECT_EQ(context.get_error(), "Integer overflow");
+ EXPECT_NE(context.get_error().find("Integer overflow"), std::string::npos);
Review Comment:
Should we use `::testing::HasSubstr` here too?
##########
cpp/src/gandiva/precompiled/time_test.cc:
##########
@@ -962,10 +962,80 @@ TEST(TestTime, TestNextDay) {
ts = StringToTimestamp("2015-08-06 11:12:30");
out = next_day_from_timestamp(context_ptr, ts, "AHSRK", 5);
- EXPECT_EQ(context.get_error(), "The weekday in this entry is invalid");
+ EXPECT_NE(context.get_error().find("NEXT_DAY"), std::string::npos);
+ EXPECT_NE(context.get_error().find("AHSRK"), std::string::npos);
context.Reset();
}
+// Document that next_day's weekday-name matching is case-sensitive:
+// the WEEK[] lookup table holds uppercase names ("MONDAY", "TUE", ...) and
+// is_substr_utf8_utf8 does a byte-exact memcmp, so lowercase or mixed-case
+// input does not match and produces a NEXT_DAY error.
+TEST(TestTime, TestNextDayCaseSensitive) {
+ ExecutionContext context;
+ int64_t context_ptr = reinterpret_cast<int64_t>(&context);
+
+ gdv_timestamp ts = StringToTimestamp("2021-11-08 10:20:34");
+
+ // Uppercase: matches.
+ auto out = next_day_from_timestamp(context_ptr, ts, "FRIDAY", 6);
+ EXPECT_EQ(StringToTimestamp("2021-11-12 00:00:00"), out);
+ EXPECT_FALSE(context.has_error());
+
+ // Lowercase: does NOT match (case-sensitive memcmp against uppercase
WEEK[]).
+ out = next_day_from_timestamp(context_ptr, ts, "friday", 6);
+ EXPECT_TRUE(context.has_error());
+ EXPECT_NE(context.get_error().find("NEXT_DAY"), std::string::npos);
+ EXPECT_NE(context.get_error().find("friday"), std::string::npos);
+ context.Reset();
+
+ // Mixed case: also does NOT match.
+ out = next_day_from_timestamp(context_ptr, ts, "Friday", 6);
+ EXPECT_TRUE(context.has_error());
+ EXPECT_NE(context.get_error().find("NEXT_DAY"), std::string::npos);
+ EXPECT_NE(context.get_error().find("Friday"), std::string::npos);
Review Comment:
Should we use `::testing::HasSubstr` here too?
##########
cpp/src/gandiva/precompiled/time_test.cc:
##########
@@ -962,10 +962,80 @@ TEST(TestTime, TestNextDay) {
ts = StringToTimestamp("2015-08-06 11:12:30");
out = next_day_from_timestamp(context_ptr, ts, "AHSRK", 5);
- EXPECT_EQ(context.get_error(), "The weekday in this entry is invalid");
+ EXPECT_NE(context.get_error().find("NEXT_DAY"), std::string::npos);
+ EXPECT_NE(context.get_error().find("AHSRK"), std::string::npos);
context.Reset();
}
+// Document that next_day's weekday-name matching is case-sensitive:
+// the WEEK[] lookup table holds uppercase names ("MONDAY", "TUE", ...) and
+// is_substr_utf8_utf8 does a byte-exact memcmp, so lowercase or mixed-case
+// input does not match and produces a NEXT_DAY error.
+TEST(TestTime, TestNextDayCaseSensitive) {
+ ExecutionContext context;
+ int64_t context_ptr = reinterpret_cast<int64_t>(&context);
+
+ gdv_timestamp ts = StringToTimestamp("2021-11-08 10:20:34");
+
+ // Uppercase: matches.
+ auto out = next_day_from_timestamp(context_ptr, ts, "FRIDAY", 6);
+ EXPECT_EQ(StringToTimestamp("2021-11-12 00:00:00"), out);
+ EXPECT_FALSE(context.has_error());
+
+ // Lowercase: does NOT match (case-sensitive memcmp against uppercase
WEEK[]).
+ out = next_day_from_timestamp(context_ptr, ts, "friday", 6);
+ EXPECT_TRUE(context.has_error());
+ EXPECT_NE(context.get_error().find("NEXT_DAY"), std::string::npos);
+ EXPECT_NE(context.get_error().find("friday"), std::string::npos);
Review Comment:
Should we use `::testing::HasSubstr` here too?
--
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]
