Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add unittests for str_to_date, fix #3556, #3557 (#3581) #3933

Merged
merged 7 commits into from
Apr 14, 2022
Merged

Add unittests for str_to_date, fix #3556, #3557 (#3581) #3933

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
033cfd0
This is an automated cherry-pick of #3581
wshwsh12 Dec 14, 2021
f9bda39
resolve conflict
wshwsh12 Apr 14, 2022
5bcaa53
Merge branch 'release-5.2' into cherry-pick-3581-to-release-5.2
wshwsh12 Apr 14, 2022
8d48371
format files
wshwsh12 Apr 14, 2022
c48a9af
Merge branch 'cherry-pick-3581-to-release-5.2' of github.com:ti-chi-b…
wshwsh12 Apr 14, 2022
a0c344b
Merge branch 'release-5.2' into cherry-pick-3581-to-release-5.2
ti-chi-bot Apr 14, 2022
b967209
Merge branch 'release-5.2' into cherry-pick-3581-to-release-5.2
ti-chi-bot Apr 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 28 additions & 41 deletions dbms/src/Common/MyTime.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ std::vector<String> parseDateFormat(String format)
{
format = Poco::trimInPlace(format);

if (format.size() == 0)
if (format.empty())
return {};

if (!std::isdigit(format[0]) || !std::isdigit(format[format.size() - 1]))
Expand Down Expand Up @@ -531,7 +531,7 @@ Field parseMyDateTime(const String & str, int8_t fsp)
{
// if tz_sign is empty, it's sure that the string literal contains timezone (e.g., 2010-10-10T10:10:10Z),
// therefore we could safely skip this branch.
if (!noAbsorb(seps) && !(tz_minute != "" && tz_sep == ""))
if (!noAbsorb(seps) && !(!tz_minute.empty() && tz_sep.empty()))
{
// we can't absorb timezone if there is no separate between tz_hour and tz_minute
if (!tz_hour.empty())
Expand Down Expand Up @@ -853,9 +853,8 @@ size_t maxFormattedDateTimeStringLength(const String & format)
{
size_t result = 0;
bool in_pattern_match = false;
for (size_t i = 0; i < format.size(); i++)
for (char x : format)
{
char x = format[i];
if (in_pattern_match)
{
switch (x)
Expand Down Expand Up @@ -968,7 +967,6 @@ void MyTimeBase::check(bool allow_zero_in_date, bool allow_invalid_date) const
{
throw TiFlashException("Incorrect datetime value", Errors::Types::WrongValue);
}
return;
}

bool toCoreTimeChecked(const UInt64 & year, const UInt64 & month, const UInt64 & day, const UInt64 & hour, const UInt64 & minute,
Expand All @@ -989,9 +987,8 @@ bool toCoreTimeChecked(const UInt64 & year, const UInt64 & month, const UInt64 &
MyDateTimeFormatter::MyDateTimeFormatter(const String & layout)
{
bool in_pattern_match = false;
for (size_t i = 0; i < layout.size(); i++)
for (char x : layout)
{
char x = layout[i];
if (in_pattern_match)
{
switch (x)
Expand Down Expand Up @@ -1226,7 +1223,7 @@ struct MyDateTimeParser::Context
// The pos we are parsing from
size_t pos = 0;

Context(StringRef view_) : view(std::move(view_)) {}
explicit Context(StringRef view_) : view(std::move(view_)) {}
};

// Try to parse digits with number of `limit` starting from view[pos]
Expand Down Expand Up @@ -1269,18 +1266,18 @@ static bool parseTime12Hour(MyDateTimeParser::Context & ctx, MyTimeBase & time)
{
// Use temp_pos instead of changing `ctx.pos` directly in case of parsing failure
size_t temp_pos = ctx.pos;
auto checkIfEnd = [&temp_pos, &ctx]() -> ParseState {
auto check_if_end = [&temp_pos, &ctx]() -> ParseState {
// To the end
if (temp_pos == ctx.view.size)
return ParseState::END_OF_FILE;
return ParseState::NORMAL;
};
auto skipWhitespaces = [&temp_pos, &ctx, &checkIfEnd]() -> ParseState {
auto skipWhitespaces = [&temp_pos, &ctx, &check_if_end]() -> ParseState {
while (temp_pos < ctx.view.size && isWhitespaceASCII(ctx.view.data[temp_pos]))
++temp_pos;
return checkIfEnd();
return check_if_end();
};
auto parseSep = [&temp_pos, &ctx, &skipWhitespaces]() -> ParseState {
auto parse_sep = [&temp_pos, &ctx, &skipWhitespaces]() -> ParseState {
if (skipWhitespaces() == ParseState::END_OF_FILE)
return ParseState::END_OF_FILE;
// parse ":"
Expand All @@ -1289,7 +1286,7 @@ static bool parseTime12Hour(MyDateTimeParser::Context & ctx, MyTimeBase & time)
temp_pos += 1; // move forward
return ParseState::NORMAL;
};
auto tryParse = [&]() -> ParseState {
auto try_parse = [&]() -> ParseState {
ParseState state = ParseState::NORMAL;
/// Note that we should update `time` as soon as possible, or we
/// can not get correct result for incomplete input like "12:13"
Expand All @@ -1310,7 +1307,7 @@ static bool parseTime12Hour(MyDateTimeParser::Context & ctx, MyTimeBase & time)
time.hour = hour;
temp_pos += step; // move forward

if (state = parseSep(); state != ParseState::NORMAL)
if (state = parse_sep(); state != ParseState::NORMAL)
return state;

int32_t minute = 0;
Expand All @@ -1322,7 +1319,7 @@ static bool parseTime12Hour(MyDateTimeParser::Context & ctx, MyTimeBase & time)
time.minute = minute;
temp_pos += step; // move forward

if (state = parseSep(); state != ParseState::NORMAL)
if (state = parse_sep(); state != ParseState::NORMAL)
return state;

int32_t second = 0;
Expand Down Expand Up @@ -1361,7 +1358,7 @@ static bool parseTime12Hour(MyDateTimeParser::Context & ctx, MyTimeBase & time)
temp_pos += 2; // move forward
return ParseState::NORMAL;
};
if (auto state = tryParse(); state == ParseState::FAIL)
if (auto state = try_parse(); state == ParseState::FAIL)
return false;
// Other state, forward the `ctx.pos` and return true
ctx.pos = temp_pos;
Expand All @@ -1373,18 +1370,18 @@ static bool parseTime24Hour(MyDateTimeParser::Context & ctx, MyTimeBase & time)
{
// Use temp_pos instead of changing `ctx.pos` directly in case of parsing failure
size_t temp_pos = ctx.pos;
auto checkIfEnd = [&temp_pos, &ctx]() -> ParseState {
auto check_if_end = [&temp_pos, &ctx]() -> ParseState {
// To the end
if (temp_pos == ctx.view.size)
return ParseState::END_OF_FILE;
return ParseState::NORMAL;
};
auto skipWhitespaces = [&temp_pos, &ctx, &checkIfEnd]() -> ParseState {
auto skipWhitespaces = [&temp_pos, &ctx, &check_if_end]() -> ParseState {
while (temp_pos < ctx.view.size && isWhitespaceASCII(ctx.view.data[temp_pos]))
++temp_pos;
return checkIfEnd();
return check_if_end();
};
auto parseSep = [&temp_pos, &ctx, &skipWhitespaces]() -> ParseState {
auto parse_sep = [&temp_pos, &ctx, &skipWhitespaces]() -> ParseState {
if (skipWhitespaces() == ParseState::END_OF_FILE)
return ParseState::END_OF_FILE;
// parse ":"
Expand All @@ -1393,7 +1390,7 @@ static bool parseTime24Hour(MyDateTimeParser::Context & ctx, MyTimeBase & time)
temp_pos += 1; // move forward
return ParseState::NORMAL;
};
auto tryParse = [&]() -> ParseState {
auto try_parse = [&]() -> ParseState {
ParseState state = ParseState::NORMAL;
/// Note that we should update `time` as soon as possible, or we
/// can not get correct result for incomplete input like "12:13"
Expand All @@ -1410,7 +1407,7 @@ static bool parseTime24Hour(MyDateTimeParser::Context & ctx, MyTimeBase & time)
time.hour = hour;
temp_pos += step; // move forward

if (state = parseSep(); state != ParseState::NORMAL)
if (state = parse_sep(); state != ParseState::NORMAL)
return state;

int32_t minute = 0;
Expand All @@ -1422,7 +1419,7 @@ static bool parseTime24Hour(MyDateTimeParser::Context & ctx, MyTimeBase & time)
time.minute = minute;
temp_pos += step; // move forward

if (state = parseSep(); state != ParseState::NORMAL)
if (state = parse_sep(); state != ParseState::NORMAL)
return state;

int32_t second = 0;
Expand All @@ -1436,7 +1433,7 @@ static bool parseTime24Hour(MyDateTimeParser::Context & ctx, MyTimeBase & time)

return ParseState::NORMAL;
};
if (auto state = tryParse(); state == ParseState::FAIL)
if (auto state = try_parse(); state == ParseState::FAIL)
return false;
// Other state, forward the `ctx.pos` and return true
ctx.pos = temp_pos;
Expand Down Expand Up @@ -1481,6 +1478,9 @@ MyDateTimeParser::MyDateTimeParser(String format_) : format(std::move(format_))
});
break;
}
case 'm':
//"%m": Month, numeric (00..12)
[[fallthrough]];
case 'c':
{
//"%c": Month, numeric (0..12)
Expand Down Expand Up @@ -1522,9 +1522,9 @@ MyDateTimeParser::MyDateTimeParser(String format_) : format(std::move(format_))
time.micro_second = 0;
return true;
}
// The siffix '0' can be ignored.
// The suffix '0' can be ignored.
// "9" means 900000
while (ms > 0 && ms * 10 < 1000000)
for (size_t i = step; i < 6; i++)
{
ms *= 10;
}
Expand Down Expand Up @@ -1620,19 +1620,6 @@ MyDateTimeParser::MyDateTimeParser(String format_) : format(std::move(format_))
});
break;
}
case 'm':
{
//"%m": Month, numeric (00..12)
parsers.emplace_back([](MyDateTimeParser::Context & ctx, MyTimeBase & time) -> bool {
auto [step, month] = parseNDigits(ctx.view, ctx.pos, 2);
if (step == 0 || month > 12)
return false;
time.month = month;
ctx.pos += step;
return true;
});
break;
}
case 'S':
//"%S": Seconds (00..59)
[[fallthrough]];
Expand Down Expand Up @@ -1879,7 +1866,7 @@ std::optional<UInt64> MyDateTimeParser::parseAsPackedUInt(const StringRef & str_
MyDateTimeParser::Context ctx(str_view);

// TODO: can we return warnings to TiDB?
for (auto & f : parsers)
for (const auto & f : parsers)
{
// Ignore all prefix white spaces before each pattern match (TODO: handle unicode space?)
while (ctx.pos < str_view.size && isWhitespaceASCII(str_view.data[ctx.pos]))
Expand All @@ -1888,7 +1875,7 @@ std::optional<UInt64> MyDateTimeParser::parseAsPackedUInt(const StringRef & str_
if (ctx.pos == ctx.view.size)
break;

if (f(ctx, my_time) != true)
if (!f(ctx, my_time))
{
#ifndef NDEBUG
LOG_TRACE(&Logger::get("MyDateTimeParser"),
Expand Down
89 changes: 89 additions & 0 deletions dbms/src/Common/tests/gtest_mytime.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,96 @@ try
// {"Tuesday 52 2001", "%W %V %Y", std::nullopt}, //
// {"Tuesday 52 2001", "%W %V %x", std::nullopt}, //
// {"Tuesday 52 2001", "%W %u %x", std::nullopt}, //

// Test cases for %b
{"10/JAN/2010", "%d/%b/%Y", MyDateTime{2010, 1, 10, 0, 0, 0, 0}}, // Right spill, case-insensitive
{"10/FeB/2010", "%d/%b/%Y", MyDateTime{2010, 2, 10, 0, 0, 0, 0}},
{"10/MAr/2010", "%d/%b/%Y", MyDateTime{2010, 3, 10, 0, 0, 0, 0}},
{"10/ApR/2010", "%d/%b/%Y", MyDateTime{2010, 4, 10, 0, 0, 0, 0}},
{"10/mAY/2010", "%d/%b/%Y", MyDateTime{2010, 5, 10, 0, 0, 0, 0}},
{"10/JuN/2010", "%d/%b/%Y", MyDateTime{2010, 6, 10, 0, 0, 0, 0}},
{"10/JUL/2010", "%d/%b/%Y", MyDateTime{2010, 7, 10, 0, 0, 0, 0}},
{"10/Aug/2010", "%d/%b/%Y", MyDateTime{2010, 8, 10, 0, 0, 0, 0}},
{"10/seP/2010", "%d/%b/%Y", MyDateTime{2010, 9, 10, 0, 0, 0, 0}},
{"10/Oct/2010", "%d/%b/%Y", MyDateTime{2010, 10, 10, 0, 0, 0, 0}},
{"10/NOV/2010", "%d/%b/%Y", MyDateTime{2010, 11, 10, 0, 0, 0, 0}},
{"10/DEC/2010", "%d/%b/%Y", MyDateTime{2010, 12, 10, 0, 0, 0, 0}},
{"10/January/2010", "%d/%b/%Y", std::nullopt}, // Test full spilling

// Test cases for %M
{"10/January/2010", "%d/%M/%Y", MyDateTime{2010, 1, 10, 0, 0, 0, 0}}, // Test full spilling
{"10/February/2010", "%d/%M/%Y", MyDateTime{2010, 2, 10, 0, 0, 0, 0}},
{"10/March/2010", "%d/%M/%Y", MyDateTime{2010, 3, 10, 0, 0, 0, 0}},
{"10/April/2010", "%d/%M/%Y", MyDateTime{2010, 4, 10, 0, 0, 0, 0}},
{"10/May/2010", "%d/%M/%Y", MyDateTime{2010, 5, 10, 0, 0, 0, 0}},
{"10/June/2010", "%d/%M/%Y", MyDateTime{2010, 6, 10, 0, 0, 0, 0}},
{"10/July/2010", "%d/%M/%Y", MyDateTime{2010, 7, 10, 0, 0, 0, 0}},
{"10/August/2010", "%d/%M/%Y", MyDateTime{2010, 8, 10, 0, 0, 0, 0}},
{"10/September/2010", "%d/%M/%Y", MyDateTime{2010, 9, 10, 0, 0, 0, 0}},
{"10/October/2010", "%d/%M/%Y", MyDateTime{2010, 10, 10, 0, 0, 0, 0}},
{"10/November/2010", "%d/%M/%Y", MyDateTime{2010, 11, 10, 0, 0, 0, 0}},
{"10/December/2010", "%d/%M/%Y", MyDateTime{2010, 12, 10, 0, 0, 0, 0}},

// Test cases for %c
// {"10/0/2010", "%d/%c/%Y", MyDateTime{2010, 0, 10, 0, 0, 0, 0}}, // TODO: Need Check NO_ZERO_DATE
{"10/1/2010", "%d/%c/%Y", MyDateTime{2010, 1, 10, 0, 0, 0, 0}},
{"10/01/2010", "%d/%c/%Y", MyDateTime{2010, 1, 10, 0, 0, 0, 0}},
{"10/001/2010", "%d/%c/%Y", std::nullopt},
{"10/13/2010", "%d/%c/%Y", std::nullopt},
{"10/12/2010", "%d/%c/%Y", MyDateTime{2010, 12, 10, 0, 0, 0, 0}},

// Test cases for %d, %e
// {"0/12/2010", "%d/%c/%Y", MyDateTime{2010, 12, 0, 0, 0, 0, 0}}, // TODO: Need Check NO_ZERO_DATE
{"1/12/2010", "%d/%c/%Y", MyDateTime{2010, 12, 1, 0, 0, 0, 0}},
{"05/12/2010", "%d/%c/%Y", MyDateTime{2010, 12, 5, 0, 0, 0, 0}},
{"05/12/2010", "%e/%c/%Y", MyDateTime{2010, 12, 5, 0, 0, 0, 0}},
{"31/12/2010", "%d/%c/%Y", MyDateTime{2010, 12, 31, 0, 0, 0, 0}},
{"32/12/2010", "%d/%c/%Y", std::nullopt},
{"30/11/2010", "%d/%c/%Y", MyDateTime{2010, 11, 30, 0, 0, 0, 0}},
{"31/11/2010", "%e/%c/%Y", MyDateTime{2010, 11, 31, 0, 0, 0, 0}},
{"28/2/2010", "%e/%c/%Y", MyDateTime{2010, 2, 28, 0, 0, 0, 0}},
{"29/2/2010", "%e/%c/%Y", MyDateTime{2010, 2, 29, 0, 0, 0, 0}},
{"29/2/2020", "%e/%c/%Y", MyDateTime{2020, 2, 29, 0, 0, 0, 0}},

// Test cases for %Y
// {"1/12/0000", "%d/%c/%Y", MyDateTime{0000, 12, 1, 0, 0, 0, 0}}, // TODO: Need Check NO_ZERO_DATE
{"1/12/01", "%d/%c/%Y", MyDateTime{2001, 12, 1, 0, 0, 0, 0}},
{"1/12/0001", "%d/%c/%Y", MyDateTime{0001, 12, 1, 0, 0, 0, 0}},
{"1/12/2020", "%d/%c/%Y", MyDateTime{2020, 12, 1, 0, 0, 0, 0}},
{"1/12/9999", "%d/%c/%Y", MyDateTime{9999, 12, 1, 0, 0, 0, 0}},

// Test cases for %f
{"01,5,2013 999999", "%d,%c,%Y %f", MyDateTime{2013, 5, 1, 0, 0, 0, 999999}},
{"01,5,2013 0", "%d,%c,%Y %f", MyDateTime{2013, 5, 1, 0, 0, 0, 0}},
{"01,5,2013 9999990000000", "%d,%c,%Y %f", MyDateTime{2013, 5, 1, 0, 0, 0, 999999}},
{"01,5,2013 1", "%d,%c,%Y %f", MyDateTime{2013, 5, 1, 0, 0, 0, 100000}},
{"01,5,2013 1230", "%d,%c,%Y %f", MyDateTime{2013, 5, 1, 0, 0, 0, 123000}},
{"01,5,2013 01", "%d,%c,%Y %f", MyDateTime{2013, 5, 1, 0, 0, 0, 10000}}, // issue 3556

// Test cases for %h, %I, %l
{"00:11:12 ", "%h:%i:%S ", std::nullopt}, // 0 is not a valid number of %h
{"01:11:12 ", "%I:%i:%S ", MyDateTime{0, 0, 0, 01, 11, 12, 0}},
{"12:11:12 ", "%l:%i:%S ", MyDateTime{0, 0, 0, 0, 11, 12, 0}},

// Test cases for %k, %H
{"00:11:12 ", "%H:%i:%S ", MyDateTime{0, 0, 0, 00, 11, 12, 0}},
{"01:11:12 ", "%k:%i:%S ", MyDateTime{0, 0, 0, 01, 11, 12, 0}},
{"12:11:12 ", "%H:%i:%S ", MyDateTime{0, 0, 0, 12, 11, 12, 0}},
{"24:11:12 ", "%k:%i:%S ", std::nullopt},

// Test cases for %i
{"00:00:12 ", "%H:%i:%S ", MyDateTime{0, 0, 0, 00, 00, 12, 0}},
{"00:01:12 ", "%H:%i:%S ", MyDateTime{0, 0, 0, 00, 01, 12, 0}},
{"00:59:12 ", "%H:%i:%S ", MyDateTime{0, 0, 0, 00, 59, 12, 0}},
{"00:60:12 ", "%H:%i:%S ", std::nullopt},

// Test cases for %s, %S
{"00:00:00 ", "%H:%i:%s ", MyDateTime{0, 0, 0, 00, 00, 00, 0}},
{"00:01:01 ", "%H:%i:%s ", MyDateTime{0, 0, 0, 00, 01, 01, 0}},
{"00:59:59 ", "%H:%i:%S ", MyDateTime{0, 0, 0, 00, 59, 59, 0}},
{"00:59:60 ", "%H:%i:%S ", std::nullopt},
};

auto result_formatter = MyDateTimeFormatter("%Y/%m/%d %T.%f");
size_t idx = 0;
for (const auto & [input, fmt, expected] : cases)
Expand Down
Loading