Skip to content

Commit

Permalink
Remove parseDateString function
Browse files Browse the repository at this point in the history
Summary: Remove fromDateString function. Rename castFromDateString to fromDateString. This makes it consistent with fromTimestampString.

Differential Revision: D58260593
  • Loading branch information
mbasmanova authored and facebook-github-bot committed Jun 6, 2024
1 parent c67589c commit 7113b86
Show file tree
Hide file tree
Showing 10 changed files with 40 additions and 120 deletions.
2 changes: 1 addition & 1 deletion velox/connectors/hive/HiveConnectorUtil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -584,7 +584,7 @@ bool applyPartitionFilter(
const std::string& partitionValue,
common::Filter* filter) {
if (type->isDate()) {
const auto result = util::castFromDateString(
const auto result = util::fromDateString(
StringView(partitionValue), util::ParseMode::kPrestoCast);
VELOX_CHECK(!result.hasError());
return applyFilter(*filter, result.value());
Expand Down
8 changes: 2 additions & 6 deletions velox/connectors/hive/SplitReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,9 @@ VectorPtr newConstantFromString(
}

if (type->isDate()) {
auto copy = util::castFromDateString(
StringView(value.value()), util::ParseMode::kPrestoCast)
.thenOrThrow(folly::identity, [&](const Status& status) {
VELOX_USER_FAIL("{}", status.message());
});
auto days = DATE()->toDays((folly::StringPiece)value.value());
return std::make_shared<ConstantVector<int32_t>>(
pool, size, false, type, std::move(copy));
pool, size, false, type, std::move(days));
}

if constexpr (std::is_same_v<T, StringView>) {
Expand Down
2 changes: 1 addition & 1 deletion velox/expression/PrestoCastHooks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ Expected<int32_t> PrestoCastHooks::castStringToDate(
const StringView& dateString) const {
// Cast from string to date allows only complete ISO 8601 formatted strings:
// [+-](YYYY-MM-DD).
return util::castFromDateString(dateString, util::ParseMode::kPrestoCast);
return util::fromDateString(dateString, util::ParseMode::kPrestoCast);
}

StringView PrestoCastHooks::removeWhiteSpaces(const StringView& view) const {
Expand Down
4 changes: 2 additions & 2 deletions velox/functions/prestosql/DateTimeFunctions.h
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ struct DateFunction : public TimestampWithTimezoneSupport<T> {

FOLLY_ALWAYS_INLINE Status
call(out_type<Date>& result, const arg_type<Varchar>& date) {
auto days = util::castFromDateString(date, util::ParseMode::kPrestoCast);
auto days = util::fromDateString(date, util::ParseMode::kPrestoCast);
if (days.hasError()) {
return days.error();
}
Expand Down Expand Up @@ -1246,7 +1246,7 @@ struct FromIso8601Date {

FOLLY_ALWAYS_INLINE Status
call(out_type<Date>& result, const arg_type<Varchar>& input) {
const auto castResult = util::castFromDateString(
const auto castResult = util::fromDateString(
input.data(), input.size(), util::ParseMode::kIso8601);
if (castResult.hasError()) {
return castResult.error();
Expand Down
2 changes: 1 addition & 1 deletion velox/functions/prestosql/tests/utils/FunctionBaseTest.h
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@ 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());
return DATE()->toDays(text);
}

/// Returns a vector of signatures for the given function name and return
Expand Down
2 changes: 1 addition & 1 deletion velox/functions/sparksql/specialforms/SparkCastHooks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ Expected<int32_t> SparkCastHooks::castStringToDate(
// sequence of characters, e.g:
// "1970-01-01 123"
// "1970-01-01 (BC)"
return util::castFromDateString(
return util::fromDateString(
removeWhiteSpaces(dateString), util::ParseMode::kSparkCast);
}

Expand Down
15 changes: 1 addition & 14 deletions velox/type/TimestampConversion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -663,20 +663,7 @@ daysSinceEpochFromDayOfYear(int32_t year, int32_t dayOfYear, int64_t& out) {
return Status::OK();
}

int32_t fromDateString(const char* str, size_t len) {
int64_t daysSinceEpoch;
size_t pos = 0;

if (!tryParseDateString(str, len, pos, daysSinceEpoch, ParseMode::kStrict)) {
VELOX_USER_FAIL(
"Unable to parse date value: \"{}\", expected format is (YYYY-MM-DD)",
std::string(str, len));
}
return daysSinceEpoch;
}

Expected<int32_t>
castFromDateString(const char* str, size_t len, ParseMode mode) {
Expected<int32_t> fromDateString(const char* str, size_t len, ParseMode mode) {
int64_t daysSinceEpoch;
size_t pos = 0;

Expand Down
20 changes: 3 additions & 17 deletions velox/type/TimestampConversion.h
Original file line number Diff line number Diff line change
Expand Up @@ -112,28 +112,14 @@ Status daysSinceEpochFromWeekDate(
Status
daysSinceEpochFromDayOfYear(int32_t year, int32_t dayOfYear, int64_t& out);

/// Returns the (signed) number of days since unix epoch (1970-01-01), following
/// the "YYYY-MM-DD" format (ISO 8601). ' ', '/' and '\' are also acceptable
/// separators. Negative years and a trailing "(BC)" are also supported.
///
/// Throws VeloxUserError if the format or date is invalid.
int32_t fromDateString(const char* buf, size_t len);

inline int32_t fromDateString(const StringView& str) {
return fromDateString(str.data(), str.size());
}

/// Cast string to date. Supported date formats vary, depending on input
/// ParseMode. Refer to ParseMode enum for further info.
///
/// Throws VeloxUserError if the format or date is invalid.
Expected<int32_t>
castFromDateString(const char* buf, size_t len, ParseMode mode);
Expected<int32_t> fromDateString(const char* buf, size_t len, ParseMode mode);

inline Expected<int32_t> castFromDateString(
const StringView& str,
ParseMode mode) {
return castFromDateString(str.data(), str.size(), mode);
inline Expected<int32_t> fromDateString(const StringView& str, ParseMode mode) {
return fromDateString(str.data(), str.size(), mode);
}

// Extracts the day of the week from the number of days since epoch
Expand Down
7 changes: 5 additions & 2 deletions velox/type/Type.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1006,11 +1006,14 @@ std::string DateType::toIso8601(int32_t days) {
}

int32_t DateType::toDays(folly::StringPiece in) const {
return util::fromDateString(in.data(), in.size());
return toDays(in.data(), in.size());
}

int32_t DateType::toDays(const char* in, size_t len) const {
return util::fromDateString(in, len);
return util::fromDateString(in, len, util::ParseMode::kPrestoCast)
.thenOrThrow(folly::identity, [&](const Status& status) {
VELOX_USER_FAIL("{}", status.message());
});
}

namespace {
Expand Down
98 changes: 23 additions & 75 deletions velox/type/tests/TimestampConversionTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ Timestamp parseTimestamp(
});
}

int32_t parseDate(const StringView& str, ParseMode mode) {
return castFromDateString(str.data(), str.size(), mode)
int32_t fromDateString(const StringView& str, ParseMode mode) {
return fromDateString(str.data(), str.size(), mode)
.thenOrThrow(folly::identity, [&](const Status& status) {
VELOX_USER_FAIL("{}", status.message());
});
Expand Down Expand Up @@ -105,97 +105,45 @@ TEST(DateTimeUtilTest, fromDateInvalid) {
}

TEST(DateTimeUtilTest, fromDateString) {
EXPECT_EQ(10957, fromDateString("2000-01-01"));
EXPECT_EQ(0, fromDateString("1970-01-01"));
EXPECT_EQ(1, fromDateString("1970-01-02"));

// Single character
EXPECT_EQ(1, fromDateString("1970-1-2"));

// Old and negative years.
EXPECT_EQ(-719528, fromDateString("0-1-1"));
EXPECT_EQ(-719162, fromDateString("1-1-1"));
EXPECT_EQ(-719893, fromDateString("-1-1-1"));
EXPECT_EQ(-720258, fromDateString("-2-1-1"));

// 1BC is equal 0-1-1.
EXPECT_EQ(-719528, fromDateString("1-1-1 (BC)"));
EXPECT_EQ(-719893, fromDateString("2-1-1 (BC)"));

// Leading zeros and spaces.
EXPECT_EQ(-719162, fromDateString("00001-1-1"));
EXPECT_EQ(-719162, fromDateString(" 1-1-1"));
EXPECT_EQ(-719162, fromDateString(" 1-1-1"));
EXPECT_EQ(-719162, fromDateString("\t1-1-1"));
EXPECT_EQ(-719162, fromDateString(" \t \n 00001-1-1 \n"));

// Different separators.
EXPECT_EQ(-719162, fromDateString("1/1/1"));
EXPECT_EQ(-719162, fromDateString("1 1 1"));
EXPECT_EQ(-719162, fromDateString("1\\1\\1"));

// Other string types.
EXPECT_EQ(0, fromDateString(StringView("1970-01-01")));
}

TEST(DateTimeUtilTest, fromDateStrInvalid) {
EXPECT_THROW(fromDateString(""), VeloxUserError);
EXPECT_THROW(fromDateString(" "), VeloxUserError);
EXPECT_THROW(fromDateString("2000"), VeloxUserError);

// Different separators.
EXPECT_THROW(fromDateString("2000/01-01"), VeloxUserError);
EXPECT_THROW(fromDateString("2000 01-01"), VeloxUserError);

// Trailing characters.
EXPECT_THROW(fromDateString("2000-01-01 asdf"), VeloxUserError);
EXPECT_THROW(fromDateString("2000-01-01 0"), VeloxUserError);

// Too large of a year.
EXPECT_THROW(fromDateString("1000000"), VeloxUserError);
EXPECT_THROW(fromDateString("-1000000"), VeloxUserError);
}

TEST(DateTimeUtilTest, castFromDateString) {
for (ParseMode mode : {ParseMode::kPrestoCast, ParseMode::kSparkCast}) {
EXPECT_EQ(0, parseDate("1970-01-01", mode));
EXPECT_EQ(3789742, parseDate("12345-12-18", mode));
EXPECT_EQ(0, fromDateString("1970-01-01", mode));
EXPECT_EQ(3789742, fromDateString("12345-12-18", mode));

EXPECT_EQ(1, parseDate("1970-1-2", mode));
EXPECT_EQ(1, parseDate("1970-01-2", mode));
EXPECT_EQ(1, parseDate("1970-1-02", mode));
EXPECT_EQ(1, fromDateString("1970-1-2", mode));
EXPECT_EQ(1, fromDateString("1970-01-2", mode));
EXPECT_EQ(1, fromDateString("1970-1-02", mode));

EXPECT_EQ(1, parseDate("+1970-01-02", mode));
EXPECT_EQ(-719893, parseDate("-1-1-1", mode));
EXPECT_EQ(1, fromDateString("+1970-01-02", mode));
EXPECT_EQ(-719893, fromDateString("-1-1-1", mode));

EXPECT_EQ(0, parseDate(" 1970-01-01", mode));
EXPECT_EQ(0, parseDate("1970-01-01 ", mode));
EXPECT_EQ(0, parseDate(" 1970-01-01 ", mode));
EXPECT_EQ(0, fromDateString(" 1970-01-01", mode));
EXPECT_EQ(0, fromDateString("1970-01-01 ", mode));
EXPECT_EQ(0, fromDateString(" 1970-01-01 ", mode));
}

EXPECT_EQ(3789391, parseDate("12345", ParseMode::kSparkCast));
EXPECT_EQ(16436, parseDate("2015", ParseMode::kSparkCast));
EXPECT_EQ(16495, parseDate("2015-03", ParseMode::kSparkCast));
EXPECT_EQ(16512, parseDate("2015-03-18T", ParseMode::kSparkCast));
EXPECT_EQ(16512, parseDate("2015-03-18T123123", ParseMode::kSparkCast));
EXPECT_EQ(16512, parseDate("2015-03-18 123142", ParseMode::kSparkCast));
EXPECT_EQ(16512, parseDate("2015-03-18 (BC)", ParseMode::kSparkCast));
EXPECT_EQ(3789391, fromDateString("12345", ParseMode::kSparkCast));
EXPECT_EQ(16436, fromDateString("2015", ParseMode::kSparkCast));
EXPECT_EQ(16495, fromDateString("2015-03", ParseMode::kSparkCast));
EXPECT_EQ(16512, fromDateString("2015-03-18T", ParseMode::kSparkCast));
EXPECT_EQ(16512, fromDateString("2015-03-18T123123", ParseMode::kSparkCast));
EXPECT_EQ(16512, fromDateString("2015-03-18 123142", ParseMode::kSparkCast));
EXPECT_EQ(16512, fromDateString("2015-03-18 (BC)", ParseMode::kSparkCast));
}

TEST(DateTimeUtilTest, castFromDateStringInvalid) {
TEST(DateTimeUtilTest, fromDateStringInvalid) {
auto testCastFromDateStringInvalid = [&](const StringView& str,
ParseMode mode) {
if (mode == ParseMode::kPrestoCast) {
VELOX_ASSERT_THROW(
parseDate(str, mode),
fromDateString(str, mode),
fmt::format(
"Unable to parse date value: \"{}\". "
"Valid date string pattern is (YYYY-MM-DD), "
"and can be prefixed with [+-]",
std::string(str.data(), str.size())));
} else if (mode == ParseMode::kSparkCast) {
VELOX_ASSERT_THROW(
parseDate(str, mode),
fromDateString(str, mode),
fmt::format(
"Unable to parse date value: \"{}\". "
"Valid date string patterns include "
Expand All @@ -205,7 +153,7 @@ TEST(DateTimeUtilTest, castFromDateStringInvalid) {
std::string(str.data(), str.size())));
} else if (mode == ParseMode::kIso8601) {
VELOX_ASSERT_THROW(
parseDate(str, mode),
fromDateString(str, mode),
fmt::format(
"Unable to parse date value: \"{}\". "
"Valid date string patterns include "
Expand Down

0 comments on commit 7113b86

Please sign in to comment.