Refactor tests to use a shared parseDate method (#10091)

Summary: Pull Request resolved: #10091 Add parseDate(string) method to FunctionTestBase and refactor tests to use that. Some tests used to define thie own parseDate, others used DATE()->toDays (string) method, yet others used util::parseDateString(string). Consolidate all call sites to use FunctionTestBase::parseDate(string). Reviewed By: pedroerp Differential Revision: D58258211 fbshipit-source-id: 706a7d7247de453dc4eef1e1ef7505132fe268b8
facebookincubator · Jun 7, 2024 · d31db67 · d31db67
1 parent bf4ee34
commit d31db67
Show file tree

Hide file tree

Showing 12 changed files with 54 additions and 93 deletions.
diff --git a/velox/exec/tests/utils/TpchQueryBuilder.cpp b/velox/exec/tests/utils/TpchQueryBuilder.cpp
@@ -25,9 +25,6 @@
 namespace facebook::velox::exec::test {
 
 namespace {
-int64_t toDate(std::string_view stringDate) {
-  return DATE()->toDays(stringDate);
-}
 
 /// DWRF does not support Date type and Varchar is used.
 /// Return the Date filter expression as per data format.

diff --git a/velox/functions/prestosql/tests/ComparisonsTest.cpp b/velox/functions/prestosql/tests/ComparisonsTest.cpp
@@ -89,10 +89,6 @@ TEST_F(ComparisonsTest, betweenVarchar) {
 }
 
 TEST_F(ComparisonsTest, betweenDate) {
-  auto parseDate = [](const std::string& dateStr) {
-    return DATE()->toDays(dateStr);
-  };
-
   std::vector<std::tuple<int32_t, bool>> testData = {
       {parseDate("2019-05-01"), false},
       {parseDate("2019-06-01"), true},

diff --git a/velox/functions/prestosql/tests/DateTimeFunctionsTest.cpp b/velox/functions/prestosql/tests/DateTimeFunctionsTest.cpp
@@ -205,10 +205,6 @@ class DateTimeFunctionsTest : public functions::test::FunctionBaseTest {
             timestamp.value(), timeZoneName.value().c_str())}));
   }
 
-  static int32_t parseDate(const std::string& dateStr) {
-    return DATE()->toDays(dateStr);
-  }
-
   VectorPtr makeTimestampWithTimeZoneVector(int64_t timestamp, const char* tz) {
     auto tzid = util::getTimeZoneID(tz);
 
@@ -461,10 +457,10 @@ TEST_F(DateTimeFunctionsTest, yearDate) {
     return evaluateOnce<int64_t>("year(c0)", DATE(), date);
   };
   EXPECT_EQ(std::nullopt, year(std::nullopt));
-  EXPECT_EQ(1970, year(DATE()->toDays("1970-01-01")));
-  EXPECT_EQ(1969, year(DATE()->toDays("1969-12-31")));
-  EXPECT_EQ(2020, year(DATE()->toDays("2020-01-01")));
-  EXPECT_EQ(1920, year(DATE()->toDays("1920-12-31")));
+  EXPECT_EQ(1970, year(parseDate("1970-01-01")));
+  EXPECT_EQ(1969, year(parseDate("1969-12-31")));
+  EXPECT_EQ(2020, year(parseDate("2020-01-01")));
+  EXPECT_EQ(1920, year(parseDate("1920-12-31")));
 }
 
 TEST_F(DateTimeFunctionsTest, yearTimestampWithTimezone) {
@@ -3984,62 +3980,62 @@ TEST_F(DateTimeFunctionsTest, castDateToTimestamp) {
     return evaluateOnce<Timestamp>("cast(c0 AS timestamp)", DATE(), date);
   };
 
-  EXPECT_EQ(Timestamp(0, 0), castDateToTimestamp(DATE()->toDays("1970-01-01")));
+  EXPECT_EQ(Timestamp(0, 0), castDateToTimestamp(parseDate("1970-01-01")));
   EXPECT_EQ(
       Timestamp(kSecondsInDay, 0),
-      castDateToTimestamp(DATE()->toDays("1970-01-02")));
+      castDateToTimestamp(parseDate("1970-01-02")));
   EXPECT_EQ(
       Timestamp(2 * kSecondsInDay, 0),
-      castDateToTimestamp(DATE()->toDays("1970-01-03")));
+      castDateToTimestamp(parseDate("1970-01-03")));
   EXPECT_EQ(
       Timestamp(18297 * kSecondsInDay, 0),
-      castDateToTimestamp(DATE()->toDays("2020-02-05")));
+      castDateToTimestamp(parseDate("2020-02-05")));
   EXPECT_EQ(
       Timestamp(-1 * kSecondsInDay, 0),
-      castDateToTimestamp(DATE()->toDays("1969-12-31")));
+      castDateToTimestamp(parseDate("1969-12-31")));
   EXPECT_EQ(
       Timestamp(-18297 * kSecondsInDay, 0),
-      castDateToTimestamp(DATE()->toDays("1919-11-28")));
+      castDateToTimestamp(parseDate("1919-11-28")));
 
   const auto tz = "America/Los_Angeles";
   const auto kTimezoneOffset = 8 * kMillisInHour / 1'000;
   setQueryTimeZone(tz);
   EXPECT_EQ(
       Timestamp(kTimezoneOffset, 0),
-      castDateToTimestamp(DATE()->toDays("1970-01-01")));
+      castDateToTimestamp(parseDate("1970-01-01")));
   EXPECT_EQ(
       Timestamp(kSecondsInDay + kTimezoneOffset, 0),
-      castDateToTimestamp(DATE()->toDays("1970-01-02")));
+      castDateToTimestamp(parseDate("1970-01-02")));
   EXPECT_EQ(
       Timestamp(2 * kSecondsInDay + kTimezoneOffset, 0),
-      castDateToTimestamp(DATE()->toDays("1970-01-03")));
+      castDateToTimestamp(parseDate("1970-01-03")));
   EXPECT_EQ(
       Timestamp(18297 * kSecondsInDay + kTimezoneOffset, 0),
-      castDateToTimestamp(DATE()->toDays("2020-02-05")));
+      castDateToTimestamp(parseDate("2020-02-05")));
   EXPECT_EQ(
       Timestamp(-1 * kSecondsInDay + kTimezoneOffset, 0),
-      castDateToTimestamp(DATE()->toDays("1969-12-31")));
+      castDateToTimestamp(parseDate("1969-12-31")));
   EXPECT_EQ(
       Timestamp(-18297 * kSecondsInDay + kTimezoneOffset, 0),
-      castDateToTimestamp(DATE()->toDays("1919-11-28")));
+      castDateToTimestamp(parseDate("1919-11-28")));
 
   disableAdjustTimestampToTimezone();
-  EXPECT_EQ(Timestamp(0, 0), castDateToTimestamp(DATE()->toDays("1970-01-01")));
+  EXPECT_EQ(Timestamp(0, 0), castDateToTimestamp(parseDate("1970-01-01")));
   EXPECT_EQ(
       Timestamp(kSecondsInDay, 0),
-      castDateToTimestamp(DATE()->toDays("1970-01-02")));
+      castDateToTimestamp(parseDate("1970-01-02")));
   EXPECT_EQ(
       Timestamp(2 * kSecondsInDay, 0),
-      castDateToTimestamp(DATE()->toDays("1970-01-03")));
+      castDateToTimestamp(parseDate("1970-01-03")));
   EXPECT_EQ(
       Timestamp(18297 * kSecondsInDay, 0),
-      castDateToTimestamp(DATE()->toDays("2020-02-05")));
+      castDateToTimestamp(parseDate("2020-02-05")));
   EXPECT_EQ(
       Timestamp(-1 * kSecondsInDay, 0),
-      castDateToTimestamp(DATE()->toDays("1969-12-31")));
+      castDateToTimestamp(parseDate("1969-12-31")));
   EXPECT_EQ(
       Timestamp(-18297 * kSecondsInDay, 0),
-      castDateToTimestamp(DATE()->toDays("1919-11-28")));
+      castDateToTimestamp(parseDate("1919-11-28")));
 }
 
 TEST_F(DateTimeFunctionsTest, lastDayOfMonthDate) {
@@ -4048,7 +4044,7 @@ TEST_F(DateTimeFunctionsTest, lastDayOfMonthDate) {
   };
 
   const auto lastDay = [&](const StringView& dateStr) {
-    return lastDayFunc(DATE()->toDays(dateStr));
+    return lastDayFunc(parseDate(dateStr));
   };
 
   EXPECT_EQ(std::nullopt, lastDayFunc(std::nullopt));
@@ -4146,9 +4142,7 @@ TEST_F(DateTimeFunctionsTest, fromUnixtimeDouble) {
 TEST_F(DateTimeFunctionsTest, toISO8601Date) {
   const auto toISO8601 = [&](const char* dateString) {
     return evaluateOnce<std::string>(
-        "to_iso8601(c0)",
-        DATE(),
-        std::make_optional(DATE()->toDays(dateString)));
+        "to_iso8601(c0)", DATE(), std::make_optional(parseDate(dateString)));
   };
 
   EXPECT_EQ("1970-01-01", toISO8601("1970-01-01"));

diff --git a/velox/functions/prestosql/tests/InPredicateTest.cpp b/velox/functions/prestosql/tests/InPredicateTest.cpp
@@ -454,7 +454,7 @@ TEST_F(InPredicateTest, varbinary) {
 }
 
 TEST_F(InPredicateTest, date) {
-  auto dateValue = DATE()->toDays("2000-01-01");
+  auto dateValue = parseDate("2000-01-01");
 
   auto input = makeRowVector({
       makeNullableFlatVector<int32_t>({dateValue}, DATE()),

diff --git a/velox/functions/prestosql/tests/SequenceTest.cpp b/velox/functions/prestosql/tests/SequenceTest.cpp
@@ -44,10 +44,6 @@ class SequenceTest : public FunctionBaseTest {
 };
 } // namespace
 
-int32_t parseDate(const std::string& dateStr) {
-  return DATE()->toDays(dateStr);
-}
-
 TEST_F(SequenceTest, sequence) {
   const auto startVector = makeFlatVector<int64_t>({1, 2, 10});
   const auto stopVector = makeFlatVector<int64_t>({2, 5, 9});

diff --git a/velox/functions/prestosql/tests/utils/FunctionBaseTest.h b/velox/functions/prestosql/tests/utils/FunctionBaseTest.h
@@ -328,6 +328,12 @@ class FunctionBaseTest : public testing::Test,
         });
   }
 
+  /// Parses a date string into days since epoch.
+  /// Accepts strings formatted as 'YYYY-MM-DD'.
+  static int32_t parseDate(const std::string& text) {
+    return util::fromDateString(text.data(), text.size());
+  }
+
   /// Returns a vector of signatures for the given function name and return
   /// type.
   /// @param returnType The name of expected return type defined in function

diff --git a/velox/functions/sparksql/tests/ArrayMaxTest.cpp b/velox/functions/sparksql/tests/ArrayMaxTest.cpp
@@ -77,16 +77,6 @@ TEST_F(ArrayMaxTest, longVarchar) {
       "red shiny car ahead");
 }
 
-TEST_F(ArrayMaxTest, date) {
-  auto dt = [](const std::string& dateStr) { return DATE()->toDays(dateStr); };
-  EXPECT_EQ(
-      arrayMax<int32_t>({dt("1970-01-01"), dt("2023-08-23")}),
-      dt("2023-08-23"));
-  EXPECT_EQ(arrayMax<int32_t>({}), std::nullopt);
-  EXPECT_EQ(
-      arrayMax<int32_t>({dt("1970-01-01"), std::nullopt}), dt("1970-01-01"));
-}
-
 TEST_F(ArrayMaxTest, timestamp) {
   auto ts = [](int64_t micros) { return Timestamp::fromMicros(micros); };
   EXPECT_EQ(arrayMax<Timestamp>({ts(0), ts(1)}), ts(1));

diff --git a/velox/functions/sparksql/tests/ArrayMinTest.cpp b/velox/functions/sparksql/tests/ArrayMinTest.cpp
@@ -78,16 +78,6 @@ TEST_F(ArrayMinTest, longVarchar) {
       "green plants make us happy");
 }
 
-TEST_F(ArrayMinTest, date) {
-  auto dt = [](const std::string& dateStr) { return DATE()->toDays(dateStr); };
-  EXPECT_EQ(
-      arrayMin<int32_t>({dt("1970-01-01"), dt("2023-08-23")}),
-      dt("1970-01-01"));
-  EXPECT_EQ(arrayMin<int32_t>({}), std::nullopt);
-  EXPECT_EQ(
-      arrayMin<int32_t>({dt("1970-01-01"), std::nullopt}), dt("1970-01-01"));
-}
-
 TEST_F(ArrayMinTest, timestamp) {
   auto ts = [](int64_t micros) { return Timestamp::fromMicros(micros); };
   EXPECT_EQ(arrayMin<Timestamp>({ts(0), ts(1)}), ts(0));

diff --git a/velox/functions/sparksql/tests/ArraySizeTest.cpp b/velox/functions/sparksql/tests/ArraySizeTest.cpp
@@ -44,14 +44,14 @@ TEST_F(ArraySizeTest, boolean) {
   EXPECT_EQ(arraySize<bool>({true, false, true, std::nullopt}), 4);
 }
 
-TEST_F(ArraySizeTest, integer) {
+TEST_F(ArraySizeTest, smallint) {
   EXPECT_EQ(arraySize<int8_t>({}), 0);
   EXPECT_EQ(arraySize<int8_t>({1}), 1);
   EXPECT_EQ(arraySize<int8_t>({std::nullopt}), 1);
   EXPECT_EQ(arraySize<int8_t>({std::nullopt, 1}), 2);
 }
 
-TEST_F(ArraySizeTest, float) {
+TEST_F(ArraySizeTest, real) {
   EXPECT_EQ(arraySize<float>({}), 0);
   EXPECT_EQ(arraySize<float>({1.1}), 1);
   EXPECT_EQ(arraySize<float>({std::nullopt}), 1);
@@ -66,9 +66,8 @@ TEST_F(ArraySizeTest, varchar) {
   EXPECT_EQ(arraySize<std::string>({std::nullopt}), 1);
 }
 
-TEST_F(ArraySizeTest, date) {
-  auto dt = [](const std::string& dateStr) { return DATE()->toDays(dateStr); };
-  EXPECT_EQ(arraySize<int32_t>({dt("1970-01-01"), dt("2023-08-23")}), 2);
+TEST_F(ArraySizeTest, integer) {
+  EXPECT_EQ(arraySize<int32_t>({1, 2}), 2);
 }
 
 TEST_F(ArraySizeTest, timestamp) {

diff --git a/velox/functions/sparksql/tests/DateTimeFunctionsTest.cpp b/velox/functions/sparksql/tests/DateTimeFunctionsTest.cpp
@@ -51,10 +51,6 @@ class DateTimeFunctionsTest : public SparkFunctionBaseTest {
     });
   }
 
-  static int32_t parseDate(const std::string& dateStr) {
-    return DATE()->toDays(dateStr);
-  }
-
   template <typename TOutput, typename TValue>
   std::optional<TOutput> evaluateDateFuncOnce(
       const std::string& expr,
@@ -184,10 +180,10 @@ TEST_F(DateTimeFunctionsTest, yearDate) {
     return evaluateOnce<int32_t>("year(c0)", DATE(), date);
   };
   EXPECT_EQ(std::nullopt, year(std::nullopt));
-  EXPECT_EQ(1970, year(DATE()->toDays("1970-05-05")));
-  EXPECT_EQ(1969, year(DATE()->toDays("1969-12-31")));
-  EXPECT_EQ(2020, year(DATE()->toDays("2020-01-01")));
-  EXPECT_EQ(1920, year(DATE()->toDays("1920-01-01")));
+  EXPECT_EQ(1970, year(parseDate("1970-05-05")));
+  EXPECT_EQ(1969, year(parseDate("1969-12-31")));
+  EXPECT_EQ(2020, year(parseDate("2020-01-01")));
+  EXPECT_EQ(1920, year(parseDate("1920-01-01")));
 }
 
 TEST_F(DateTimeFunctionsTest, weekOfYear) {
@@ -213,12 +209,9 @@ TEST_F(DateTimeFunctionsTest, weekOfYear) {
 }
 
 TEST_F(DateTimeFunctionsTest, unixDate) {
-  const auto unixDate = [&](std::string_view date) {
+  const auto unixDate = [&](const std::string& date) {
     return evaluateOnce<int32_t>(
-        "unix_date(c0)",
-        DATE(),
-        std::make_optional<int32_t>(
-            util::fromDateString(date.data(), date.length())));
+        "unix_date(c0)", DATE(), std::make_optional<int32_t>(parseDate(date)));
   };
 
   EXPECT_EQ(unixDate("1970-01-01"), 0);
@@ -316,8 +309,8 @@ TEST_F(DateTimeFunctionsTest, makeDate) {
                             std::optional<int32_t> day) {
     return evaluateOnce<int32_t>("make_date(c0, c1, c2)", year, month, day);
   };
-  EXPECT_EQ(makeDate(1920, 1, 25), DATE()->toDays("1920-01-25"));
-  EXPECT_EQ(makeDate(-10, 1, 30), DATE()->toDays("-0010-01-30"));
+  EXPECT_EQ(makeDate(1920, 1, 25), parseDate("1920-01-25"));
+  EXPECT_EQ(makeDate(-10, 1, 30), parseDate("-0010-01-30"));
 
   auto errorMessage = fmt::format("Date out of range: {}-12-15", kMax);
   VELOX_ASSERT_THROW(makeDate(kMax, 12, 15), errorMessage);
@@ -329,10 +322,10 @@ TEST_F(DateTimeFunctionsTest, makeDate) {
   VELOX_ASSERT_THROW(makeDate(2022, 3, 35), "Date out of range: 2022-3-35");
 
   VELOX_ASSERT_THROW(makeDate(2023, 4, 31), "Date out of range: 2023-4-31");
-  EXPECT_EQ(makeDate(2023, 3, 31), DATE()->toDays("2023-03-31"));
+  EXPECT_EQ(makeDate(2023, 3, 31), parseDate("2023-03-31"));
 
   VELOX_ASSERT_THROW(makeDate(2023, 2, 29), "Date out of range: 2023-2-29");
-  EXPECT_EQ(makeDate(2023, 3, 29), DATE()->toDays("2023-03-29"));
+  EXPECT_EQ(makeDate(2023, 3, 29), parseDate("2023-03-29"));
 }
 
 TEST_F(DateTimeFunctionsTest, lastDay) {
@@ -341,11 +334,11 @@ TEST_F(DateTimeFunctionsTest, lastDay) {
   };
 
   const auto lastDay = [&](const std::string& dateStr) {
-    return lastDayFunc(DATE()->toDays(dateStr));
+    return lastDayFunc(parseDate(dateStr));
   };
 
   const auto parseDateStr = [&](const std::string& dateStr) {
-    return DATE()->toDays(dateStr);
+    return parseDate(dateStr);
   };
 
   EXPECT_EQ(lastDay("2015-02-28"), parseDateStr("2015-02-28"));
@@ -563,10 +556,10 @@ TEST_F(DateTimeFunctionsTest, dayOfWeekDate) {
   EXPECT_EQ(6, dayOfWeek(parseDate("2023-08-25")));
   EXPECT_EQ(7, dayOfWeek(parseDate("2023-08-26")));
   EXPECT_EQ(1, dayOfWeek(parseDate("2023-08-27")));
-  EXPECT_EQ(6, dayOfWeek(util::fromDateString("2011-05-06")));
-  EXPECT_EQ(4, dayOfWeek(util::fromDateString("2015-04-08")));
-  EXPECT_EQ(7, dayOfWeek(util::fromDateString("2017-05-27")));
-  EXPECT_EQ(6, dayOfWeek(util::fromDateString("1582-10-15")));
+  EXPECT_EQ(6, dayOfWeek(parseDate("2011-05-06")));
+  EXPECT_EQ(4, dayOfWeek(parseDate("2015-04-08")));
+  EXPECT_EQ(7, dayOfWeek(parseDate("2017-05-27")));
+  EXPECT_EQ(6, dayOfWeek(parseDate("1582-10-15")));
 }
 
 TEST_F(DateTimeFunctionsTest, weekdayDate) {

diff --git a/velox/type/TimestampConversion.cpp b/velox/type/TimestampConversion.cpp
@@ -663,7 +663,7 @@ daysSinceEpochFromDayOfYear(int32_t year, int32_t dayOfYear, int64_t& out) {
   return Status::OK();
 }
 
-int64_t fromDateString(const char* str, size_t len) {
+int32_t fromDateString(const char* str, size_t len) {
   int64_t daysSinceEpoch;
   size_t pos = 0;
 

diff --git a/velox/type/TimestampConversion.h b/velox/type/TimestampConversion.h
@@ -117,9 +117,9 @@ daysSinceEpochFromDayOfYear(int32_t year, int32_t dayOfYear, int64_t& out);
 /// separators. Negative years and a trailing "(BC)" are also supported.
 ///
 /// Throws VeloxUserError if the format or date is invalid.
-int64_t fromDateString(const char* buf, size_t len);
+int32_t fromDateString(const char* buf, size_t len);
 
-inline int64_t fromDateString(const StringView& str) {
+inline int32_t fromDateString(const StringView& str) {
   return fromDateString(str.data(), str.size());
 }