#At file:///work/bzrroot/49771-bug-next-mr-bugfixing/ based on revid:mattias.jonsson@stripped
3200 Evgeny Potemkin 2010-05-25
Bug#49771: Incorrect MIN/MAX for date/time values.
This bug is a design flaw of the fix for the bug#33546. It assumed that an
item can be used only in one comparison context, but actually it isn't the
case. Item_cache_datetime is used to store result for MIX/MAX aggregate
functions. Because Arg_comparator always compares datetime values as INTs when
possible the Item_cache_datetime most time caches only INT value. But
since all datetime values has STRING result type MIN/MAX functions are asked
for a STRING value when the result is being sent to a client. The
Item_cache_datetime was designed to avoid conversions and get INT/STRING
values from an underlying item, but at the moment the values is asked
underlying item doesn't hold it anymore thus wrong result is returned.
Beside that MIN/MAX aggregate functions was wrongly initializing cached result
and this led to a wrong result.
The Item::is_datetime_comparable helper function is added. It checks whether
this and given items can be compared as DATETIME values by Arg_comparator.
The equality propagation optimization is adjusted to take into account that
items which being compared as DATETIME can have different comparison
contexts.
The Item_cache_datetime now converts cached INT value to a correct STRING
DATETIME value by means of get_date & my_TIME_to_str functions.
The Arg_comparator::set_cmp_context_for_datetime helper function is added.
It sets comparison context of items being compared as DATETIMEs to INT if
items will be compared as longlong.
The Item_sum_hybrid::setup function now correctly initializes its result
value.
In order to avoid unnecessary conversions Item_sum_hybrid now states that it
can provide correct longlong value if the item being aggregated can do it
too.
@ mysql-test/r/group_by.result
Added a test case for the bug#49771.
@ mysql-test/t/group_by.test
Added a test case for the bug#49771.
@ sql/item.cc
Bug#49771: Incorrect MIN/MAX for date/time values.
The equality propagation mechanism is adjusted to take into account that
items which being compared as DATETIME can have different comparison
contexts.
The Item_cache_datetime now converts cached INT value to a correct STRING
DATETIME/TIME value.
@ sql/item.h
Bug#49771: Incorrect MIN/MAX for date/time values.
The Item::is_datetime_comparable helper function is added. It checks whether
this and given items can be compared as DATETIME values by Arg_comparator.
Added Item_cache::clear helper function.
@ sql/item_cmpfunc.cc
Bug#49771: Incorrect MIN/MAX for date/time values.
The Arg_comparator::set_cmp_func now sets the correct comparison context
for items being compared as DATETIME values.
@ sql/item_cmpfunc.h
Bug#49771: Incorrect MIN/MAX for date/time values.
The Arg_comparator::set_cmp_context_for_datetime helper function is added.
It sets comparison context of items being compared as DATETIMEs to INT if
items will be compared as longlong.
@ sql/item_sum.cc
Bug#49771: Incorrect MIN/MAX for date/time values.
The Item_sum_hybrid::setup function now correctly initializes its result
value.
@ sql/item_sum.h
Bug#49771: Incorrect MIN/MAX for date/time values.
In order to avoid unnecessary conversions Item_sum_hybrid now states that it
can provide correct longlong value if the item being aggregated can do it
too.
modified:
mysql-test/r/group_by.result
mysql-test/t/group_by.test
sql/item.cc
sql/item.h
sql/item_cmpfunc.cc
sql/item_cmpfunc.h
sql/item_sum.cc
sql/item_sum.h
=== modified file 'mysql-test/r/group_by.result'
--- a/mysql-test/r/group_by.result 2010-03-24 15:03:44 +0000
+++ b/mysql-test/r/group_by.result 2010-05-25 12:07:47 +0000
@@ -1792,3 +1792,41 @@ aa b COUNT( b)
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');
+INSERT INTO t1 VALUES (1,'0000-00-00');
+INSERT INTO t1 VALUES (1,'2004-04-18');
+INSERT INTO t1 VALUES (2,'2004-05-19');
+INSERT INTO t1 VALUES (2,'0001-01-01');
+INSERT INTO t1 VALUES (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');
+INSERT INTO t1 VALUES (1,'06:11:01');
+INSERT INTO t1 VALUES (2,'19:53:05');
+INSERT INTO t1 VALUES (2,'21:44:25');
+INSERT INTO t1 VALUES (3,'10:55:12');
+INSERT INTO t1 VALUES (3,'05:45:11');
+INSERT INTO t1 VALUES (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
=== modified file 'mysql-test/t/group_by.test'
--- a/mysql-test/t/group_by.test 2010-02-24 13:52:27 +0000
+++ b/mysql-test/t/group_by.test 2010-05-25 12:07:47 +0000
@@ -1209,3 +1209,35 @@ 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');
+INSERT INTO t1 VALUES (1,'0000-00-00');
+INSERT INTO t1 VALUES (1,'2004-04-18');
+INSERT INTO t1 VALUES (2,'2004-05-19');
+INSERT INTO t1 VALUES (2,'0001-01-01');
+INSERT INTO t1 VALUES (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');
+INSERT INTO t1 VALUES (1,'06:11:01');
+INSERT INTO t1 VALUES (2,'19:53:05');
+INSERT INTO t1 VALUES (2,'21:44:25');
+INSERT INTO t1 VALUES (3,'10:55:12');
+INSERT INTO t1 VALUES (3,'05:45:11');
+INSERT INTO t1 VALUES (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
=== modified file 'sql/item.cc'
--- a/sql/item.cc 2010-05-05 13:57:29 +0000
+++ b/sql/item.cc 2010-05-25 12:07:47 +0000
@@ -4906,11 +4906,12 @@ Item *Item_field::equal_fields_propagato
e.g. <bin_col> = <int_col> AND <bin_col> = <hex_string>) since
Items don't know the context they are in and there are functions like
IF (<hex_string>, 'yes', 'no').
- The same problem occurs when comparing a DATE/TIME field with a
- DATE/TIME represented as an int and as a string.
+ DATE/DATETIME fields are exception and because of this they are checked
+ by the is_datetime_comparable function.
*/
if (!item ||
- (cmp_context != (Item_result)-1 && item->cmp_context != cmp_context))
+ (cmp_context != (Item_result)-1 && item->cmp_context != cmp_context &&
+ !is_datetime_comparable(item)))
item= this;
else if (field && (field->flags & ZEROFILL_FLAG) && IS_NUM(field->type()))
{
@@ -4975,7 +4976,8 @@ Item *Item_field::replace_equal_field(uc
if (const_item)
{
if (cmp_context != (Item_result)-1 &&
- const_item->cmp_context != cmp_context)
+ const_item->cmp_context != cmp_context &&
+ !is_datetime_comparable(const_item))
return this;
return const_item;
}
@@ -5045,21 +5047,6 @@ enum_field_types Item::field_type() cons
}
-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 */
@@ -7440,6 +7427,8 @@ bool Item_cache_datetime::cache_value_i
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;
@@ -7452,7 +7441,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)
@@ -7476,8 +7471,52 @@ void Item_cache_datetime::store(Item *it
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;
+ memset(<ime, 0, sizeof(MYSQL_TIME));
+ ltime.second= time % 100;
+ time/= 100;
+ ltime.minute= time % 100;
+ time/= 100;
+ ltime.hour= time % 100;
+ DBUG_ASSERT(!(time/100));
+ ltime.time_type= MYSQL_TIMESTAMP_TIME;
+ } else {
+ /*
+ get_date uses result type to determine the type of the source
+ value. Tweak it to get datetime from cached int value.
+ */
+ cached_result_type= INT_RESULT;
+ if (get_date(<ime, TIME_FUZZY_DATE))
+ {
+ cached_result_type= STRING_RESULT;
+ return NULL;
+ }
+ cached_result_type= STRING_RESULT;
+ }
+ my_TIME_to_str(<ime, (char*)str_value.ptr());
+ str_value.length(strlen(str_value.ptr()));
+ str_value_cached= TRUE;
+ }
+ else if (!cache_value())
+ return NULL;
+ }
return &str_value;
}
=== modified file 'sql/item.h'
--- a/sql/item.h 2010-04-13 22:07:08 +0000
+++ b/sql/item.h 2010-05-25 12:07:47 +0000
@@ -1162,7 +1162,33 @@ public:
representation is more precise than the string one).
*/
virtual bool result_as_longlong() { return FALSE; }
- bool is_datetime();
+ bool inline is_datetime()
+ {
+ switch (field_type())
+ {
+ case MYSQL_TYPE_DATE:
+ case MYSQL_TYPE_DATETIME:
+ case MYSQL_TYPE_TIMESTAMP:
+ return TRUE;
+ default:
+ break;
+ }
+ return FALSE;
+ }
+ /*
+ Fast check whether given item can be compared with this item as datetime
+ by Arg_comparator.
+ */
+ bool inline is_datetime_comparable(Item *item)
+ {
+ bool datetime= FALSE;
+ if (((datetime= is_datetime()) || cmp_context == STRING_RESULT) &&
+ ((datetime|= item->is_datetime()) ||
+ item->cmp_context == STRING_RESULT) &&
+ datetime)
+ return TRUE;
+ 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);
@@ -3193,6 +3219,7 @@ public:
virtual bool cache_value()= 0;
bool basic_const_item() const
{ return test(example && example->basic_const_item());}
+ virtual void clear() { null_value= 1; value_cached= FALSE; }
};
@@ -3349,11 +3376,12 @@ protected:
String str_value;
ulonglong int_value;
bool str_value_cached;
+ Item_result cached_result_type;
public:
Item_cache_datetime(enum_field_types field_type_arg):
Item_cache(field_type_arg), int_value(0), str_value_cached(0)
{
- cmp_context= STRING_RESULT;
+ cmp_context= cached_result_type= STRING_RESULT;
}
void store(Item *item, longlong val_arg);
@@ -3361,7 +3389,7 @@ public:
longlong val_int();
String* val_str(String *str);
my_decimal *val_decimal(my_decimal *);
- enum Item_result result_type() const { return STRING_RESULT; }
+ enum Item_result result_type() const { return cached_result_type; }
bool result_as_longlong() { return TRUE; }
/*
In order to avoid INT <-> STRING conversion of a DATETIME value
@@ -3372,6 +3400,7 @@ public:
*/
bool cache_value_int();
bool cache_value();
+ void clear() { null_value= 1; value_cached= str_value_cached= FALSE; }
};
=== modified file 'sql/item_cmpfunc.cc'
--- a/sql/item_cmpfunc.cc 2010-03-31 14:05:33 +0000
+++ b/sql/item_cmpfunc.cc 2010-05-25 12:07:47 +0000
@@ -877,7 +877,8 @@ get_time_value(THD *thd, Item ***item_ar
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))
+ ((Item_func*)item)->functype() != Item_func::GUSERVAR_FUNC) &&
+ item->type() != Item::CACHE_ITEM)
{
Item_cache_int *cache= new Item_cache_int();
/* Mark the cache as non-const to prevent re-caching. */
@@ -937,6 +938,7 @@ int Arg_comparator::set_cmp_func(Item_re
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 +951,7 @@ int Arg_comparator::set_cmp_func(Item_re
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 +1008,7 @@ bool Arg_comparator::try_year_cmp_func(I
is_nulls_eq= is_owner_equal_func();
func= &Arg_comparator::compare_datetime;
+ set_cmp_context_for_datetime();
return TRUE;
}
@@ -1058,6 +1062,7 @@ void Arg_comparator::set_datetime_cmp_fu
func= &Arg_comparator::compare_datetime;
get_value_a_func= &get_datetime_value;
get_value_b_func= &get_datetime_value;
+ set_cmp_context_for_datetime();
}
@@ -1145,7 +1150,8 @@ get_datetime_value(THD *thd, Item ***ite
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))
+ ((Item_func*)item)->functype() != Item_func::GUSERVAR_FUNC) &&
+ item->type() != Item::CACHE_ITEM)
{
Item_cache_int *cache= new Item_cache_int(MYSQL_TYPE_DATETIME);
/* Mark the cache as non-const to prevent re-caching. */
=== modified file 'sql/item_cmpfunc.h'
--- a/sql/item_cmpfunc.h 2010-04-01 19:34:09 +0000
+++ b/sql/item_cmpfunc.h 2010-05-25 12:07:47 +0000
@@ -118,7 +118,18 @@ public:
return (owner->type() == Item::FUNC_ITEM &&
((Item_func*)owner)->functype() == Item_func::EQUAL_FUNC);
}
-
+ /*
+ Set correct cmp_context if items will be compared as INTs.
+ */
+ void inline set_cmp_context_for_datetime()
+ {
+ if (func != &Arg_comparator::compare_datetime)
+ return;
+ if ((*a)->result_as_longlong())
+ (*a)->cmp_context= INT_RESULT;
+ if ((*b)->result_as_longlong())
+ (*b)->cmp_context= INT_RESULT;
+ }
friend class Item_func;
};
=== modified file 'sql/item_sum.cc'
--- a/sql/item_sum.cc 2010-04-09 11:20:40 +0000
+++ b/sql/item_sum.cc 2010-05-25 12:07:47 +0000
@@ -1217,8 +1217,7 @@ void Item_sum_hybrid::setup_hybrid(Item
{
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);
@@ -1906,7 +1905,7 @@ void Item_sum_variance::update_field()
void Item_sum_hybrid::clear()
{
- value->null_value= 1;
+ value->clear();
null_value= 1;
}
=== modified file 'sql/item_sum.h'
--- a/sql/item_sum.h 2010-04-09 11:20:40 +0000
+++ b/sql/item_sum.h 2010-05-25 12:07:47 +0000
@@ -1006,6 +1006,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 inline result_as_longlong() { return args[0]->result_as_longlong(); }
};
Attachment: [text/bzr-bundle] bzr/epotemkin@mysql.com-20100525120747-hg7pfk7b90gz43vc.bundle
