List:Commits« Previous MessageNext Message »
From:Olav Sandstaa Date:March 31 2011 1:24pm
Subject:Re: bzr commit into mysql-trunk branch (tor.didriksen:3354) Bug#11852644
View as plain text  
Hi Tor,

The patch looks good. I agree with you that the correct fix is to remove 
the assert and revert the original code change.

It would have been good to have a test case that produced correct result 
with the reverted code and wrong results if the code was not included. I 
tried to extend your test case to produce it but failed so I guess it is 
either hard or not possible(?).

Do you have any idea of why the original regression test case for the 
supposedly "dead code" introduced in Bug#11298 no longer trigger this code?

I have a few minor comments to the test case.

OK to push.

Olav


On 03/22/11 03:58 PM, Tor Didriksen wrote:
> #At file:///export/home/didrik/repo/next-mr-opt-backporting-bug11852644itemref/ based
> on revid:jorgen.loland@stripped
>
>   3354 Tor Didriksen	2011-03-22
>        Bug#11852644 - CRASH IN ITEM_REF::SAVE_IN_FIELD ON SELECT DISTINCT
>
>        Reverting this patch:
>        # sp1r-gkodinov/kgeorge@stripped
>        # Addendum to fix of bug #22344 : removed dead code.
>
>        With the current set of optimizations, this is no longer dead code.
>       @ mysql-test/include/subquery_mat.inc
>          New test case.
>       @ mysql-test/r/subquery_mat.result
>          New test case.
>       @ mysql-test/r/subquery_mat_all.result
>          New test case.
>       @ mysql-test/r/subquery_mat_none.result
>          New test case.
>       @ sql/item.cc
>          Re-enable the code for handling Item_ref::result_field in
> Item_ref::save_in_field.
>          The actual type of the object was Item_direct_view_ref, which got its
>          result_field set by create_tmp_field()
>
>      modified:
>        mysql-test/include/subquery_mat.inc
>        mysql-test/r/subquery_mat.result
>        mysql-test/r/subquery_mat_all.result
>        mysql-test/r/subquery_mat_none.result
>        sql/item.cc
> === modified file 'mysql-test/include/subquery_mat.inc'
> --- a/mysql-test/include/subquery_mat.inc	2011-02-14 11:21:26 +0000
> +++ b/mysql-test/include/subquery_mat.inc	2011-03-22 14:58:15 +0000
> @@ -847,3 +847,40 @@ eval $query;
>   DROP TABLE t1, t2;
>
>   --echo # End Bug#59833
> +
> +--echo #
> +--echo # Bug#11852644 - CRASH IN ITEM_REF::SAVE_IN_FIELD ON SELECT DISTINCT
> +--echo #
> +
> +CREATE TABLE t1 (
> +  col_varchar_key varchar(1) DEFAULT NULL,
> +  col_varchar_nokey varchar(1) DEFAULT NULL,
> +  KEY col_varchar_key (col_varchar_key))
> +ENGINE=MyISAM DEFAULT CHARSET=latin1;

I think this bug is not dependent on the storage engine so consider 
removing "ENGINE=MyISAM DEFAULT CHARSET=latin1".

> +
> +INSERT INTO t1 VALUES
> +('v','v'),('r','r');
> +
> +CREATE TABLE t2 (
> +  col_varchar_key varchar(1) DEFAULT NULL,
> +  col_varchar_nokey varchar(1) DEFAULT NULL,
> +  KEY col_varchar_key(col_varchar_key))
> +ENGINE=MyISAM DEFAULT CHARSET=latin1;

Same as above.

> +INSERT INTO t2 VALUES
> +('r','r'),('c','c');
> +
> +CREATE VIEW v3 AS SELECT * FROM t2;
> +
> +## EXPLAIN

I think you should remove the above comment.

> +SELECT DISTINCT alias2.col_varchar_key
> +FROM t1 AS alias1 JOIN v3 AS alias2
> +ON alias2.col_varchar_key = alias1.col_varchar_key
> +HAVING col_varchar_key IN (SELECT col_varchar_nokey FROM t2)
> +;
> +
> +DROP TABLE t1;
> +DROP TABLE t2;

To save a line and a few cpu cycles the above could have been combined 
on one line: DROP TABLE t1, t2;

> +DROP VIEW v3;
> +
> +--echo # End Bug#11852644
>
> === modified file 'mysql-test/r/subquery_mat.result'
> --- a/mysql-test/r/subquery_mat.result	2011-02-14 11:21:26 +0000
> +++ b/mysql-test/r/subquery_mat.result	2011-03-22 14:58:15 +0000
> @@ -1124,4 +1124,33 @@ WHERE t1.f1 IN (SELECT t1.pk FROM t1 ORD
>   f1	pk	pk
>   DROP TABLE t1, t2;
>   # End Bug#59833
> +#
> +# Bug#11852644 - CRASH IN ITEM_REF::SAVE_IN_FIELD ON SELECT DISTINCT
> +#
> +CREATE TABLE t1 (
> +col_varchar_key varchar(1) DEFAULT NULL,
> +col_varchar_nokey varchar(1) DEFAULT NULL,
> +KEY col_varchar_key (col_varchar_key))
> +ENGINE=MyISAM DEFAULT CHARSET=latin1;
> +INSERT INTO t1 VALUES
> +('v','v'),('r','r');
> +CREATE TABLE t2 (
> +col_varchar_key varchar(1) DEFAULT NULL,
> +col_varchar_nokey varchar(1) DEFAULT NULL,
> +KEY col_varchar_key(col_varchar_key))
> +ENGINE=MyISAM DEFAULT CHARSET=latin1;
> +INSERT INTO t2 VALUES
> +('r','r'),('c','c');
> +CREATE VIEW v3 AS SELECT * FROM t2;
> +SELECT DISTINCT alias2.col_varchar_key
> +FROM t1 AS alias1 JOIN v3 AS alias2
> +ON alias2.col_varchar_key = alias1.col_varchar_key
> +HAVING col_varchar_key IN (SELECT col_varchar_nokey FROM t2)
> +;
> +col_varchar_key
> +r
> +DROP TABLE t1;
> +DROP TABLE t2;
> +DROP VIEW v3;
> +# End Bug#11852644
>   set optimizer_switch=default;
>
> === modified file 'mysql-test/r/subquery_mat_all.result'
> --- a/mysql-test/r/subquery_mat_all.result	2011-03-17 11:23:06 +0000
> +++ b/mysql-test/r/subquery_mat_all.result	2011-03-22 14:58:15 +0000
> @@ -1123,4 +1123,33 @@ WHERE t1.f1 IN (SELECT t1.pk FROM t1 ORD
>   f1	pk	pk
>   DROP TABLE t1, t2;
>   # End Bug#59833
> +#
> +# Bug#11852644 - CRASH IN ITEM_REF::SAVE_IN_FIELD ON SELECT DISTINCT
> +#
> +CREATE TABLE t1 (
> +col_varchar_key varchar(1) DEFAULT NULL,
> +col_varchar_nokey varchar(1) DEFAULT NULL,
> +KEY col_varchar_key (col_varchar_key))
> +ENGINE=MyISAM DEFAULT CHARSET=latin1;
> +INSERT INTO t1 VALUES
> +('v','v'),('r','r');
> +CREATE TABLE t2 (
> +col_varchar_key varchar(1) DEFAULT NULL,
> +col_varchar_nokey varchar(1) DEFAULT NULL,
> +KEY col_varchar_key(col_varchar_key))
> +ENGINE=MyISAM DEFAULT CHARSET=latin1;
> +INSERT INTO t2 VALUES
> +('r','r'),('c','c');
> +CREATE VIEW v3 AS SELECT * FROM t2;
> +SELECT DISTINCT alias2.col_varchar_key
> +FROM t1 AS alias1 JOIN v3 AS alias2
> +ON alias2.col_varchar_key = alias1.col_varchar_key
> +HAVING col_varchar_key IN (SELECT col_varchar_nokey FROM t2)
> +;
> +col_varchar_key
> +r
> +DROP TABLE t1;
> +DROP TABLE t2;
> +DROP VIEW v3;
> +# End Bug#11852644
>   set optimizer_switch=default;
>
> === modified file 'mysql-test/r/subquery_mat_none.result'
> --- a/mysql-test/r/subquery_mat_none.result	2011-02-14 11:21:26 +0000
> +++ b/mysql-test/r/subquery_mat_none.result	2011-03-22 14:58:15 +0000
> @@ -1122,4 +1122,33 @@ WHERE t1.f1 IN (SELECT t1.pk FROM t1 ORD
>   f1	pk	pk
>   DROP TABLE t1, t2;
>   # End Bug#59833
> +#
> +# Bug#11852644 - CRASH IN ITEM_REF::SAVE_IN_FIELD ON SELECT DISTINCT
> +#
> +CREATE TABLE t1 (
> +col_varchar_key varchar(1) DEFAULT NULL,
> +col_varchar_nokey varchar(1) DEFAULT NULL,
> +KEY col_varchar_key (col_varchar_key))
> +ENGINE=MyISAM DEFAULT CHARSET=latin1;
> +INSERT INTO t1 VALUES
> +('v','v'),('r','r');
> +CREATE TABLE t2 (
> +col_varchar_key varchar(1) DEFAULT NULL,
> +col_varchar_nokey varchar(1) DEFAULT NULL,
> +KEY col_varchar_key(col_varchar_key))
> +ENGINE=MyISAM DEFAULT CHARSET=latin1;
> +INSERT INTO t2 VALUES
> +('r','r'),('c','c');
> +CREATE VIEW v3 AS SELECT * FROM t2;
> +SELECT DISTINCT alias2.col_varchar_key
> +FROM t1 AS alias1 JOIN v3 AS alias2
> +ON alias2.col_varchar_key = alias1.col_varchar_key
> +HAVING col_varchar_key IN (SELECT col_varchar_nokey FROM t2)
> +;
> +col_varchar_key
> +r
> +DROP TABLE t1;
> +DROP TABLE t2;
> +DROP VIEW v3;
> +# End Bug#11852644
>   set optimizer_switch=default;
>
> === modified file 'sql/item.cc'
> --- a/sql/item.cc	2011-03-17 09:47:50 +0000
> +++ b/sql/item.cc	2011-03-22 14:58:15 +0000
> @@ -6741,7 +6741,19 @@ my_decimal *Item_ref::val_decimal(my_dec
>   int Item_ref::save_in_field(Field *to, bool no_conversions)
>   {
>     int res;
> -  DBUG_ASSERT(!result_field);
> +  if (result_field)
> +  {
> +    if (result_field->is_null())
> +    {
> +      null_value= 1;
> +      res= set_field_to_null_with_conversions(to, no_conversions);
> +      return res;
> +    }
> +    to->set_notnull();
> +    res= field_conv(to, result_field);
> +    null_value= 0;
> +    return res;
> +  }
>     res= (*ref)->save_in_field(to, no_conversions);
>     null_value= (*ref)->null_value;
>     return res;
>
>
>
>


Thread
bzr commit into mysql-trunk branch (tor.didriksen:3354) Bug#11852644Tor Didriksen22 Mar
  • Re: bzr commit into mysql-trunk branch (tor.didriksen:3354) Bug#11852644Olav Sandstaa31 Mar
    • Re: bzr commit into mysql-trunk branch (tor.didriksen:3354) Bug#11852644Tor Didriksen1 Apr