Skip to content

Commit

Permalink
Remove fromTimeString function (facebookincubator#10096)
Browse files Browse the repository at this point in the history 
Summary:
Pull Request resolved: facebookincubator#10096

Change 2 call sites in test to use fromTimestampString.

Reviewed By: xiaoxmeng

Differential Revision: D58264094

fbshipit-source-id: 8a2e787c8960aa65cae98ba500e1872c5f0acac6
  • Loading branch information
@mbasmanova @facebook-github-bot
mbasmanova authored and facebook-github-bot committed Jun 7, 2024
1 parent d9d730c commit b3f53f6
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 106 deletions.
50 changes: 30 additions & 20 deletions velox/functions/prestosql/tests/DateTimeFunctionsTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -517,10 +517,15 @@ TEST_F(DateTimeFunctionsTest, weekDate) {
}

TEST_F(DateTimeFunctionsTest, week) {
const auto weekTimestamp = [&](const char* time) {
auto timestampInSeconds = util::fromTimeString(time) / 1'000'000;
const auto weekTimestamp = [&](std::string_view time) {
auto ts = util::fromTimestampString(
time.data(), time.size(), util::TimestampParseMode::kIso8601)
.thenOrThrow(folly::identity, [&](const Status& status) {
VELOX_USER_FAIL("{}", status.message());
});
auto timestamp =
std::make_optional(Timestamp(timestampInSeconds * 100'000'000, 0));
std::make_optional(Timestamp(ts.getSeconds() * 100'000'000, 0));

auto week = evaluateOnce<int64_t>("week(c0)", timestamp).value();
auto weekOfYear =
evaluateOnce<int64_t>("week_of_year(c0)", timestamp).value();
Expand All @@ -529,19 +534,24 @@ TEST_F(DateTimeFunctionsTest, week) {
return week;
};

EXPECT_EQ(1, weekTimestamp("00:00:00"));
EXPECT_EQ(10, weekTimestamp("11:59:59"));
EXPECT_EQ(51, weekTimestamp("06:01:01"));
EXPECT_EQ(24, weekTimestamp("06:59:59"));
EXPECT_EQ(27, weekTimestamp("12:00:01"));
EXPECT_EQ(7, weekTimestamp("12:59:59"));
EXPECT_EQ(1, weekTimestamp("T00:00:00"));
EXPECT_EQ(10, weekTimestamp("T11:59:59"));
EXPECT_EQ(51, weekTimestamp("T06:01:01"));
EXPECT_EQ(24, weekTimestamp("T06:59:59"));
EXPECT_EQ(27, weekTimestamp("T12:00:01"));
EXPECT_EQ(7, weekTimestamp("T12:59:59"));
}

TEST_F(DateTimeFunctionsTest, weekTimestampWithTimezone) {
const auto weekTimestampTimezone = [&](const char* time,
const auto weekTimestampTimezone = [&](std::string_view time,
const char* timezone) {
auto timestampInSeconds = util::fromTimeString(time) / 1'000'000;
auto timestamp = timestampInSeconds * 100'000'000;
auto ts = util::fromTimestampString(
time.data(), time.size(), util::TimestampParseMode::kIso8601)
.thenOrThrow(folly::identity, [&](const Status& status) {
VELOX_USER_FAIL("{}", status.message());
});

auto timestamp = ts.getSeconds() * 100'000'000;
auto week = evaluateWithTimestampWithTimezone<int64_t>(
"week(c0)", timestamp, timezone)
.value();
Expand All @@ -553,14 +563,14 @@ TEST_F(DateTimeFunctionsTest, weekTimestampWithTimezone) {
return week;
};

EXPECT_EQ(1, weekTimestampTimezone("00:00:00", "-12:00"));
EXPECT_EQ(1, weekTimestampTimezone("00:00:00", "+12:00"));
EXPECT_EQ(47, weekTimestampTimezone("11:59:59", "-12:00"));
EXPECT_EQ(47, weekTimestampTimezone("11:59:59", "+12:00"));
EXPECT_EQ(33, weekTimestampTimezone("06:01:01", "-12:00"));
EXPECT_EQ(34, weekTimestampTimezone("06:01:01", "+12:00"));
EXPECT_EQ(47, weekTimestampTimezone("12:00:01", "-12:00"));
EXPECT_EQ(47, weekTimestampTimezone("12:00:01", "+12:00"));
EXPECT_EQ(1, weekTimestampTimezone("T00:00:00", "-12:00"));
EXPECT_EQ(1, weekTimestampTimezone("T00:00:00", "+12:00"));
EXPECT_EQ(47, weekTimestampTimezone("T11:59:59", "-12:00"));
EXPECT_EQ(47, weekTimestampTimezone("T11:59:59", "+12:00"));
EXPECT_EQ(33, weekTimestampTimezone("T06:01:01", "-12:00"));
EXPECT_EQ(34, weekTimestampTimezone("T06:01:01", "+12:00"));
EXPECT_EQ(47, weekTimestampTimezone("T12:00:01", "-12:00"));
EXPECT_EQ(47, weekTimestampTimezone("T12:00:01", "+12:00"));
}

TEST_F(DateTimeFunctionsTest, quarter) {
Expand Down
32 changes: 1 addition & 31 deletions velox/type/TimestampConversion.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -738,36 +738,6 @@ fromTime(int32_t hour, int32_t minute, int32_t second, int32_t microseconds) {
return result;
}

int64_t fromTimeString(const char* str, size_t len) {
int64_t microsSinceMidnight;
size_t pos;

if (!tryParseTimeString(
str,
len,
pos,
microsSinceMidnight,
TimestampParseMode::kPrestoCast)) {
VELOX_USER_FAIL(
"Unable to parse time value: \"{}\", "
"expected format is (HH:MM:SS[.MS])",
std::string_view(str, len));
}

// Check remaining string for non-space characters.
skipSpaces(str, len, pos);

// Check position. If end was not reached, non-space chars remaining.
VELOX_USER_CHECK_EQ(
pos,
len,
"Unable to parse time value: \"{}\", "
"expected format is (HH:MM:SS[.MS])",
std::string_view(str, len));

return microsSinceMidnight;
}

Timestamp fromDatetime(int64_t daysSinceEpoch, int64_t microsSinceMidnight) {
int64_t secondsSinceEpoch =
static_cast<int64_t>(daysSinceEpoch) * kSecsPerDay;
Expand All @@ -793,7 +763,7 @@ Status parserError(const char* str, size_t len) {

Expected<Timestamp>
fromTimestampString(const char* str, size_t len, TimestampParseMode parseMode) {
size_t pos;
size_t pos = 0;
Timestamp resultTimestamp;

if (!tryParseTimestampString(str, len, pos, resultTimestamp, parseMode)) {
Expand Down
12 changes: 0 additions & 12 deletions velox/type/TimestampConversion.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,18 +132,6 @@ int32_t extractISODayOfTheWeek(int32_t daysSinceEpoch);
int64_t
fromTime(int32_t hour, int32_t minute, int32_t second, int32_t microseconds);

// TODO These are used only in tests. Move them to test-only location.

/// Parses the input string and returns the number of cumulative microseconds,
/// following the "HH:MM[:SS[.MS]]" format (ISO 8601).
//
/// Throws VeloxUserError if the format or time is invalid.
int64_t fromTimeString(const char* buf, size_t len);

inline int64_t fromTimeString(const StringView& str) {
return fromTimeString(str.data(), str.size());
}

// Timestamp conversion

enum class TimestampParseMode {
Expand Down
43 changes: 0 additions & 43 deletions velox/type/tests/TimestampConversionTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,49 +188,6 @@ TEST(DateTimeUtilTest, fromDateStringInvalid) {
testCastFromDateStringInvalid("1970-01-01T01:00:47.000", ParseMode::kIso8601);
}

TEST(DateTimeUtilTest, fromTimeString) {
EXPECT_EQ(0, fromTimeString("00:00:00"));
EXPECT_EQ(0, fromTimeString("00:00:00.00"));
EXPECT_EQ(1, fromTimeString("00:00:00.000001"));
EXPECT_EQ(10, fromTimeString("00:00:00.00001"));
EXPECT_EQ(100, fromTimeString("00:00:00.0001"));
EXPECT_EQ(1000, fromTimeString("00:00:00.001"));
EXPECT_EQ(10000, fromTimeString("00:00:00.01"));
EXPECT_EQ(100000, fromTimeString("00:00:00.1"));
EXPECT_EQ(1'000'000, fromTimeString("00:00:01"));
EXPECT_EQ(60'000'000, fromTimeString("00:01:00"));
EXPECT_EQ(3'600'000'000, fromTimeString("01:00:00"));

// 1 day minus 1 second.
EXPECT_EQ(86'399'000'000, fromTimeString("23:59:59"));

// Single digit.
EXPECT_EQ(0, fromTimeString("0:0:0.0"));
EXPECT_EQ(3'661'000'000, fromTimeString("1:1:1"));

// Leading and trailing spaces.
EXPECT_EQ(0, fromTimeString(" \t \n 00:00:00.00 \t"));
}

TEST(DateTimeUtilTest, fromTimeStrInvalid) {
const std::string errorMsg = "Unable to parse time value: ";
VELOX_ASSERT_THROW(fromTimeString(""), errorMsg);
VELOX_ASSERT_THROW(fromTimeString("00"), errorMsg);
VELOX_ASSERT_THROW(fromTimeString("00:"), errorMsg);
VELOX_ASSERT_THROW(fromTimeString("00:00:"), errorMsg);
VELOX_ASSERT_THROW(fromTimeString("00:00:00."), errorMsg);
VELOX_ASSERT_THROW(fromTimeString("00:00-00"), errorMsg);
VELOX_ASSERT_THROW(fromTimeString("00/00:00"), errorMsg);

// Invalid hour, minutes and seconds.
VELOX_ASSERT_THROW(fromTimeString("24:00:00"), errorMsg);
VELOX_ASSERT_THROW(fromTimeString("00:61:00"), errorMsg);
VELOX_ASSERT_THROW(fromTimeString("00:00:61"), errorMsg);

// Trailing characters.
VELOX_ASSERT_THROW(fromTimeString("00:00:00 12"), errorMsg);
}

// bash command to verify:
// $ date -d "2000-01-01 12:21:56Z" +%s
// ('Z' at the end means UTC).
Expand Down

0 comments on commit b3f53f6

Please sign in to comment.