Hi Jørgen,
The patch works fine and solves the crash. OK to push as it is.
One minor comment (that you can feel free to ignore):
- I think we are trying to standardize(?) on using upper-case letters in
SQL statements in our test files.
You also have the possibility of taking advantage of the latest changes
to our coding style and use the C++ true bool value instead of TRUE (but
given that TRUE and FALSE already is used in this function it is
probably not adding to readability to start mixing them).
Thanks for fixing this bug.
Best regards,
Olav
Jorgen Loland wrote:
> #At file:///export/home/jl208045/mysql/mysql-5.5-bugteam/ based on
> revid:alexander.nozdrin@stripped
>
> 3120 Jorgen Loland 2010-11-03
> Bug#57882 - Item_func_conv_charset::val_str(String*):
> Assertion `fixed == 1' failed
>
> (also fixes duplicate bug 57515)
>
> agg_item_set_converter() (item.cc) handles conversion of
> character sets by creating a new Item. fix_fields() is then
> called on this newly created item. Prior to this patch, it was
> not checked whether fix_fields() was successful or not. Thus,
> agg_item_set_converter() would return success even when an
> error occured. This patch makes it return error (TRUE) if
> fix_fields() fails.
> @ mysql-test/r/errors.result
> Add test for BUG#57882
> @ mysql-test/t/errors.test
> Add test for BUG#57882
> @ sql/item.cc
> Make agg_item_set_converter() return with error if fix_fields()
> on the newly created converted item fails.
>
> modified:
> mysql-test/r/errors.result
> mysql-test/t/errors.test
> sql/item.cc
> === modified file 'mysql-test/r/errors.result'
> --- a/mysql-test/r/errors.result 2010-05-27 16:01:43 +0000
> +++ b/mysql-test/r/errors.result 2010-11-03 13:47:59 +0000
> @@ -134,3 +134,15 @@ INSERT INTO t1 VALUES ('abc\0\0');
> INSERT INTO t1 VALUES ('abc\0\0');
> ERROR 23000: Duplicate entry 'abc\x00\x00' for key 'PRIMARY'
> DROP TABLE t1;
> +#
> +# Bug#57882: Item_func_conv_charset::val_str(String*):
> +# Assertion `fixed == 1' failed
> +#
> +select (convert('0' using latin1) in (char(cot('v') using utf8),''));
> +ERROR 22003: DOUBLE value is out of range in 'cot('v')'
> +set names utf8 collate utf8_latvian_ci ;
> +select updatexml(-73 * -2465717823867977728,@@global.slave_net_timeout,null);
> +ERROR 22003: BIGINT value is out of range in '(-(73) * -(2465717823867977728))'
> +#
> +# End Bug#57882
> +#
>
> === modified file 'mysql-test/t/errors.test'
> --- a/mysql-test/t/errors.test 2010-05-27 16:01:43 +0000
> +++ b/mysql-test/t/errors.test 2010-11-03 13:47:59 +0000
> @@ -155,3 +155,19 @@ INSERT INTO t1 VALUES ('abc\0\0');
> --error ER_DUP_ENTRY
> INSERT INTO t1 VALUES ('abc\0\0');
> DROP TABLE t1;
> +
> +--echo #
> +--echo # Bug#57882: Item_func_conv_charset::val_str(String*):
> +--echo # Assertion `fixed == 1' failed
> +--echo #
> +
> +--error ER_DATA_OUT_OF_RANGE
> +select (convert('0' using latin1) in (char(cot('v') using utf8),''));
> +
> +set names utf8 collate utf8_latvian_ci ;
> +--error ER_DATA_OUT_OF_RANGE
> +select updatexml(-73 * -2465717823867977728,@@global.slave_net_timeout,null);
> +
> +--echo #
> +--echo # End Bug#57882
> +--echo #
>
> === modified file 'sql/item.cc'
> --- a/sql/item.cc 2010-10-08 14:06:31 +0000
> +++ b/sql/item.cc 2010-11-03 13:47:59 +0000
> @@ -1857,7 +1857,11 @@ bool agg_item_set_converter(DTCollation
> We do not check conv->fixed, because Item_func_conv_charset which can
> be return by safe_charset_converter can't be fixed at creation
> */
> - conv->fix_fields(thd, arg);
> + if (conv->fix_fields(thd, arg))
> + {
> + res= TRUE;
> + break; // we cannot return here, we need to restore "arena".
> + }
> }
> if (arena)
> thd->restore_active_arena(arena, &backup);
>
>
> ------------------------------------------------------------------------
>
>
