Hi,
Approved.
--
Øystein
On 02/11/11 09:49 AM, Tor Didriksen wrote:
> #At file:///export/home/didrik/repo/5.5-bug60085nulltime/ based on
> revid:alexander.barkov@stripped
>
> 3323 Tor Didriksen 2011-02-11
> Bug #60085 crash in Item::save_in_field() with time data type
>
> Item_singlerow_subselect::val_str() incorrectly assumed that
> value->val_str() could set value->null_value= true;
> @ mysql-test/r/subselect_innodb.result
> New test case.
> @ mysql-test/t/subselect_innodb.test
> New test case.
> @ sql/item.cc
> Add some DBUG_TRACE for easier bug-hunting.
> @ sql/item_subselect.cc
> Item_singlerow_subselect::val_xxx() should do
> null_value= value->null_value;
> rather than
> null_value= false;
> Add some DBUG_TRACE for easier bug-hunting.
>
> modified:
> mysql-test/r/subselect_innodb.result
> mysql-test/t/subselect_innodb.test
> sql/item.cc
> sql/item_subselect.cc
> === modified file 'mysql-test/r/subselect_innodb.result'
> --- a/mysql-test/r/subselect_innodb.result 2006-01-26 16:54:34 +0000
> +++ b/mysql-test/r/subselect_innodb.result 2011-02-11 08:48:59 +0000
> @@ -245,3 +245,11 @@ x
> NULL
> drop procedure p1;
> drop tables t1,t2,t3;
> +#
> +# Bug#60085 crash in Item::save_in_field() with time data type
> +#
> +CREATE TABLE t1(a date, b int, unique(b), unique(a), key(b)) engine=innodb;
> +INSERT INTO t1 VALUES ('2011-05-13', 0);
> +SELECT 1 FROM t1 WHERE b< (SELECT CAST(a as date) FROM t1 GROUP BY a);
> +1
> +DROP TABLE t1;
>
> === modified file 'mysql-test/t/subselect_innodb.test'
> --- a/mysql-test/t/subselect_innodb.test 2006-01-26 16:54:34 +0000
> +++ b/mysql-test/t/subselect_innodb.test 2011-02-11 08:48:59 +0000
> @@ -238,3 +238,12 @@ call p1();
> call p1();
> drop procedure p1;
> drop tables t1,t2,t3;
> +
> +--echo #
> +--echo # Bug#60085 crash in Item::save_in_field() with time data type
> +--echo #
> +
> +CREATE TABLE t1(a date, b int, unique(b), unique(a), key(b)) engine=innodb;
> +INSERT INTO t1 VALUES ('2011-05-13', 0);
> +SELECT 1 FROM t1 WHERE b< (SELECT CAST(a as date) FROM t1 GROUP BY a);
> +DROP TABLE t1;
>
> === modified file 'sql/item.cc'
> --- a/sql/item.cc 2011-01-12 12:58:47 +0000
> +++ b/sql/item.cc 2011-02-11 08:48:59 +0000
> @@ -1051,6 +1051,7 @@ CHARSET_INFO *Item::default_charset()
>
> int Item::save_in_field_no_warnings(Field *field, bool no_conversions)
> {
> + DBUG_ENTER("Item::save_in_field_no_warnings");
> int res;
> TABLE *table= field->table;
> THD *thd= table->in_use;
> @@ -1059,11 +1060,13 @@ int Item::save_in_field_no_warnings(Fiel
> ulonglong sql_mode= thd->variables.sql_mode;
> thd->variables.sql_mode&= ~(MODE_NO_ZERO_IN_DATE | MODE_NO_ZERO_DATE);
> thd->count_cuted_fields= CHECK_FIELD_IGNORE;
> +
> res= save_in_field(field, no_conversions);
> +
> thd->count_cuted_fields= tmp;
> dbug_tmp_restore_column_map(table->write_set, old_map);
> thd->variables.sql_mode= sql_mode;
> - return res;
> + DBUG_RETURN(res);
> }
>
>
> @@ -5373,6 +5376,7 @@ int Item_null::save_safe_in_field(Field
> int Item::save_in_field(Field *field, bool no_conversions)
> {
> int error;
> + DBUG_ENTER("Item::save_in_field");
> if (result_type() == STRING_RESULT)
> {
> String *result;
> @@ -5383,7 +5387,8 @@ int Item::save_in_field(Field *field, bo
> if (null_value)
> {
> str_value.set_quick(0, 0, cs);
> - return set_field_to_null_with_conversions(field, no_conversions);
> + int retval= set_field_to_null_with_conversions(field, no_conversions);
> + DBUG_RETURN(retval);
> }
>
> /* NOTE: If null_value == FALSE, "result" must be not NULL. */
> @@ -5396,8 +5401,10 @@ int Item::save_in_field(Field *field, bo
> field->result_type() == STRING_RESULT)
> {
> double nr= val_real();
> - if (null_value)
> - return set_field_to_null_with_conversions(field, no_conversions);
> + if (null_value) {
> + int retval= set_field_to_null_with_conversions(field, no_conversions);
> + DBUG_RETURN(retval);
> + }
> field->set_notnull();
> error= field->store(nr);
> }
> @@ -5405,7 +5412,10 @@ int Item::save_in_field(Field *field, bo
> {
> double nr= val_real();
> if (null_value)
> - return set_field_to_null_with_conversions(field, no_conversions);
> + {
> + int retval= set_field_to_null_with_conversions(field, no_conversions);
> + DBUG_RETURN(retval);
> + }
> field->set_notnull();
> error=field->store(nr);
> }
> @@ -5414,7 +5424,10 @@ int Item::save_in_field(Field *field, bo
> my_decimal decimal_value;
> my_decimal *value= val_decimal(&decimal_value);
> if (null_value)
> - return set_field_to_null_with_conversions(field, no_conversions);
> + {
> + int retval= set_field_to_null_with_conversions(field, no_conversions);
> + DBUG_RETURN(retval);
> + }
> field->set_notnull();
> error=field->store_decimal(value);
> }
> @@ -5422,11 +5435,14 @@ int Item::save_in_field(Field *field, bo
> {
> longlong nr=val_int();
> if (null_value)
> - return set_field_to_null_with_conversions(field, no_conversions);
> + {
> + int retval= set_field_to_null_with_conversions(field, no_conversions);
> + DBUG_RETURN(retval);
> + }
> field->set_notnull();
> error=field->store(nr, unsigned_flag);
> }
> - return error ? error : (field->table->in_use->is_error() ? 1 : 0);
> + DBUG_RETURN(error ? error : (field->table->in_use->is_error() ? 1 : 0));
> }
>
>
> @@ -7516,8 +7532,10 @@ String *Item_cache_datetime::val_str(Str
> {
> DBUG_ASSERT(fixed == 1);
>
> + DBUG_ENTER("Item_cache_datetime::val_str");
> +
> if ((value_cached || str_value_cached)&& null_value)
> - return NULL;
> + DBUG_RETURN(NULL);
>
> if (!str_value_cached)
> {
> @@ -7535,7 +7553,7 @@ String *Item_cache_datetime::val_str(Str
> /* Return NULL in case of OOM/conversion error. */
> null_value= TRUE;
> if (str_value.alloc(MAX_DATE_STRING_REP_LENGTH))
> - return NULL;
> + DBUG_RETURN(NULL);
> if (cached_field_type == MYSQL_TYPE_TIME)
> {
> longlong time= int_value;
> @@ -7558,7 +7576,7 @@ String *Item_cache_datetime::val_str(Str
> longlong res;
> res= number_to_datetime(int_value,<ime,
> TIME_FUZZY_DATE,&was_cut);
> if (res == -1)
> - return NULL;
> + DBUG_RETURN(NULL);
> }
> str_value.length(my_TIME_to_str(<ime,
> const_cast<char*>(str_value.ptr())));
> @@ -7566,9 +7584,9 @@ String *Item_cache_datetime::val_str(Str
> null_value= FALSE;
> }
> else if (!cache_value())
> - return NULL;
> + DBUG_RETURN(NULL);
> }
> - return&str_value;
> + DBUG_RETURN(&str_value);
> }
>
>
>
> === modified file 'sql/item_subselect.cc'
> --- a/sql/item_subselect.cc 2011-01-12 12:15:22 +0000
> +++ b/sql/item_subselect.cc 2011-02-11 08:48:59 +0000
> @@ -258,28 +258,31 @@ bool Item_subselect::exec()
> {
> int res;
>
> + DBUG_ENTER("Item_subselect::exec");
> +
> /*
> Do not execute subselect in case of a fatal error
> or if the query has been killed.
> */
> if (thd->is_error() || thd->killed)
> - return 1;
> + DBUG_RETURN(1);
>
> DBUG_ASSERT(!thd->lex->context_analysis_only);
> /*
> Simulate a failure in sub-query execution. Used to test e.g.
> out of memory or query being killed conditions.
> */
> - DBUG_EXECUTE_IF("subselect_exec_fail", return 1;);
> + DBUG_EXECUTE_IF("subselect_exec_fail", DBUG_RETURN(1););
>
> res= engine->exec();
>
> if (engine_changed)
> {
> engine_changed= 0;
> - return exec();
> + bool retval= exec();
> + DBUG_RETURN(retval);
> }
> - return (res);
> + DBUG_RETURN(res);
> }
>
> Item::Type Item_subselect::type() const
> @@ -572,8 +575,9 @@ double Item_singlerow_subselect::val_rea
> DBUG_ASSERT(fixed == 1);
> if (!exec()&& !value->null_value)
> {
> - null_value= FALSE;
> - return value->val_real();
> + double retval= value->val_real();
> + null_value= value->null_value;
> + return retval;
> }
> else
> {
> @@ -587,8 +591,9 @@ longlong Item_singlerow_subselect::val_i
> DBUG_ASSERT(fixed == 1);
> if (!exec()&& !value->null_value)
> {
> - null_value= FALSE;
> - return value->val_int();
> + longlong retval= value->val_int();
> + null_value= value->null_value;
> + return retval;
> }
> else
> {
> @@ -599,15 +604,17 @@ longlong Item_singlerow_subselect::val_i
>
> String *Item_singlerow_subselect::val_str(String *str)
> {
> + DBUG_ENTER("Item_singlerow_subselect::val_str");
> if (!exec()&& !value->null_value)
> {
> - null_value= FALSE;
> - return value->val_str(str);
> + String *retval= value->val_str(str);
> + null_value= value->null_value;
> + DBUG_RETURN(retval);
> }
> else
> {
> reset();
> - return 0;
> + DBUG_RETURN(0);
> }
> }
>
> @@ -616,8 +623,9 @@ my_decimal *Item_singlerow_subselect::va
> {
> if (!exec()&& !value->null_value)
> {
> - null_value= FALSE;
> - return value->val_decimal(decimal_value);
> + my_decimal *retval= value->val_decimal(decimal_value);
> + null_value= value->null_value;
> + return retval;
> }
> else
> {
> @@ -631,8 +639,9 @@ bool Item_singlerow_subselect::val_bool(
> {
> if (!exec()&& !value->null_value)
> {
> - null_value= FALSE;
> - return value->val_bool();
> + bool retval= value->val_bool();
> + null_value= value->null_value;
> + return retval;
> }
> else
> {
>
>
>
>
>
--
Øystein Grøvlen, Principal Software Engineer
MySQL Group, Oracle
Trondheim, Norway
