From 5417002dae7a9f15ad77c58fae93eada80699b4b Mon Sep 17 00:00:00 2001 From: Alexander Barkov Date: Wed, 21 Feb 2018 08:18:44 +0400 Subject: [PATCH] A cleanup for MDEV-15340 + fix MDEV-15363 Wrong result for CAST(LAST_DAY(TIME'00:00:00') AS TIME) The change N7 in MDEV-15340 (see the commit message) introduced a regression in how CAST(AS TIME), HOUR(), TIME_TO_SEC() treat datetimes '0000-00-DD mm:hh:ss' (i.e. with zero YYYYMM part and a non-zero day). These functions historically do not mix days to hours on datetime-to-time conversion. Implementations of the underlying methods used get_arg0_time() to fetch MYSQL_TIME. After MDEV-15340, get_arg0_time() went through the Time() constructor, which always adds '0000-00-DD' to hours automatically (as in all other places in the code we do mix days to hours). Changes: 1. Extending Time() to make it possible to choose a desired way of treating '0000-00-DD' (ignore or mix to hours) on datetime-to-time conversion. Adding a helper class Time::Options for this, which now describes two aspects of Time() creation: 1. Flags for get_date() 2. Days/hours mixing behavior. 2. Removing Item_func::get_arg0_time(). Using Time() directly in all affected classes. Forcing Time() to ignore (rather than mix) '0000-00-DD' in these affected classes by passing a suitable Options value. 3. Adding Time::to_seconds(), to reuse the code between Item_func_time_to_sec::decimal_op() and Item_func_time_to_sec::int_op(). 4. Item_func::get_arg0_date() now returns only a datetime value, with automatic time-to-datetime conversion if needed. An assert was added to catch attempts to pass TIME_TIME_ONLY to get_arg0_date(). All callers were checked not to pass TIME_TIME_ONLY, this revealed a bug MDEV-15363. 5. Changing Item_func_last_day::get_date() to remove the TIME_TIME_ONLY flag before calling get_arg0_date(). This fixes MDEV-15363. --- mysql-test/r/func_time.result | 19 ++++++++++ mysql-test/t/func_time.test | 23 ++++++++++++ sql/item_func.h | 10 +----- sql/item_timefunc.cc | 54 ++++++++++++---------------- sql/sql_type.cc | 6 ++-- sql/sql_type.h | 66 +++++++++++++++++++++++++++++++---- 6 files changed, 127 insertions(+), 51 deletions(-) diff --git a/mysql-test/r/func_time.result b/mysql-test/r/func_time.result index 53f61f6644f..e03de2ca582 100644 --- a/mysql-test/r/func_time.result +++ b/mysql-test/r/func_time.result @@ -3382,4 +3382,23 @@ ADDTIME(TIME'10:20:30', DATE'2001-01-01') AS c3, ADDTIME(TIME'10:20:30', COALESCE(DATE'2001-01-01',TIMESTAMP'2001-01-01 00:00:00')) AS c4; c1 c2 c3 c4 NULL NULL NULL NULL +SELECT +HOUR(TIMESTAMP'0000-00-01 10:00:00') AS h0, +TIME_TO_SEC(TIMESTAMP'0000-00-01 10:00:00') AS tts0, +TIME_TO_SEC(TIMESTAMP'0000-00-01 10:00:00.1') AS tts1, +CAST(TIMESTAMP'0000-00-01 10:00:00' AS TIME) AS c0, +CAST(TIMESTAMP'0000-00-01 10:00:00.1' AS TIME(1)) AS c2; +h0 tts0 tts1 c0 c2 +10 36000 36000.1 10:00:00 10:00:00.1 +SET TIMESTAMP=DEFAULT; +# +# MDEV-15363 Wrong result for CAST(LAST_DAY(TIME'00:00:00') AS TIME) +# +SET TIMESTAMP=UNIX_TIMESTAMP('2018-02-17 01:02:03'); +SELECT +LAST_DAY(TIME'00:00:00') AS c1, +CAST(CAST(LAST_DAY(TIME'00:00:00') AS DATE) AS TIME) AS c2, +CAST(LAST_DAY(TIME'00:00:00') AS TIME) AS c3; +c1 c2 c3 +2018-02-28 00:00:00 00:00:00 SET TIMESTAMP=DEFAULT; diff --git a/mysql-test/t/func_time.test b/mysql-test/t/func_time.test index 0066d4a434b..a475c33b925 100644 --- a/mysql-test/t/func_time.test +++ b/mysql-test/t/func_time.test @@ -1974,4 +1974,27 @@ SELECT ADDTIME(TIME'10:20:30', DATE'2001-01-01') AS c3, ADDTIME(TIME'10:20:30', COALESCE(DATE'2001-01-01',TIMESTAMP'2001-01-01 00:00:00')) AS c4; +# +# Make sure that time functions that in 10.2 used get_arg0_time() +# do not mix days to hours for dates with zero YYYYMM and non-zero days. +# + +SELECT + HOUR(TIMESTAMP'0000-00-01 10:00:00') AS h0, + TIME_TO_SEC(TIMESTAMP'0000-00-01 10:00:00') AS tts0, + TIME_TO_SEC(TIMESTAMP'0000-00-01 10:00:00.1') AS tts1, + CAST(TIMESTAMP'0000-00-01 10:00:00' AS TIME) AS c0, + CAST(TIMESTAMP'0000-00-01 10:00:00.1' AS TIME(1)) AS c2; + +SET TIMESTAMP=DEFAULT; + +--echo # +--echo # MDEV-15363 Wrong result for CAST(LAST_DAY(TIME'00:00:00') AS TIME) +--echo # + +SET TIMESTAMP=UNIX_TIMESTAMP('2018-02-17 01:02:03'); +SELECT + LAST_DAY(TIME'00:00:00') AS c1, + CAST(CAST(LAST_DAY(TIME'00:00:00') AS DATE) AS TIME) AS c2, + CAST(LAST_DAY(TIME'00:00:00') AS TIME) AS c3; SET TIMESTAMP=DEFAULT; diff --git a/sql/item_func.h b/sql/item_func.h index 063a80de737..7d7cd61f474 100644 --- a/sql/item_func.h +++ b/sql/item_func.h @@ -160,18 +160,10 @@ public: void print_args(String *str, uint from, enum_query_type query_type); inline bool get_arg0_date(MYSQL_TIME *ltime, ulonglong fuzzy_date) { - return fuzzy_date & TIME_TIME_ONLY ? get_arg0_time(ltime) : - get_arg0_datetime(ltime, fuzzy_date); - } - inline bool get_arg0_datetime(MYSQL_TIME *ltime, ulonglong fuzzy_date) - { + DBUG_ASSERT(!(fuzzy_date & TIME_TIME_ONLY)); Datetime dt(current_thd, args[0], fuzzy_date); return (null_value= dt.copy_to_mysql_time(ltime)); } - inline bool get_arg0_time(MYSQL_TIME *ltime) - { - return (null_value= Time(args[0]).copy_to_mysql_time(ltime)); - } bool is_null() { update_null_value(); return null_value; diff --git a/sql/item_timefunc.cc b/sql/item_timefunc.cc index 816514dcc35..12b6d4cb9f3 100644 --- a/sql/item_timefunc.cc +++ b/sql/item_timefunc.cc @@ -993,15 +993,15 @@ longlong Item_func_quarter::val_int() longlong Item_func_hour::val_int() { DBUG_ASSERT(fixed == 1); - MYSQL_TIME ltime; - return get_arg0_time(<ime) ? 0 : ltime.hour; + Time tm(args[0], Time::Options_for_cast()); + return (null_value= !tm.is_valid_time()) ? 0 : tm.get_mysql_time()->hour; } longlong Item_func_minute::val_int() { DBUG_ASSERT(fixed == 1); - MYSQL_TIME ltime; - return get_arg0_time(<ime) ? 0 : ltime.minute; + Time tm(args[0], Time::Options_for_cast()); + return (null_value= !tm.is_valid_time()) ? 0 : tm.get_mysql_time()->minute; } /** @@ -1010,8 +1010,8 @@ longlong Item_func_minute::val_int() longlong Item_func_second::val_int() { DBUG_ASSERT(fixed == 1); - MYSQL_TIME ltime; - return get_arg0_time(<ime) ? 0 : ltime.second; + Time tm(args[0], Time::Options_for_cast()); + return (null_value= !tm.is_valid_time()) ? 0 : tm.get_mysql_time()->second; } @@ -1267,24 +1267,20 @@ longlong Item_func_unix_timestamp::val_int_endpoint(bool left_endp, bool *incl_e longlong Item_func_time_to_sec::int_op() { DBUG_ASSERT(fixed == 1); - MYSQL_TIME ltime; - if (get_arg0_time(<ime)) - return 0; - - longlong seconds=ltime.hour*3600L+ltime.minute*60+ltime.second; - return ltime.neg ? -seconds : seconds; + Time tm(args[0], Time::Options_for_cast()); + return ((null_value= !tm.is_valid_time())) ? 0 : tm.to_seconds(); } my_decimal *Item_func_time_to_sec::decimal_op(my_decimal* buf) { DBUG_ASSERT(fixed == 1); - MYSQL_TIME ltime; - if (get_arg0_time(<ime)) + Time tm(args[0], Time::Options_for_cast()); + if ((null_value= !tm.is_valid_time())) return 0; - - longlong seconds= ltime.hour*3600L+ltime.minute*60+ltime.second; - return seconds2my_decimal(ltime.neg, seconds, ltime.second_part, buf); + const MYSQL_TIME *ltime= tm.get_mysql_time(); + longlong seconds= tm.to_seconds_abs(); + return seconds2my_decimal(ltime->neg, seconds, ltime->second_part, buf); } @@ -1991,7 +1987,7 @@ String *Item_func_date_format::val_str(String *str) const MY_LOCALE *lc= 0; DBUG_ASSERT(fixed == 1); - if (get_arg0_date(&l_time, is_time_format ? TIME_TIME_ONLY : 0)) + if ((null_value= args[0]->get_date(&l_time, is_time_format ? TIME_TIME_ONLY : 0))) return 0; if (!(format = args[1]->val_str(str)) || !format->length()) @@ -2607,17 +2603,12 @@ void Item_char_typecast::fix_length_and_dec_internal(CHARSET_INFO *from_cs) bool Item_time_typecast::get_date(MYSQL_TIME *ltime, ulonglong fuzzy_date) { - if (get_arg0_time(ltime)) - return 1; + Time tm(args[0], Time::Options_for_cast()); + if ((null_value= !tm.is_valid_time())) + return true; + tm.copy_to_mysql_time(ltime); if (decimals < TIME_SECOND_PART_DIGITS) my_time_trunc(ltime, decimals); - /* - MYSQL_TIMESTAMP_TIME value can have non-zero day part, - which we should not lose. - */ - if (ltime->time_type != MYSQL_TIMESTAMP_TIME) - ltime->year= ltime->month= ltime->day= 0; - ltime->time_type= MYSQL_TIMESTAMP_TIME; return (fuzzy_date & TIME_TIME_ONLY) ? 0 : (null_value= check_date_with_warn(ltime, fuzzy_date, MYSQL_TIMESTAMP_ERROR)); @@ -2936,10 +2927,9 @@ bool Item_func_maketime::get_date(MYSQL_TIME *ltime, ulonglong fuzzy_date) longlong Item_func_microsecond::val_int() { DBUG_ASSERT(fixed == 1); - MYSQL_TIME ltime; - if (!get_arg0_date(<ime, TIME_TIME_ONLY)) - return ltime.second_part; - return 0; + Time tm(args[0], Time::Options_for_cast()); + return ((null_value= !tm.is_valid_time())) ? + 0 : tm.get_mysql_time()->second_part; } @@ -3322,7 +3312,7 @@ bool Item_func_str_to_date::get_date(MYSQL_TIME *ltime, ulonglong fuzzy_date) bool Item_func_last_day::get_date(MYSQL_TIME *ltime, ulonglong fuzzy_date) { - if (get_arg0_date(ltime, fuzzy_date) || + if (get_arg0_date(ltime, fuzzy_date & ~TIME_TIME_ONLY) || (ltime->month == 0)) return (null_value=1); uint month_idx= ltime->month-1; diff --git a/sql/sql_type.cc b/sql/sql_type.cc index d2d05ae0ff4..5dfe13c9ad3 100644 --- a/sql/sql_type.cc +++ b/sql/sql_type.cc @@ -121,12 +121,12 @@ bool Type_handler_data::init() Type_handler_data *type_handler_data= NULL; -void Time::make_from_item(Item *item, sql_mode_t flags) +void Time::make_from_item(Item *item, const Options opt) { - if (item->get_date(this, flags)) + if (item->get_date(this, opt.get_date_flags())) time_type= MYSQL_TIMESTAMP_NONE; else - valid_MYSQL_TIME_to_valid_value(); + valid_MYSQL_TIME_to_valid_value(opt); } diff --git a/sql/sql_type.h b/sql/sql_type.h index 2e2029f43f1..348a6cd4fb6 100644 --- a/sql/sql_type.h +++ b/sql/sql_type.h @@ -94,6 +94,47 @@ struct SORT_FIELD_ATTR; */ class Time: private MYSQL_TIME { +public: + enum datetime_to_time_mode_t + { + DATETIME_TO_TIME_YYYYMMDD_000000DD_MIX_TO_HOURS, + DATETIME_TO_TIME_YYYYMMDD_TRUNCATE + }; + class Options + { + sql_mode_t m_get_date_flags; + datetime_to_time_mode_t m_datetime_to_time_mode; + public: + Options() + :m_get_date_flags(flags_for_get_date()), + m_datetime_to_time_mode(DATETIME_TO_TIME_YYYYMMDD_000000DD_MIX_TO_HOURS) + { } + Options(sql_mode_t flags) + :m_get_date_flags(flags), + m_datetime_to_time_mode(DATETIME_TO_TIME_YYYYMMDD_000000DD_MIX_TO_HOURS) + { } + Options(sql_mode_t flags, datetime_to_time_mode_t dtmode) + :m_get_date_flags(flags), + m_datetime_to_time_mode(dtmode) + { } + sql_mode_t get_date_flags() const + { return m_get_date_flags; } + datetime_to_time_mode_t datetime_to_time_mode() const + { return m_datetime_to_time_mode; } + }; + /* + CAST(AS TIME) historically does not mix days to hours. + This is different comparing to how implicit conversion + in Field::store_time_dec() works (e.g. on INSERT). + */ + class Options_for_cast: public Options + { + public: + Options_for_cast() + :Options(flags_for_get_date(), DATETIME_TO_TIME_YYYYMMDD_TRUNCATE) + { } + }; +private: bool is_valid_value_slow() const { return time_type == MYSQL_TIMESTAMP_NONE || is_valid_time_slow(); @@ -113,7 +154,7 @@ class Time: private MYSQL_TIME e.g. returned from Item::get_date(). After this call, "this" is a valid TIME value. */ - void valid_datetime_to_valid_time() + void valid_datetime_to_valid_time(const Options opt) { DBUG_ASSERT(time_type == MYSQL_TIMESTAMP_DATE || time_type == MYSQL_TIMESTAMP_DATETIME); @@ -123,7 +164,9 @@ class Time: private MYSQL_TIME */ DBUG_ASSERT(day < 32); DBUG_ASSERT(hour < 24); - if (year == 0 && month == 0) + if (year == 0 && month == 0 && + opt.datetime_to_time_mode() == + DATETIME_TO_TIME_YYYYMMDD_000000DD_MIX_TO_HOURS) { /* The maximum hour value after mixing days will be 31*24+23=767, @@ -148,12 +191,12 @@ class Time: private MYSQL_TIME - either a valid TIME (within the supported TIME range), - or MYSQL_TIMESTAMP_NONE */ - void valid_MYSQL_TIME_to_valid_value() + void valid_MYSQL_TIME_to_valid_value(const Options opt) { switch (time_type) { case MYSQL_TIMESTAMP_DATE: case MYSQL_TIMESTAMP_DATETIME: - valid_datetime_to_valid_time(); + valid_datetime_to_valid_time(opt); break; case MYSQL_TIMESTAMP_NONE: break; @@ -165,11 +208,11 @@ class Time: private MYSQL_TIME break; } } - void make_from_item(class Item *item, sql_mode_t flags); + void make_from_item(class Item *item, const Options opt); public: Time() { time_type= MYSQL_TIMESTAMP_NONE; } - Time(Item *item) { make_from_item(item, flags_for_get_date()); } - Time(Item *item, sql_mode_t flags) { make_from_item(item, flags); } + Time(Item *item) { make_from_item(item, Options()); } + Time(Item *item, const Options opt) { make_from_item(item, opt); } static sql_mode_t flags_for_get_date() { return TIME_TIME_ONLY | TIME_INVALID_DATES; } static sql_mode_t comparison_flags_for_get_date() @@ -207,6 +250,15 @@ public: return 1; return 0; } + longlong to_seconds_abs() const + { + DBUG_ASSERT(is_valid_time_slow()); + return hour * 3600L + minute * 60 + second; + } + longlong to_seconds() const + { + return neg ? -to_seconds_abs() : to_seconds_abs(); + } };