pearu commented on code in PR #50146:
URL: https://github.com/apache/arrow/pull/50146#discussion_r4263238416


##########
cpp/src/arrow/util/value_parsing_test.cc:
##########
@@ -869,11 +870,45 @@ TEST(TimestampParser, StrptimeParser) {
   }
 
   // Unparseable strings
-  std::vector<std::string> unparseables = {"foo", "5/1/2000", "5/1/2000 
12:34:56:6"};
+  std::vector<std::string> unparseables = {
+      "foo", "5/1/2000", "5/1/2000 12:34:56:6",
+      // Leap seconds are rejected, as by the
+      // ISO-8601 parser
+      "5/31/2000 12:30:60", "5/31/2000 23:59:60", "5/31/2000 23:59:61",
+      // Impossible dates are rejected rather
+      // than rolled over into the next month
+      "2/30/2000 12:34:56", "4/31/2000 12:34:56", "2/29/2001 12:34:56",
+      // A zero day is rejected rather than
+      // taken as the first of the month
+      "2/00/2000 12:34:56"};
   for (auto& value : unparseables) {
     int64_t dummy;
     ASSERT_FALSE((*parser)(value.c_str(), value.size(), TimeUnit::SECOND, 
&dummy));
   }
+
+  // A format without a day still parses (the day defaults to the first)
+  auto month_parser = TimestampParser::MakeStrptime("%Y-%m");
+  int64_t converted, expected;
+  ASSERT_TRUE((*month_parser)("2000-02", 7, TimeUnit::SECOND, &converted));
+  ASSERT_TRUE(ParseTimestampISO8601("2000-02-01", 10, TimeUnit::SECOND, 
&expected));
+  ASSERT_EQ(expected, converted);
+}
+
+TEST(TimestampParser, StrptimeYearOutOfRange) {
+  auto parser = TimestampParser::MakeStrptime("%s");
+  int64_t converted = 0;
+  if (!(*parser)("1584282600", 10, TimeUnit::SECOND, &converted)) {

Review Comment:
   Thanks. Applied your probe in 8ccc1ceb85, with the comment not tied to 
glibc/i386: since glibc 2.38 (commit 41349f6f67, bug 30053, also on the 2.36 
branch) `strptime_l.c` defines `time_t` as `__time64_t` inside libc, so `%s` is 
parsed into 64 bits whatever the caller's `time_t`. I checked in an 
`i386/debian:13` container (glibc 2.41-12+deb13u4, 32-bit `time_t` in the 
program): `4102444800` parses to 2100-01-01 and the three test values to 
`tm_year` 31688808, 2147483641 and 2147483647, so the test passes there as it 
was; same on `i386/debian:12` (glibc 2.36). Submitting the nightly job to 
confirm.



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