List:Commits« Previous MessageNext Message »
From:Tor Didriksen Date:April 1 2011 10:51am
Subject:Re: bzr commit into mysql-trunk branch (tor.didriksen:3354) Bug#11852644
View as plain text  
On Thu, Mar 31, 2011 at 3:24 PM, Olav Sandstaa <olav.sandstaa@stripped>wrote:

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

Thanks, done.

-- didrik


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