List:Commits« Previous MessageNext Message »
From:Roy Lyseng Date:June 8 2011 11:03am
Subject:Re: bzr commit into mysql-5.5 branch (jorgen.loland:3409) Bug#12561818
View as plain text  
Hi Jørgen,

thank you for fixing this problem, which should definitely not be a problem.

The solution looks good to me, but please consider the comments below.

On 27.05.11 14.04, Jorgen Loland wrote:
> #At file:///export/home/jl208045/mysql/mysql-5.5-12561818/ based on
> revid:dmitry.shulga@stripped
>
>   3409 Jorgen Loland	2011-05-27
>        BUG#12561818 - RERUN OF STORED FUNCTION GIVES ERROR 1172:
>                       RESULT CONSISTED OF MORE THAN ONE ROW
>
>        MySQL converts incorrect DATEs and DATETIMEs to '0000-00-00' on
>        insertion by default. This means that this sequence is possible:
>
>        CREATE TABLE t1(date_notnull DATE NOT NULL);
>        INSERT INTO t1 values (NULL);
>        SELECT * FROM t1;
>        0000-00-00
>
>        At the same time, ODBC drivers do not (or at least did not in the
>        90's) understand the DATE and DATETIME value '0000-00-00'. Thus,
>        to be able to query for the value 0000-00-00 it was decided in
>        MySQL 4.x (or maybe even before that) that for the special case
>        of DATE/DATETIME NOT NULL columns, the query "SELECT ... WHERE
>        date_notnull IS NULL" should return rows with date_notnull ==
>        '0000-00-00'. This is documented misbehavior that we do not want
>        to change.
>
>        The hack used to make MySQL return these rows is to convert
>        "date_notnull IS NULL" to "date_notnull = 0". This is, however,
>        only done if the table date_notnull belongs to is not an inner
>        table of an outer join. The rationale for this seems to be that
>        if there is no join match for the row in the outer table,
>        null-complemented rows would otherwise not be returned because
>        the null-complemented DATE value is actually NULL. On the other
>        hand, this means that the "return rows with 0000-00-00 when the
>        query asks for IS NULL"-hack is not in effect for outer joins.
>
>        In this bug, we have a LEFT JOIN that does not misbehave like
>        the documentation says it should. The fix is to rewrite
>
>        "date_notnull IS NULL" to "date_notnull IS NULL OR
>                                   date_notnull = 0"
>        instead of
>        "date_notnull IS NULL" to "date_notnull = 0"
>        and also do this rewrite for outer joins.

Please explain the differences seen in first and second execution, and why they 
occur.
>       @ mysql-test/r/ps.result
>          Added test for BUG#12561818
>       @ mysql-test/r/type_datetime.result
>          "<col>  IS NULL" for DATE NOT NULL column is now rewritten to
>          "<col>  IS NULL OR<col>  = 0" and also done for outer joins.
>       @ mysql-test/t/ps.test
>          Added test for BUG#12561818
>       @ sql/sql_select.cc
>          Special handling of NULL for DATE/DATETIME NOT NULL columns:
>          Instead of rewriting
>          "date_notnull IS NULL" =>  "date_notnull = 0"
>          it is now rewritten to
>          "date_notnull IS NULL" =>  "date_notnull IS NULL OR date_notnull = 0"
>
>      modified:
>        mysql-test/r/ps.result
>        mysql-test/r/type_datetime.result
>        mysql-test/t/ps.test
>        sql/sql_select.cc
> === modified file 'mysql-test/r/ps.result'
> --- a/mysql-test/r/ps.result	2011-03-22 11:48:56 +0000
> +++ b/mysql-test/r/ps.result	2011-05-27 12:04:16 +0000
> @@ -3746,4 +3746,42 @@ FROM (SELECT 1 UNION SELECT 2) t;
>   1
>   2
>   #
> +# BUG#12561818: RERUN OF STORED FUNCTION GIVES ERROR 1172:
> +#               RESULT CONSISTED OF MORE THAN ONE ROW
> +#
> +CREATE TABLE t1 (a DATE NOT NULL, b INT);
> +INSERT INTO t1 VALUES ('0000-00-00',1), ('1999-05-10',2);
> +
> +SELECT * FROM t1 WHERE a IS NULL;
> +a	b
> +0000-00-00	1
> +
> +SELECT * FROM t1 LEFT JOIN t1 AS t1_2 ON 1 WHERE t1_2.a IS NULL;
> +a	b	a	b
> +0000-00-00	1	0000-00-00	1
> +1999-05-10	2	0000-00-00	1
> +
> +SELECT * FROM t1 JOIN t1 AS t1_2 ON 1 WHERE t1_2.a IS NULL;
> +a	b	a	b
> +0000-00-00	1	0000-00-00	1
> +1999-05-10	2	0000-00-00	1
> +
> +CREATE PROCEDURE p1()
> +BEGIN
> +SELECT *
> +FROM t1 LEFT JOIN t1 AS t1_2 ON 1
> +WHERE t1_2.a IS NULL AND t1_2.b<  2;
> +END $
> +
> +CALL p1();
> +a	b	a	b
> +0000-00-00	1	0000-00-00	1
> +1999-05-10	2	0000-00-00	1
> +CALL p1();
> +a	b	a	b
> +0000-00-00	1	0000-00-00	1
> +1999-05-10	2	0000-00-00	1
> +DROP PROCEDURE p1;
> +DROP TABLE t1;
> +#

I wonder if it would be better to represent the test as a prepared statement. It 
might then be possible to find a better test file than ps.test.

I also think that you should add a test case for the DATETIME data type.
>   # End of 5.5 tests.
>
> === modified file 'mysql-test/r/type_datetime.result'
> --- a/mysql-test/r/type_datetime.result	2011-01-19 14:12:43 +0000
> +++ b/mysql-test/r/type_datetime.result	2011-05-27 12:04:16 +0000
> @@ -543,7 +543,7 @@ id	select_type	table	type	possible_keys	
>   2	DEPENDENT SUBQUERY	x1	ALL	NULL	NULL	NULL	NULL	2	100.00	Using where
>   Warnings:
>   Note	1276	Field or reference 'test.t1.cur_date' of SELECT #2 was resolved in SELECT
> #1
> -Note	1003	select `test`.`t1`.`id` AS `id`,`test`.`t1`.`cur_date` AS `cur_date` from
> `test`.`t1` where<in_optimizer>(`test`.`t1`.`id`,<exists>(select 1 from
> `test`.`t1` `x1` where ((`test`.`t1`.`cur_date` = 0) and (<cache>(`test`.`t1`.`id`)
> = `test`.`x1`.`id`))))
> +Note	1003	select `test`.`t1`.`id` AS `id`,`test`.`t1`.`cur_date` AS `cur_date` from
> `test`.`t1` where<in_optimizer>(`test`.`t1`.`id`,<exists>(select 1 from
> `test`.`t1` `x1` where (((`test`.`t1`.`cur_date` = 0)
> or<cache>(isnull(`test`.`t1`.`cur_date`))) and (<cache>(`test`.`t1`.`id`) =
> `test`.`x1`.`id`))))
>   select * from t1
>   where id in (select id from t1 as x1 where (t1.cur_date is null));
>   id	cur_date
> @@ -555,7 +555,7 @@ id	select_type	table	type	possible_keys	
>   2	DEPENDENT SUBQUERY	x1	ALL	NULL	NULL	NULL	NULL	2	100.00	Using where
>   Warnings:
>   Note	1276	Field or reference 'test.t2.cur_date' of SELECT #2 was resolved in SELECT
> #1
> -Note	1003	select `test`.`t2`.`id` AS `id`,`test`.`t2`.`cur_date` AS `cur_date` from
> `test`.`t2` where<in_optimizer>(`test`.`t2`.`id`,<exists>(select 1 from
> `test`.`t2` `x1` where ((`test`.`t2`.`cur_date` = 0) and (<cache>(`test`.`t2`.`id`)
> = `test`.`x1`.`id`))))
> +Note	1003	select `test`.`t2`.`id` AS `id`,`test`.`t2`.`cur_date` AS `cur_date` from
> `test`.`t2` where<in_optimizer>(`test`.`t2`.`id`,<exists>(select 1 from
> `test`.`t2` `x1` where (((`test`.`t2`.`cur_date` = 0)
> or<cache>(isnull(`test`.`t2`.`cur_date`))) and (<cache>(`test`.`t2`.`id`) =
> `test`.`x1`.`id`))))
>   select * from t2
>   where id in (select id from t2 as x1 where (t2.cur_date is null));
>   id	cur_date
>
> === modified file 'mysql-test/t/ps.test'
> --- a/mysql-test/t/ps.test	2011-03-22 11:48:56 +0000
> +++ b/mysql-test/t/ps.test	2011-05-27 12:04:16 +0000
> @@ -3357,6 +3357,36 @@ disconnect con1;
>   SELECT *
>   FROM (SELECT 1 UNION SELECT 2) t;
>
> +--echo #
> +--echo # BUG#12561818: RERUN OF STORED FUNCTION GIVES ERROR 1172:
> +--echo #               RESULT CONSISTED OF MORE THAN ONE ROW
> +--echo #
> +
> +CREATE TABLE t1 (a DATE NOT NULL, b INT);
> +INSERT INTO t1 VALUES ('0000-00-00',1), ('1999-05-10',2);
> +
> +--echo
> +SELECT * FROM t1 WHERE a IS NULL;
> +--echo
> +SELECT * FROM t1 LEFT JOIN t1 AS t1_2 ON 1 WHERE t1_2.a IS NULL;
> +--echo
> +SELECT * FROM t1 JOIN t1 AS t1_2 ON 1 WHERE t1_2.a IS NULL;
> +
> +--echo
> +DELIMITER $;
> +CREATE PROCEDURE p1()
> +BEGIN
> +        SELECT *
> +        FROM t1 LEFT JOIN t1 AS t1_2 ON 1
> +        WHERE t1_2.a IS NULL AND t1_2.b<  2;
> +END $
> +DELIMITER ;$
> +--echo
> +CALL p1();
> +CALL p1();
> +
> +DROP PROCEDURE p1;
> +DROP TABLE t1;
>
>   --echo #
>   --echo # End of 5.5 tests.
>
> === modified file 'sql/sql_select.cc'
> --- a/sql/sql_select.cc	2011-05-06 08:27:04 +0000
> +++ b/sql/sql_select.cc	2011-05-27 12:04:16 +0000
> @@ -9519,24 +9519,29 @@ internal_remove_eq_conds(THD *thd, COND
>         Field *field=((Item_field*) args[0])->field;
>         /* fix to replace 'NULL' dates with '0' (shreeve@stripped) */
>         /*
> -        datetime_field IS NULL has to be modified to
> -        datetime_field == 0
> +        See BUG#12594011
> +        Documentation says that
> +        SELECT datetime_notnull d FROM t1 WHERE d IS NULL
> +        shall return rows where d=='0000-00-00'
> +
> +        Thus, for DATE and DATETIME columns defined as NOT NULL,
> +        "date_notnull IS NULL" has to be modified to
> +        "date_notnull IS NULL OR date_notnull == 0"
>         */
>         if (((field->type() == MYSQL_TYPE_DATE) ||
>              (field->type() == MYSQL_TYPE_DATETIME))&&
> -          (field->flags&  NOT_NULL_FLAG)&& 
> !field->table->maybe_null)
> +          (field->flags&  NOT_NULL_FLAG))
>         {
> -	COND *new_cond;
> -	if ((new_cond= new Item_func_eq(args[0],new Item_int("0", 0, 2))))
> -	{
> -	  cond=new_cond;
> -          /*
> -            Item_func_eq can't be fixed after creation so we do not check
> -            cond->fixed, also it do not need tables so we use 0 as second
> -            argument.
> -          */
> -	  cond->fix_fields(thd,&cond);
> -	}
> +        COND *eq_cond= new Item_func_eq(args[0],new Item_int("0", 0, 2));
> +        if (!eq_cond)
> +          return cond;

Space after comma, please.

Consider using new(thd->mem_root) which avoids call to get_thread_specific().

Consider using this constructor: Item_int((longlong)0, 1)

The error handling is a bit troublesome here, but it is probably the best 
solution. The caller must check the thd, though.

My favorite solution would be to have a standard bool return with false for 
success and true for error. Obviously, the generated condition would have to be 
an output argument in that case. This comment is for a possible future 
refactoring and not for this bugfix to consider.
> +
> +        COND *or_cond= new Item_cond_or(eq_cond, cond);
> +        if (!or_cond)
> +          return cond;
> +        cond= or_cond;
> +
> +        cond->fix_fields(thd,&cond);
>         }
>       }
>       if (cond->const_item())

Thanks,
Roy
Thread
bzr commit into mysql-5.5 branch (jorgen.loland:3409) Bug#12561818Jorgen Loland27 May
  • Re: bzr commit into mysql-5.5 branch (jorgen.loland:3409) Bug#12561818Roy Lyseng8 Jun
  • Re: bzr commit into mysql-5.5 branch (jorgen.loland:3409) Bug#12561818Sergey Glukhov9 Jun