From 7e81a95b81b2f8d51f73cfea1687e09d7c24878a Mon Sep 17 00:00:00 2001 From: Kostas Kyrimis Date: Mon, 20 Sep 2021 17:53:42 +0300 Subject: [PATCH] Fix parsing negative durations (#237) --- src/utils/temporal.hpp | 8 +++-- tests/unit/query_dump.cpp | 6 +++- tests/unit/query_expression_evaluator.cpp | 13 ++++++++ tests/unit/query_pretty_print.cpp | 3 +- tests/unit/utils_temporal.cpp | 38 ++++++++++++++--------- 5 files changed, 49 insertions(+), 19 deletions(-) diff --git a/src/utils/temporal.hpp b/src/utils/temporal.hpp index 8a14d0dd0..90db6531d 100644 --- a/src/utils/temporal.hpp +++ b/src/utils/temporal.hpp @@ -77,14 +77,18 @@ struct Duration { int64_t SubSecondsAsNanoseconds() const; friend std::ostream &operator<<(std::ostream &os, const Duration &dur) { - // Format [DD]T[hh]:[mm]:[ss]. + // Format [nD]T[nH]:[nM]:[nS]. namespace chrono = std::chrono; auto micros = chrono::microseconds(dur.microseconds); const auto dd = GetAndSubtractDuration(micros); const auto h = GetAndSubtractDuration(micros); const auto m = GetAndSubtractDuration(micros); const auto s = GetAndSubtractDuration(micros); - return os << fmt::format("P{:0>9}DT{:0>2}H{:0>2}M{:0>2}.{:0>6}S", dd, h, m, s, micros.count()); + os << fmt::format("P{}DT{}H{}M", dd, h, m); + if (s == 0 && micros.count() < 0) { + os << '-'; + } + return os << fmt::format("{}.{:0>6}S", s, std::abs(micros.count())); } Duration operator-() const; diff --git a/tests/unit/query_dump.cpp b/tests/unit/query_dump.cpp index 3821e1c7e..edce9f9f2 100644 --- a/tests/unit/query_dump.cpp +++ b/tests/unit/query_dump.cpp @@ -415,7 +415,7 @@ TEST(DumpTest, PropertyValue) { "CREATE (:__mg_vertex__ {__mg_id__: 0, `p1`: [{`prop 1`: 13, " "`prop``2```: true}, Null, -1.2, DATE(\"1994-12-07\"), " "LOCALTIME(\"14:10:44.099099\"), LOCALDATETIME(\"1994-12-07T14:10:44.099099\"), " - "DURATION(\"P000000003DT04H05M06.010011S\")" + "DURATION(\"P3DT4H5M6.010011S\")" "], `p2`: \"hello \\'world\\'\"});", kDropInternalIndex, kRemoveInternalLabelProperty); } @@ -661,6 +661,10 @@ TEST(DumpTest, CheckStateSimpleGraph) { CreateEdge(&dba, &w, &z, "Duration", {{"time", storage::PropertyValue(storage::TemporalData( storage::TemporalType::Duration, utils::Duration({3, 4, 5, 6, 10, 11}).microseconds))}}); + CreateEdge( + &dba, &w, &z, "NegativeDuration", + {{"time", storage::PropertyValue(storage::TemporalData( + storage::TemporalType::Duration, utils::Duration({-3, -4, -5, -6, -10, -11}).microseconds))}}); ASSERT_FALSE(dba.Commit().HasError()); } { diff --git a/tests/unit/query_expression_evaluator.cpp b/tests/unit/query_expression_evaluator.cpp index 9968ffb03..381ad6c32 100644 --- a/tests/unit/query_expression_evaluator.cpp +++ b/tests/unit/query_expression_evaluator.cpp @@ -1984,5 +1984,18 @@ TEST_F(FunctionTest, Duration) { EXPECT_THROW( EvaluateFunction("DURATION", TypedValue(std::map{{"seconds", TypedValue(1970)}})), QueryRuntimeException); + + const auto map_param_negative = TypedValue(std::map{{"day", TypedValue(-3)}, + {"hour", TypedValue(-4)}, + {"minute", TypedValue(-5)}, + {"second", TypedValue(-6)}, + {"millisecond", TypedValue(-7)}, + {"microsecond", TypedValue(-8)}}); + EXPECT_EQ(EvaluateFunction("DURATION", map_param_negative).ValueDuration(), + utils::Duration({-3, -4, -5, -6, -7, -8})); + + EXPECT_EQ(EvaluateFunction("DURATION", "P4DT4H5M6.2S").ValueDuration(), utils::Duration({4, 4, 5, 6, 0, 200000})); + EXPECT_EQ(EvaluateFunction("DURATION", "P3DT4H5M6.100S").ValueDuration(), utils::Duration({3, 4, 5, 6, 0, 100000})); + EXPECT_EQ(EvaluateFunction("DURATION", "P3DT4H5M6.100110S").ValueDuration(), utils::Duration({3, 4, 5, 6, 100, 110})); } } // namespace diff --git a/tests/unit/query_pretty_print.cpp b/tests/unit/query_pretty_print.cpp index f16131675..8306ebd15 100644 --- a/tests/unit/query_pretty_print.cpp +++ b/tests/unit/query_pretty_print.cpp @@ -50,11 +50,12 @@ TEST_F(ExpressionPrettyPrinterTest, Literals) { std::vector tt_vec{ storage::PropertyValue(storage::TemporalData(storage::TemporalType::Duration, 1)), + storage::PropertyValue(storage::TemporalData(storage::TemporalType::Duration, -2)), storage::PropertyValue(storage::TemporalData(storage::TemporalType::LocalTime, 2)), storage::PropertyValue(storage::TemporalData(storage::TemporalType::LocalDateTime, 3)), storage::PropertyValue(storage::TemporalData(storage::TemporalType::Date, 4))}; EXPECT_EQ(ToString(LITERAL(storage::PropertyValue(tt_vec))), - "[DURATION(\"P000000000DT00H00M00.000001S\"), LOCALTIME(\"00:00:00.000002\"), " + "[DURATION(\"P0DT0H0M0.000001S\"), DURATION(\"P0DT0H0M-0.000002S\"), LOCALTIME(\"00:00:00.000002\"), " "LOCALDATETIME(\"1970-01-01T00:00:00.000003\"), DATE(\"1970-01-01\")]"); } diff --git a/tests/unit/utils_temporal.cpp b/tests/unit/utils_temporal.cpp index 81a2acf44..0561eb20f 100644 --- a/tests/unit/utils_temporal.cpp +++ b/tests/unit/utils_temporal.cpp @@ -277,14 +277,27 @@ TEST(TemporalTest, DurationParsing) { ASSERT_THROW(utils::ParseDurationParameters("PT2M3S32"), utils::BasicException); ASSERT_THROW(utils::ParseDurationParameters("PT2.5M3S"), utils::BasicException); ASSERT_THROW(utils::ParseDurationParameters("PT2.5M3.5S"), utils::BasicException); + ASSERT_THROW(utils::ParseDurationParameters("PT2.5M3.-5S"), utils::BasicException); CheckDurationParameters(utils::ParseDurationParameters("P1256.5D"), utils::DurationParameters{1256.5}); CheckDurationParameters(utils::ParseDurationParameters("P1222DT2H"), utils::DurationParameters{1222, 2}); CheckDurationParameters(utils::ParseDurationParameters("P1222DT2H44M"), utils::DurationParameters{1222, 2, 44}); CheckDurationParameters(utils::ParseDurationParameters("P22DT1H9M20S"), utils::DurationParameters{22, 1, 9, 20}); - CheckDurationParameters(utils::ParseDurationParameters("P22DT1H9M20.100S"), - utils::DurationParameters{22, 1, 9, 20.100, 0, 0}); - CheckDurationParameters(utils::ParseDurationParameters("P22DT1H9M20.1000S"), - utils::DurationParameters{22, 1, 9, 20.100, 0, 0}); + CheckDurationParameters(utils::ParseDurationParameters("P22DT1H9M20.100S"), utils::DurationParameters{ + 22, + 1, + 9, + 20.1, + }); + CheckDurationParameters(utils::ParseDurationParameters("P-22222222DT1H9M20.100S"), + utils::DurationParameters{-22222222, 1, 9, 20.1}); + CheckDurationParameters(utils::ParseDurationParameters("P-22222222DT-10H8M21.200S"), + utils::DurationParameters{-22222222, -10, 8, 21.2}); + CheckDurationParameters(utils::ParseDurationParameters("P-22222222DT-1H-7M22.300S"), + utils::DurationParameters{-22222222, -1, -7, 22.3}); + CheckDurationParameters(utils::ParseDurationParameters("P-22222222DT-1H-6M-20.100S"), + utils::DurationParameters{-22222222, -1, -6, -20.1}); + CheckDurationParameters(utils::ParseDurationParameters("P-22222222DT-1H-5M-20.100S"), + utils::DurationParameters{-22222222, -1, -5, -20.1}); } TEST(TemporalTest, PrintDate) { @@ -308,24 +321,19 @@ TEST(TemporalTest, PrintDuration) { std::ostringstream stream; stream << dur; ASSERT_TRUE(stream); - ASSERT_EQ(stream.view(), "P000000001DT00H00M00.000000S"); + ASSERT_EQ(stream.view(), "P1DT0H0M0.000000S"); stream.str(""); stream.clear(); const auto complex_dur = utils::Duration({10, 3, 30, 33, 100, 50}); stream << complex_dur; ASSERT_TRUE(stream); - ASSERT_EQ(stream.view(), "P000000010DT03H30M33.100050S"); - /// stream.str(""); - /// stream.clear(); - /// TODO (kostasrim) - /// We do not support pasring negative Durations yet. We are the only ones we have access - /// to the constructor below. - /* - const auto negative_dur = utils::Duration({-1, 5, -10, -3, -30, -33}); + ASSERT_EQ(stream.view(), "P10DT3H30M33.100050S"); + stream.str(""); + stream.clear(); + const auto negative_dur = utils::Duration({-10, -3, -30, -33, -100, -50}); stream << negative_dur; ASSERT_TRUE(stream); - ASSERT_EQ(stream.view(), "P0001-05-10T03:30:33.000000"); - */ + ASSERT_EQ(stream.view(), "P-10DT-3H-30M-33.100050S"); } TEST(TemporalTest, PrintLocalDateTime) {