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.

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

Fix/twice timezone conversion #4448

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 11 additions & 2 deletions src/common/function/FunctionManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1712,9 +1712,14 @@ FunctionManager::FunctionManager() {
if (args[0].get().isStr()) {
auto result = time::TimeUtils::parseTime(args[0].get().getStr());
if (!result.ok()) {
DLOG(ERROR) << "DEBUG POINT: " << result.status();
return Value::kNullBadData;
}
return time::TimeUtils::timeToUTC(result.value());
if (result.value().withTimeZone) {
return result.value().t;
} else {
return time::TimeUtils::timeToUTC(result.value().t);
}
} else if (args[0].get().isMap()) {
auto result = time::TimeUtils::timeFromMap(args[0].get().getMap());
if (!result.ok()) {
Expand Down Expand Up @@ -1749,7 +1754,11 @@ FunctionManager::FunctionManager() {
if (!result.ok()) {
return Value::kNullBadData;
}
return time::TimeUtils::dateTimeToUTC(result.value());
if (result.value().withTimeZone) {
return result.value().dt;
} else {
return time::TimeUtils::dateTimeToUTC(result.value().dt);
}
} else if (args[0].get().isMap()) {
auto result = time::TimeUtils::dateTimeFromMap(args[0].get().getMap());
if (!result.ok()) {
Expand Down
18 changes: 18 additions & 0 deletions src/common/function/test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,24 @@ nebula_add_test(
gtest_main
)

nebula_add_test(
NAME
twice_timezone_conversion_test
SOURCES
TwiceTimezoneConversionTest.cpp
OBJECTS
$<TARGET_OBJECTS:function_manager_obj>
$<TARGET_OBJECTS:wkt_wkb_io_obj>
$<TARGET_OBJECTS:base_obj>
$<TARGET_OBJECTS:datatypes_obj>
$<TARGET_OBJECTS:time_utils_obj>
$<TARGET_OBJECTS:datetime_parser_obj>
$<TARGET_OBJECTS:time_obj>
$<TARGET_OBJECTS:fs_obj>
LIBRARIES
gtest
)

nebula_add_test(
NAME
agg_function_manager_test
Expand Down
62 changes: 62 additions & 0 deletions src/common/function/test/TwiceTimezoneConversionTest.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
// Copyright (c) 2022 vesoft inc. All rights reserved.
//
// This source code is licensed under Apache 2.0 License.

#include <gtest/gtest.h>

#include "common/datatypes/Date.h"
#include "common/function/FunctionManager.h"
#include "common/time/TimezoneInfo.h"

DECLARE_string(timezone_name);

// #4432
TEST(DatetimeFunction, TwiceTimezoneConversion) {
// date
{
auto result = nebula::FunctionManager::get("date", 1);
ASSERT_TRUE(result.ok());
nebula::Value arg = nebula::Value("2019-01-01");
auto res = std::move(result).value()({arg});
ASSERT_EQ(res.type(), nebula::Value::Type::DATE);
EXPECT_EQ(res.getDate(), nebula::Date(2019, 1, 1));
}

// time
{
auto result = nebula::FunctionManager::get("time", 1);
ASSERT_TRUE(result.ok());
nebula::Value arg = nebula::Value("23:04:05+08:00");
auto res = std::move(result).value()({arg});
ASSERT_EQ(res.type(), nebula::Value::Type::TIME);
EXPECT_EQ(res.getTime(), nebula::Time(15, 4, 5, 0));
}

// datetime
{
auto result = nebula::FunctionManager::get("datetime", 1);
ASSERT_TRUE(result.ok());
nebula::Value arg = nebula::Value("2019-01-01T23:04:05+08:00");
auto res = std::move(result).value()({arg});
ASSERT_EQ(res.type(), nebula::Value::Type::DATETIME);
EXPECT_EQ(res.getDateTime(), nebula::DateTime(2019, 1, 1, 15, 4, 5, 0));
}
}

int main(int argc, char **argv) {
testing::InitGoogleTest(&argc, argv);
folly::init(&argc, &argv, true);
google::SetStderrLogging(google::INFO);

FLAGS_timezone_name = "UTC+08:00";

auto result = nebula::time::Timezone::initializeGlobalTimezone();
if (!result.ok()) {
LOG(FATAL) << result;
}

DLOG(INFO) << "Timezone: " << nebula::time::Timezone::getGlobalTimezone().stdZoneName();
DLOG(INFO) << "Timezone offset: " << nebula::time::Timezone::getGlobalTimezone().utcOffsetSecs();

return RUN_ALL_TESTS();
}
7 changes: 4 additions & 3 deletions src/common/time/TimeUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,8 @@ StatusOr<Value> TimeUtils::toTimestamp(const Value &val) {
if (!status.ok()) {
return status.status();
}
auto dateTime = std::move(status).value();
auto result = std::move(status).value();
auto dateTime = result.withTimeZone ? result.dt : dateTimeToUTC(result.dt);
if (dateTime.microsec != 0) {
return Status::Error("The timestamp only supports seconds unit.");
}
Expand Down Expand Up @@ -206,7 +207,7 @@ StatusOr<Value> TimeUtils::toTimestamp(const Value &val) {
return d;
}

/*static*/ StatusOr<DateTime> TimeUtils::parseDateTime(const std::string &str) {
/*static*/ StatusOr<Result> TimeUtils::parseDateTime(const std::string &str) {
auto p = DatetimeReader();
auto result = p.readDatetime(str);
NG_RETURN_IF_ERROR(result);
Expand All @@ -220,7 +221,7 @@ StatusOr<Value> TimeUtils::toTimestamp(const Value &val) {
return result.value();
}

/*static*/ StatusOr<Time> TimeUtils::parseTime(const std::string &str) {
/*static*/ StatusOr<TimeResult> TimeUtils::parseTime(const std::string &str) {
auto p = DatetimeReader();
return p.readTime(str);
}
Expand Down
5 changes: 3 additions & 2 deletions src/common/time/TimeUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "common/time/TimeConversion.h"
#include "common/time/TimezoneInfo.h"
#include "common/time/WallClock.h"
#include "common/time/parser/Result.h"

namespace nebula {
namespace time {
Expand Down Expand Up @@ -66,7 +67,7 @@ class TimeUtils {
return Status::OK();
}

static StatusOr<DateTime> parseDateTime(const std::string &str);
static StatusOr<Result> parseDateTime(const std::string &str);

static StatusOr<DateTime> dateTimeFromMap(const Map &m);

Expand Down Expand Up @@ -153,7 +154,7 @@ class TimeUtils {

static StatusOr<Time> timeFromMap(const Map &m);

static StatusOr<Time> parseTime(const std::string &str);
static StatusOr<TimeResult> parseTime(const std::string &str);

// utc + offset = local
static Time timeToUTC(const Time &time) {
Expand Down
16 changes: 8 additions & 8 deletions src/common/time/parser/DatetimeReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
namespace nebula {
namespace time {

DatetimeReader::DatetimeReader() : parser_(scanner_, error_, &dt_) {
DatetimeReader::DatetimeReader() : parser_(scanner_, error_, &result_) {
// Callback invoked by DatetimeScanner
auto readBuffer = [this](char *buf, int maxSize) -> int {
// Reach the end
Expand All @@ -26,7 +26,7 @@ DatetimeReader::DatetimeReader() : parser_(scanner_, error_, &dt_) {
scanner_.setReadBuffer(std::move(readBuffer));
}

StatusOr<DateTime> DatetimeReader::read(std::string input) {
StatusOr<Result> DatetimeReader::read(std::string input) {
// Since DatetimeScanner needs a writable buffer, we have to copy the query string
buffer_ = std::move(input);
pos_ = &buffer_[0];
Expand All @@ -38,19 +38,19 @@ StatusOr<DateTime> DatetimeReader::read(std::string input) {
end_ = nullptr;
// To flush the internal buffer to recover from a failure
scanner_.flushBuffer();
if (dt_ != nullptr) {
delete dt_;
dt_ = nullptr;
if (result_ != nullptr) {
delete result_;
result_ = nullptr;
}
scanner_.setInput(nullptr);
return Status::SyntaxError(error_);
}

if (dt_ == nullptr) {
if (result_ == nullptr) {
return Status::StatementEmpty(); // empty
}
auto dt = dt_;
dt_ = nullptr;
auto dt = result_;
result_ = nullptr;
scanner_.setInput(nullptr);
auto tmp = std::move(*dt);
delete dt;
Expand Down
13 changes: 7 additions & 6 deletions src/common/time/parser/DatetimeReader.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "common/datatypes/Date.h"
#include "common/time/parser/DatetimeParser.hpp"
#include "common/time/parser/DatetimeScanner.h"
#include "common/time/parser/Result.h"

namespace nebula {
namespace time {
Expand All @@ -20,10 +21,10 @@ class DatetimeReader {
DatetimeReader();

~DatetimeReader() {
if (dt_ != nullptr) delete dt_;
if (result_ != nullptr) delete result_;
}

StatusOr<DateTime> readDatetime(std::string input) {
StatusOr<Result> readDatetime(std::string input) {
input = kDatetimePrefix + input;
return read(std::move(input));
}
Expand All @@ -32,26 +33,26 @@ class DatetimeReader {
input = kDatePrefix + input;
auto result = read(std::move(input));
NG_RETURN_IF_ERROR(result);
return result.value().date();
return result.value().dt.date();
}

StatusOr<Time> readTime(std::string input) {
StatusOr<TimeResult> readTime(std::string input) {
input = kTimePrefix + input;
auto result = read(std::move(input));
NG_RETURN_IF_ERROR(result);
return result.value().time();
}

private:
StatusOr<DateTime> read(std::string input);
StatusOr<Result> read(std::string input);

std::string buffer_;
const char *pos_{nullptr};
const char *end_{nullptr};
DatetimeScanner scanner_;
DatetimeParser parser_;
std::string error_;
DateTime *dt_{nullptr};
Result *result_{nullptr};

inline static const std::string kDatetimePrefix = "DATETIME__";
inline static const std::string kDatePrefix = "DATE__";
Expand Down
52 changes: 52 additions & 0 deletions src/common/time/parser/Result.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/* Copyright (c) 2022 vesoft inc. All rights reserved.
*
* This source code is licensed under Apache 2.0 License.
*/

#pragma once

#include "common/datatypes/Date.h"

namespace nebula {
namespace time {

struct TimeResult {
Time t;
bool withTimeZone{false};
};

static inline bool operator==(const TimeResult& lhs, const TimeResult& rhs) {
return lhs.t == rhs.t && lhs.withTimeZone == rhs.withTimeZone;
}

static inline std::ostream& operator<<(std::ostream& os, const TimeResult& result) {
os << result.t;
if (result.withTimeZone) {
os << "Z";
}
return os;
}

struct Result {
TimeResult time() const {
return TimeResult{dt.time(), withTimeZone};
}

DateTime dt;
bool withTimeZone{false};
};

static inline bool operator==(const Result& lhs, const Result& rhs) {
return lhs.dt == rhs.dt && lhs.withTimeZone == rhs.withTimeZone;
}

static inline std::ostream& operator<<(std::ostream& os, const Result& result) {
os << result.dt;
if (result.withTimeZone) {
os << "Z";
}
return os;
}

} // namespace time
} // namespace nebula
Loading