1berry/MariaDB-server
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Bug#49771: Incorrect MIN/MAX for date/time values.

Browse Source 
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::has_compatible_context helper function is added. It checks whether
this and given items has the same comparison context or 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 number_to_datetime & 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.
This commit is contained in:
Evgeny Potemkin 2010-07-19 21:11:47 +04:00
parent 40856e830d
commit 589027b2f5
8 changed files with 169 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
mysql-test/r/group_by.result
View File

@ -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

23
mysql-test/t/group_by.test
View File

@ -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

74
sql/item.cc
View File

@ -4914,11 +4914,8 @@ Item *Item_field::equal_fields_propagator(uchar *arg)
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.
*/
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(&ltime, 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(), &ltime, TIME_FUZZY_DATE, &was_cut);
if (res == -1)
return NULL;
}
str_value.length(my_TIME_to_str(&ltime,
const_cast<char*>(str_value.ptr())));
str_value_cached= TRUE;
}
else if (!cache_value())
return NULL;
}
return &str_value;
}

37
sql/item.h
View File

@ -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; }
};

16
sql/item_cmpfunc.cc
View File

@ -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. */

12
sql/item_cmpfunc.h
View File

@ -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;
};

5
sql/item_sum.cc
View File

@ -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;
}

5
sql/item_sum.h
View File

@ -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(); }
};