From 4cd978ce65d93853963095d5f5b2f12e859966ae Mon Sep 17 00:00:00 2001 From: xufei Date: Thu, 17 Mar 2022 08:04:42 +0800 Subject: [PATCH 1/2] fix bug of handling zero date in dayofweek and dayofyear Signed-off-by: xufei --- dbms/src/Flash/Coprocessor/DAGUtils.cpp | 4 +- dbms/src/Functions/FunctionsDateTime.cpp | 2 + dbms/src/Functions/FunctionsDateTime.h | 69 +++++++++++++++++++ .../Functions/tests/gtest_dayofweekyear.cpp | 36 ++++++---- 4 files changed, 97 insertions(+), 14 deletions(-) diff --git a/dbms/src/Flash/Coprocessor/DAGUtils.cpp b/dbms/src/Flash/Coprocessor/DAGUtils.cpp index fa58b131e0f..7299246eba4 100644 --- a/dbms/src/Flash/Coprocessor/DAGUtils.cpp +++ b/dbms/src/Flash/Coprocessor/DAGUtils.cpp @@ -504,8 +504,8 @@ const std::unordered_map scalar_func_map({ {tipb::ScalarFuncSig::DayName, "toDayName"}, {tipb::ScalarFuncSig::DayOfMonth, "toDayOfMonth"}, - {tipb::ScalarFuncSig::DayOfWeek, "toDayOfWeek"}, - {tipb::ScalarFuncSig::DayOfYear, "toDayOfYear"}, + {tipb::ScalarFuncSig::DayOfWeek, "tidbDayOfWeek"}, + {tipb::ScalarFuncSig::DayOfYear, "tidbDayOfYear"}, //{tipb::ScalarFuncSig::WeekWithMode, "cast"}, //{tipb::ScalarFuncSig::WeekWithoutMode, "cast"}, diff --git a/dbms/src/Functions/FunctionsDateTime.cpp b/dbms/src/Functions/FunctionsDateTime.cpp index 930bc9b6bd9..ebfaf176945 100644 --- a/dbms/src/Functions/FunctionsDateTime.cpp +++ b/dbms/src/Functions/FunctionsDateTime.cpp @@ -134,6 +134,8 @@ void registerFunctionsDateTime(FunctionFactory & factory) factory.registerFunction(); factory.registerFunction(); factory.registerFunction(); + factory.registerFunction(); + factory.registerFunction(); factory.registerFunction(); factory.registerFunction(); diff --git a/dbms/src/Functions/FunctionsDateTime.h b/dbms/src/Functions/FunctionsDateTime.h index 04621fbd63a..8e1eb66b043 100644 --- a/dbms/src/Functions/FunctionsDateTime.h +++ b/dbms/src/Functions/FunctionsDateTime.h @@ -3314,6 +3314,73 @@ struct TiDBLastDayTransformerImpl } }; +template +struct TiDBDayOfWeekTransformerImpl +{ + static constexpr auto name = "tidbDayOfWeek"; + + static void execute(const Context & context, + const ColumnVector::Container & vec_from, + typename ColumnVector::Container & vec_to, + typename ColumnVector::Container & vec_null_map) + { + for (size_t i = 0; i < vec_from.size(); ++i) + { + bool is_null = false; + MyTimeBase val(vec_from[i]); + vec_to[i] = execute(context, val, is_null); + vec_null_map[i] = is_null; + } + } + + static ToFieldType execute(const Context & context, const MyTimeBase & val, bool & is_null) + { + // TiDB also considers NO_ZERO_DATE sql_mode. But sql_mode is not handled by TiFlash for now. + if (val.month == 0 || val.day == 0) + { + context.getDAGContext()->handleInvalidTime( + fmt::format("Invalid time value: month({}) or day({}) is zero", val.month, val.day), + Errors::Types::WrongValue); + is_null = true; + return 0; + } + return static_cast(val.weekDay() + 1); + } +}; + +template +struct TiDBDayOfYearTransformerImpl +{ + static constexpr auto name = "tidbDayOfYear"; + + static void execute(const Context & context, + const ColumnVector::Container & vec_from, + typename ColumnVector::Container & vec_to, + typename ColumnVector::Container & vec_null_map) + { + for (size_t i = 0; i < vec_from.size(); ++i) + { + bool is_null = false; + MyTimeBase val(vec_from[i]); + vec_to[i] = execute(context, val, is_null); + vec_null_map[i] = is_null; + } + } + + static ToFieldType execute(const Context & context, const MyTimeBase & val, bool & is_null) + { + // TiDB also considers NO_ZERO_DATE sql_mode. But sql_mode is not handled by TiFlash for now. + if (val.month == 0 || val.day == 0) + { + context.getDAGContext()->handleInvalidTime( + fmt::format("Invalid time value: month({}) or day({}) is zero", val.month, val.day), + Errors::Types::WrongValue); + is_null = true; + return 0; + } + return static_cast(val.yearDay()); + } +}; // Similar to FunctionDateOrDateTimeToSomething, but also handle nullable result and mysql sql mode. template class Transformer, bool return_nullable> class FunctionMyDateOrMyDateTimeToSomething : public IFunction @@ -3410,6 +3477,8 @@ using FunctionToStartOfFifteenMinutes = FunctionDateOrDateTimeToSomething; using FunctionToTime = FunctionDateOrDateTimeToSomething; using FunctionToLastDay = FunctionMyDateOrMyDateTimeToSomething; +using FunctionToTiDBDayOfWeek = FunctionMyDateOrMyDateTimeToSomething; +using FunctionToTiDBDayOfYear = FunctionMyDateOrMyDateTimeToSomething; using FunctionToRelativeYearNum = FunctionDateOrDateTimeToSomething; using FunctionToRelativeQuarterNum = FunctionDateOrDateTimeToSomething; diff --git a/dbms/src/Functions/tests/gtest_dayofweekyear.cpp b/dbms/src/Functions/tests/gtest_dayofweekyear.cpp index 243bdd0c403..d95144e34c3 100644 --- a/dbms/src/Functions/tests/gtest_dayofweekyear.cpp +++ b/dbms/src/Functions/tests/gtest_dayofweekyear.cpp @@ -29,8 +29,11 @@ class TestDayOfWeekYear : public DB::tests::FunctionTest TEST_F(TestDayOfWeekYear, TestDayOfWeek) try { + DAGContext * dag_context = context.getDAGContext(); + UInt64 ori_flags = dag_context->getFlags(); + dag_context->addFlag(TiDBSQLFlags::TRUNCATE_AS_WARNING); /// ColumnVector(nullable) - const String func_name = "toDayOfWeek"; + const String func_name = "tidbDayOfWeek"; static auto const nullable_datetime_type_ptr = makeNullable(std::make_shared(6)); static auto const datetime_type_ptr = std::make_shared(6); static auto const date_type_ptr = std::make_shared(); @@ -40,6 +43,7 @@ try // FIXME: https://github.com/pingcap/tiflash/issues/4186 // MyDateTime(2022, 12, 0, 1, 1, 1, 1).toPackedUInt(), // MyDateTime(2022, 13, 31, 1, 1, 1, 1).toPackedUInt(), + MyDateTime(0, 0, 0, 0, 0, 0, 0).toPackedUInt(), MyDateTime(1969, 1, 2, 1, 1, 1, 1).toPackedUInt(), MyDateTime(2022, 3, 13, 6, 7, 8, 9).toPackedUInt(), MyDateTime(2022, 3, 14, 9, 8, 7, 6).toPackedUInt(), @@ -51,12 +55,13 @@ try }) .column; auto input_col = ColumnWithTypeAndName(data_col_ptr, nullable_datetime_type_ptr, "input"); - auto output_col = createColumn>({{}, 5, 1, 2, 3, 4, 5, 6, 7}); + auto output_col = createColumn>({{}, {}, 5, 1, 2, 3, 4, 5, 6, 7}); ASSERT_COLUMN_EQ(output_col, executeFunction(func_name, input_col)); /// ColumnVector(non-null) data_col_ptr = createColumn( { + MyDateTime(0, 0, 0, 0, 0, 0, 0).toPackedUInt(), MyDateTime(1969, 1, 2, 1, 1, 1, 1).toPackedUInt(), MyDateTime(2022, 3, 13, 6, 7, 8, 9).toPackedUInt(), MyDateTime(2022, 3, 14, 9, 8, 7, 6).toPackedUInt(), @@ -68,22 +73,22 @@ try }) .column; input_col = ColumnWithTypeAndName(data_col_ptr, datetime_type_ptr, "input"); - output_col = createColumn({5, 1, 2, 3, 4, 5, 6, 7}); + output_col = createColumn>({{}, 5, 1, 2, 3, 4, 5, 6, 7}); ASSERT_COLUMN_EQ(output_col, executeFunction(func_name, input_col)); /// ColumnConst(non-null) input_col = ColumnWithTypeAndName(createConstColumn(1, MyDateTime(2022, 3, 19, 1, 1, 1, 1).toPackedUInt()).column, datetime_type_ptr, "input"); - output_col = createConstColumn(1, {7}); + output_col = createConstColumn>(1, {7}); ASSERT_COLUMN_EQ(output_col, executeFunction(func_name, input_col)); /// ColumnConst(nullable) input_col = ColumnWithTypeAndName(createConstColumn>(1, MyDateTime(2022, 3, 19, 1, 1, 1, 1).toPackedUInt()).column, nullable_datetime_type_ptr, "input"); - output_col = createConstColumn>(1, {7}); + output_col = createConstColumn>(1, {7}); ASSERT_COLUMN_EQ(output_col, executeFunction(func_name, input_col)); /// ColumnConst(nullable(null)) input_col = ColumnWithTypeAndName(createConstColumn>(1, {}).column, nullable_datetime_type_ptr, "input"); - output_col = createConstColumn>(1, {}); + output_col = createConstColumn>(1, {}); ASSERT_COLUMN_EQ(output_col, executeFunction(func_name, input_col)); /// MyDate ColumnVector(non-null) @@ -100,16 +105,20 @@ try }) .column; input_col = ColumnWithTypeAndName(data_col_ptr, date_type_ptr, "input"); - output_col = createColumn({5, 1, 2, 3, 4, 5, 6, 7}); + output_col = createColumn>({5, 1, 2, 3, 4, 5, 6, 7}); ASSERT_COLUMN_EQ(output_col, executeFunction(func_name, input_col)); + dag_context->setFlags(ori_flags); } CATCH TEST_F(TestDayOfWeekYear, TestDayOfYear) try { + DAGContext * dag_context = context.getDAGContext(); + UInt64 ori_flags = dag_context->getFlags(); + dag_context->addFlag(TiDBSQLFlags::TRUNCATE_AS_WARNING); /// ColumnVector(nullable) - const String func_name = "toDayOfYear"; + const String func_name = "tidbDayOfYear"; static auto const nullable_datetime_type_ptr = makeNullable(std::make_shared(6)); static auto const datetime_type_ptr = std::make_shared(6); static auto const date_type_ptr = std::make_shared(); @@ -119,6 +128,7 @@ try // FIXME: https://github.com/pingcap/tiflash/issues/4186 // MyDateTime(2022, 12, 0, 1, 1, 1, 1).toPackedUInt(), // MyDateTime(2022, 13, 31, 1, 1, 1, 1).toPackedUInt(), + MyDateTime(0, 0, 0, 0, 0, 0, 0).toPackedUInt(), MyDateTime(1969, 1, 2, 1, 1, 1, 1).toPackedUInt(), MyDateTime(2022, 3, 13, 6, 7, 8, 9).toPackedUInt(), MyDateTime(2022, 3, 14, 9, 8, 7, 6).toPackedUInt(), @@ -133,12 +143,13 @@ try }) .column; auto input_col = ColumnWithTypeAndName(data_col_ptr, nullable_datetime_type_ptr, "input"); - auto output_col = createColumn>({{}, 2, 72, 73, 74, 75, 76, 77, 78, 365, 366, 365}); + auto output_col = createColumn>({{}, {}, 2, 72, 73, 74, 75, 76, 77, 78, 365, 366, 365}); ASSERT_COLUMN_EQ(output_col, executeFunction(func_name, input_col)); /// ColumnVector(non-null) data_col_ptr = createColumn( { + MyDateTime(0, 0, 0, 0, 0, 0, 0).toPackedUInt(), MyDateTime(1969, 1, 2, 1, 1, 1, 1).toPackedUInt(), MyDateTime(2022, 3, 13, 6, 7, 8, 9).toPackedUInt(), MyDateTime(2022, 3, 14, 9, 8, 7, 6).toPackedUInt(), @@ -153,12 +164,12 @@ try }) .column; input_col = ColumnWithTypeAndName(data_col_ptr, datetime_type_ptr, "input"); - output_col = createColumn({2, 72, 73, 74, 75, 76, 77, 78, 365, 366, 365}); + output_col = createColumn>({{}, 2, 72, 73, 74, 75, 76, 77, 78, 365, 366, 365}); ASSERT_COLUMN_EQ(output_col, executeFunction(func_name, input_col)); /// ColumnConst(non-null) input_col = ColumnWithTypeAndName(createConstColumn(1, MyDateTime(2022, 3, 19, 1, 1, 1, 1).toPackedUInt()).column, datetime_type_ptr, "input"); - output_col = createConstColumn(1, {78}); + output_col = createConstColumn>(1, {78}); ASSERT_COLUMN_EQ(output_col, executeFunction(func_name, input_col)); /// ColumnConst(nullable) @@ -188,8 +199,9 @@ try }) .column; input_col = ColumnWithTypeAndName(data_col_ptr, date_type_ptr, "input"); - output_col = createColumn({2, 72, 73, 74, 75, 76, 77, 78, 365, 366, 365}); + output_col = createColumn>({2, 72, 73, 74, 75, 76, 77, 78, 365, 366, 365}); ASSERT_COLUMN_EQ(output_col, executeFunction(func_name, input_col)); + dag_context->setFlags(ori_flags); } CATCH From 1e1705e87a9cd4ccb04b0fc68a627a04745084a7 Mon Sep 17 00:00:00 2001 From: xufei Date: Thu, 17 Mar 2022 08:22:20 +0800 Subject: [PATCH 2/2] add more tests Signed-off-by: xufei --- dbms/src/Functions/FunctionsDateTime.h | 3 +++ dbms/src/Functions/tests/gtest_dayofweekyear.cpp | 6 ++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/dbms/src/Functions/FunctionsDateTime.h b/dbms/src/Functions/FunctionsDateTime.h index 8e1eb66b043..2b069a1153a 100644 --- a/dbms/src/Functions/FunctionsDateTime.h +++ b/dbms/src/Functions/FunctionsDateTime.h @@ -3344,6 +3344,9 @@ struct TiDBDayOfWeekTransformerImpl is_null = true; return 0; } + /// Behavior differences from TiDB: + /// for date in ['0000-01-01', '0000-03-01'), dayOfWeek is the same with MySQL, while TiDB is offset by one day + /// In TiDB dayOfWeek('0000-01-01') = 7, in MySQL/TiFlash dayOfWeek('0000-01-01') = 1 return static_cast(val.weekDay() + 1); } }; diff --git a/dbms/src/Functions/tests/gtest_dayofweekyear.cpp b/dbms/src/Functions/tests/gtest_dayofweekyear.cpp index d95144e34c3..6c2ec085aec 100644 --- a/dbms/src/Functions/tests/gtest_dayofweekyear.cpp +++ b/dbms/src/Functions/tests/gtest_dayofweekyear.cpp @@ -44,6 +44,7 @@ try // MyDateTime(2022, 12, 0, 1, 1, 1, 1).toPackedUInt(), // MyDateTime(2022, 13, 31, 1, 1, 1, 1).toPackedUInt(), MyDateTime(0, 0, 0, 0, 0, 0, 0).toPackedUInt(), + MyDateTime(0, 1, 1, 0, 0, 0, 0).toPackedUInt(), MyDateTime(1969, 1, 2, 1, 1, 1, 1).toPackedUInt(), MyDateTime(2022, 3, 13, 6, 7, 8, 9).toPackedUInt(), MyDateTime(2022, 3, 14, 9, 8, 7, 6).toPackedUInt(), @@ -55,7 +56,7 @@ try }) .column; auto input_col = ColumnWithTypeAndName(data_col_ptr, nullable_datetime_type_ptr, "input"); - auto output_col = createColumn>({{}, {}, 5, 1, 2, 3, 4, 5, 6, 7}); + auto output_col = createColumn>({{}, {}, 1, 5, 1, 2, 3, 4, 5, 6, 7}); ASSERT_COLUMN_EQ(output_col, executeFunction(func_name, input_col)); /// ColumnVector(non-null) @@ -129,6 +130,7 @@ try // MyDateTime(2022, 12, 0, 1, 1, 1, 1).toPackedUInt(), // MyDateTime(2022, 13, 31, 1, 1, 1, 1).toPackedUInt(), MyDateTime(0, 0, 0, 0, 0, 0, 0).toPackedUInt(), + MyDateTime(0, 1, 1, 0, 0, 0, 0).toPackedUInt(), MyDateTime(1969, 1, 2, 1, 1, 1, 1).toPackedUInt(), MyDateTime(2022, 3, 13, 6, 7, 8, 9).toPackedUInt(), MyDateTime(2022, 3, 14, 9, 8, 7, 6).toPackedUInt(), @@ -143,7 +145,7 @@ try }) .column; auto input_col = ColumnWithTypeAndName(data_col_ptr, nullable_datetime_type_ptr, "input"); - auto output_col = createColumn>({{}, {}, 2, 72, 73, 74, 75, 76, 77, 78, 365, 366, 365}); + auto output_col = createColumn>({{}, {}, 1, 2, 72, 73, 74, 75, 76, 77, 78, 365, 366, 365}); ASSERT_COLUMN_EQ(output_col, executeFunction(func_name, input_col)); /// ColumnVector(non-null)