diff --git a/mysql-test/r/group_by.result b/mysql-test/r/group_by.result index cdf48376fb0..3416e368dff 100644 --- a/mysql-test/r/group_by.result +++ b/mysql-test/r/group_by.result @@ -1812,3 +1812,32 @@ MAX(t2.a) DROP TABLE t1, t2; # # End of 5.1 tests +# +# Bug#49771: Incorrect MIN (date) when minimum value is 0000-00-00 +# +CREATE TABLE t1 (f1 int, f2 DATE); +INSERT INTO t1 VALUES (1,'2004-04-19'), (1,'0000-00-00'), (1,'2004-04-18'), +(2,'2004-05-19'), (2,'0001-01-01'), (3,'2004-04-10'); +SELECT MIN(f2),MAX(f2) FROM t1; +MIN(f2) MAX(f2) +0000-00-00 2004-05-19 +SELECT f1,MIN(f2),MAX(f2) FROM t1 GROUP BY 1; +f1 MIN(f2) MAX(f2) +1 0000-00-00 2004-04-19 +2 0001-01-01 2004-05-19 +3 2004-04-10 2004-04-10 +DROP TABLE t1; +CREATE TABLE t1 ( f1 int, f2 time); +INSERT INTO t1 VALUES (1,'01:27:35'), (1,'06:11:01'), (2,'19:53:05'), +(2,'21:44:25'), (3,'10:55:12'), (3,'05:45:11'), (4,'00:25:00'); +SELECT MIN(f2),MAX(f2) FROM t1; +MIN(f2) MAX(f2) +00:25:00 21:44:25 +SELECT f1,MIN(f2),MAX(f2) FROM t1 GROUP BY 1; +f1 MIN(f2) MAX(f2) +1 01:27:35 06:11:01 +2 19:53:05 21:44:25 +3 05:45:11 10:55:12 +4 00:25:00 00:25:00 +DROP TABLE t1; +#End of test#49771 diff --git a/mysql-test/t/group_by.test b/mysql-test/t/group_by.test index f90c1dc3c58..f46a20b2db4 100644 --- a/mysql-test/t/group_by.test +++ b/mysql-test/t/group_by.test @@ -1224,3 +1224,26 @@ DROP TABLE t1, t2; --echo # --echo # End of 5.1 tests + +--echo # +--echo # Bug#49771: Incorrect MIN (date) when minimum value is 0000-00-00 +--echo # +CREATE TABLE t1 (f1 int, f2 DATE); + +INSERT INTO t1 VALUES (1,'2004-04-19'), (1,'0000-00-00'), (1,'2004-04-18'), +(2,'2004-05-19'), (2,'0001-01-01'), (3,'2004-04-10'); + +SELECT MIN(f2),MAX(f2) FROM t1; +SELECT f1,MIN(f2),MAX(f2) FROM t1 GROUP BY 1; + +DROP TABLE t1; + +CREATE TABLE t1 ( f1 int, f2 time); +INSERT INTO t1 VALUES (1,'01:27:35'), (1,'06:11:01'), (2,'19:53:05'), +(2,'21:44:25'), (3,'10:55:12'), (3,'05:45:11'), (4,'00:25:00'); + +SELECT MIN(f2),MAX(f2) FROM t1; +SELECT f1,MIN(f2),MAX(f2) FROM t1 GROUP BY 1; + +DROP TABLE t1; +--echo #End of test#49771 diff --git a/sql/item.cc b/sql/item.cc index 13b4aa96c76..92cf2df8a4c 100644 --- a/sql/item.cc +++ b/sql/item.cc @@ -4914,11 +4914,8 @@ Item *Item_field::equal_fields_propagator(uchar *arg) e.g. = AND = ) since Items don't know the context they are in and there are functions like IF (, 'yes', 'no'). - The same problem occurs when comparing a DATE/TIME field with a - DATE/TIME represented as an int and as a string. */ - if (!item || - (cmp_context != (Item_result)-1 && item->cmp_context != cmp_context)) + if (!item || !has_compatible_context(item)) item= this; else if (field && (field->flags & ZEROFILL_FLAG) && IS_NUM(field->type())) { @@ -4982,8 +4979,7 @@ Item *Item_field::replace_equal_field(uchar *arg) Item *const_item= item_equal->get_const(); if (const_item) { - if (cmp_context != (Item_result)-1 && - const_item->cmp_context != cmp_context) + if (!has_compatible_context(const_item)) return this; return const_item; } @@ -5053,21 +5049,6 @@ enum_field_types Item::field_type() const } -bool Item::is_datetime() -{ - switch (field_type()) - { - case MYSQL_TYPE_DATE: - case MYSQL_TYPE_DATETIME: - case MYSQL_TYPE_TIMESTAMP: - return TRUE; - default: - break; - } - return FALSE; -} - - String *Item::check_well_formed_result(String *str, bool send_error) { /* Check whether we got a well-formed string */ @@ -7468,6 +7449,8 @@ bool Item_cache_datetime::cache_value_int() return FALSE; value_cached= TRUE; + // Mark cached string value obsolete + str_value_cached= FALSE; /* Assume here that the underlying item will do correct conversion.*/ int_value= example->val_int_result(); null_value= example->null_value; @@ -7480,7 +7463,13 @@ bool Item_cache_datetime::cache_value() { if (!example) return FALSE; + + if (cmp_context == INT_RESULT) + return cache_value_int(); + str_value_cached= TRUE; + // Mark cached int value obsolete + value_cached= FALSE; /* Assume here that the underlying item will do correct conversion.*/ String *res= example->str_result(&str_value); if (res && res != &str_value) @@ -7504,8 +7493,47 @@ void Item_cache_datetime::store(Item *item, longlong val_arg) String *Item_cache_datetime::val_str(String *str) { DBUG_ASSERT(fixed == 1); - if (!str_value_cached && !cache_value()) - return NULL; + if (!str_value_cached) + { + /* + When it's possible the Item_cache_datetime uses INT datetime + representation due to speed reasons. But still, it always has the STRING + result type and thus it can be asked to return a string value. + It is possible that at this time cached item doesn't contain correct + string value, thus we have to convert cached int value to string and + return it. + */ + if (value_cached) + { + MYSQL_TIME ltime; + if (str_value.alloc(MAX_DATE_STRING_REP_LENGTH)) + return NULL; + if (cached_field_type == MYSQL_TYPE_TIME) + { + ulonglong time= int_value; + DBUG_ASSERT(time < TIME_MAX_VALUE); + set_zero_time(<ime, MYSQL_TIMESTAMP_TIME); + ltime.second= time % 100; + time/= 100; + ltime.minute= time % 100; + time/= 100; + ltime.hour= time % 100; + } + else + { + int was_cut; + longlong res; + res= number_to_datetime(val_int(), <ime, TIME_FUZZY_DATE, &was_cut); + if (res == -1) + return NULL; + } + str_value.length(my_TIME_to_str(<ime, + const_cast(str_value.ptr()))); + str_value_cached= TRUE; + } + else if (!cache_value()) + return NULL; + } return &str_value; } diff --git a/sql/item.h b/sql/item.h index 35a0e8373f2..c7a97ca716a 100644 --- a/sql/item.h +++ b/sql/item.h @@ -1171,7 +1171,40 @@ public: representation is more precise than the string one). */ virtual bool result_as_longlong() { return FALSE; } - bool is_datetime(); + inline bool is_datetime() const + { + switch (field_type()) + { + case MYSQL_TYPE_DATE: + case MYSQL_TYPE_DATETIME: + case MYSQL_TYPE_TIMESTAMP: + return TRUE; + default: + break; + } + return FALSE; + } + /** + Check whether this and the given item has compatible comparison context. + Used by the equality propagation. See Item_field::equal_fields_propagator. + + @return + TRUE if the context is the same or if fields could be + compared as DATETIME values by the Arg_comparator. + FALSE otherwise. + */ + inline bool has_compatible_context(Item *item) const + { + /* Same context. */ + if (cmp_context == (Item_result)-1 || item->cmp_context == cmp_context) + return TRUE; + /* DATETIME comparison context. */ + if (is_datetime()) + return item->is_datetime() || item->cmp_context == STRING_RESULT; + if (item->is_datetime()) + return is_datetime() || cmp_context == STRING_RESULT; + return FALSE; + } virtual Field::geometry_type get_geometry_type() const { return Field::GEOM_GEOMETRY; }; String *check_well_formed_result(String *str, bool send_error= 0); @@ -3232,6 +3265,7 @@ public: virtual bool cache_value()= 0; bool basic_const_item() const { return test(example && example->basic_const_item());} + virtual void clear() { null_value= TRUE; value_cached= FALSE; } }; @@ -3412,6 +3446,7 @@ public: */ bool cache_value_int(); bool cache_value(); + void clear() { Item_cache::clear(); str_value_cached= FALSE; } }; diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc index d31799d7e60..2a7c9ac8144 100644 --- a/sql/item_cmpfunc.cc +++ b/sql/item_cmpfunc.cc @@ -876,8 +876,10 @@ get_time_value(THD *thd, Item ***item_arg, Item **cache_arg, Do not cache GET_USER_VAR() function as its const_item() may return TRUE for the current thread but it still may change during the execution. */ - if (item->const_item() && cache_arg && (item->type() != Item::FUNC_ITEM || - ((Item_func*)item)->functype() != Item_func::GUSERVAR_FUNC)) + if (item->const_item() && cache_arg && + item->type() != Item::CACHE_ITEM && + (item->type() != Item::FUNC_ITEM || + ((Item_func*)item)->functype() != Item_func::GUSERVAR_FUNC)) { Item_cache_int *cache= new Item_cache_int(); /* Mark the cache as non-const to prevent re-caching. */ @@ -937,6 +939,7 @@ int Arg_comparator::set_cmp_func(Item_result_field *owner_arg, get_value_a_func= &get_datetime_value; get_value_b_func= &get_datetime_value; cmp_collation.set(&my_charset_numeric); + set_cmp_context_for_datetime(); return 0; } else if (type == STRING_RESULT && (*a)->field_type() == MYSQL_TYPE_TIME && @@ -949,6 +952,7 @@ int Arg_comparator::set_cmp_func(Item_result_field *owner_arg, func= &Arg_comparator::compare_datetime; get_value_a_func= &get_time_value; get_value_b_func= &get_time_value; + set_cmp_context_for_datetime(); return 0; } else if (type == STRING_RESULT && @@ -1005,6 +1009,7 @@ bool Arg_comparator::try_year_cmp_func(Item_result type) is_nulls_eq= is_owner_equal_func(); func= &Arg_comparator::compare_datetime; + set_cmp_context_for_datetime(); return TRUE; } @@ -1058,6 +1063,7 @@ void Arg_comparator::set_datetime_cmp_func(Item_result_field *owner_arg, func= &Arg_comparator::compare_datetime; get_value_a_func= &get_datetime_value; get_value_b_func= &get_datetime_value; + set_cmp_context_for_datetime(); } @@ -1144,8 +1150,10 @@ get_datetime_value(THD *thd, Item ***item_arg, Item **cache_arg, Do not cache GET_USER_VAR() function as its const_item() may return TRUE for the current thread but it still may change during the execution. */ - if (item->const_item() && cache_arg && (item->type() != Item::FUNC_ITEM || - ((Item_func*)item)->functype() != Item_func::GUSERVAR_FUNC)) + if (item->const_item() && cache_arg && + item->type() != Item::CACHE_ITEM && + (item->type() != Item::FUNC_ITEM || + ((Item_func*)item)->functype() != Item_func::GUSERVAR_FUNC)) { Item_cache_int *cache= new Item_cache_int(MYSQL_TYPE_DATETIME); /* Mark the cache as non-const to prevent re-caching. */ diff --git a/sql/item_cmpfunc.h b/sql/item_cmpfunc.h index 8813324262c..b20a6892ce2 100644 --- a/sql/item_cmpfunc.h +++ b/sql/item_cmpfunc.h @@ -123,7 +123,17 @@ public: delete [] comparators; comparators= 0; } - + /* + Set correct cmp_context if items would be compared as INTs. + */ + inline void set_cmp_context_for_datetime() + { + DBUG_ASSERT(func == &Arg_comparator::compare_datetime); + if ((*a)->result_as_longlong()) + (*a)->cmp_context= INT_RESULT; + if ((*b)->result_as_longlong()) + (*b)->cmp_context= INT_RESULT; + } friend class Item_func; }; diff --git a/sql/item_sum.cc b/sql/item_sum.cc index 77c45ea85f7..b05d845dead 100644 --- a/sql/item_sum.cc +++ b/sql/item_sum.cc @@ -1214,8 +1214,7 @@ void Item_sum_hybrid::setup_hybrid(Item *item, Item *value_arg) { value= Item_cache::get_cache(item); value->setup(item); - if (value_arg) - value->store(value_arg); + value->store(value_arg); cmp= new Arg_comparator(); cmp->set_cmp_func(this, args, (Item**)&value, FALSE); collation.set(item->collation); @@ -1903,7 +1902,7 @@ void Item_sum_variance::update_field() void Item_sum_hybrid::clear() { - value->null_value= 1; + value->clear(); null_value= 1; } diff --git a/sql/item_sum.h b/sql/item_sum.h index b4539995632..2a722b93165 100644 --- a/sql/item_sum.h +++ b/sql/item_sum.h @@ -1010,6 +1010,11 @@ protected: void no_rows_in_result(); Field *create_tmp_field(bool group, TABLE *table, uint convert_blob_length); + /* + MIN/MAX uses Item_cache_datetime for storing DATETIME values, thus + in this case a correct INT value can be provided. + */ + bool result_as_longlong() { return args[0]->result_as_longlong(); } };