List:Commits« Previous MessageNext Message »
From:Øystein Grøvlen Date:June 23 2011 9:30am
Subject:Re: bzr commit into mysql-5.1 branch (sergey.glukhov:3649) Bug#11766594
View as plain text  
Hi Sergey,

Thanks for the patch.  The approach looks good.  See in-line for some 
comments.

On 06/21/11 12:03 PM, Sergey Glukhov wrote:
> #At file:///home/gluh/MySQL/mysql-5.1/ based on
> revid:sergey.glukhov@stripped
>
>   3649 Sergey Glukhov	2011-06-21
>        Bug#11766594 59736: SELECT DISTINCT.. INCORRECT RESULT WITH DETERMINISTIC
> FUNCTION IN WHERE C
>        There is an optimization of DISTINCT in JOIN::optimize()
>        which depends on THD::used_tables value. Each SELECT statement
>        inside SP resets used_tables value(see mysql_select()) and it
>        leads to wrong result. The fix is to replace THD::used_tables
>        with LEX::used_tables.
>       @ mysql-test/r/sp.result
>          test case
>       @ mysql-test/t/sp.test
>          test case
>       @ sql/sql_base.cc
>          THD::used_tables is replaced with LEX::used_tables
>       @ sql/sql_class.cc
>          THD::used_tables is replaced with LEX::used_tables
>       @ sql/sql_class.h
>          THD::used_tables is replaced with LEX::used_tables
>       @ sql/sql_insert.cc
>          THD::used_tables is replaced with LEX::used_tables
>       @ sql/sql_lex.cc
>          THD::used_tables is replaced with LEX::used_tables
>       @ sql/sql_lex.h
>          THD::used_tables is replaced with LEX::used_tables
>       @ sql/sql_prepare.cc
>          THD::used_tables is replaced with LEX::used_tables
>       @ sql/sql_select.cc
>          THD::used_tables is replaced with LEX::used_tables
>
>      modified:
>        mysql-test/r/sp.result
>        mysql-test/t/sp.test
>        sql/sql_base.cc
>        sql/sql_class.cc
>        sql/sql_class.h
>        sql/sql_insert.cc
>        sql/sql_lex.cc
>        sql/sql_lex.h
>        sql/sql_prepare.cc
>        sql/sql_select.cc

...

> === modified file 'mysql-test/t/sp.test'
> --- a/mysql-test/t/sp.test	2009-12-23 13:44:03 +0000
> +++ b/mysql-test/t/sp.test	2011-06-21 10:03:04 +0000
> @@ -8350,6 +8350,33 @@ SET @@GLOBAL.init_connect= @old_init_con
>   DROP PROCEDURE p2;
>   DROP PROCEDURE p5;
>
> +--echo #
> +--echo # Bug#11766594  59736: SELECT DISTINCT.. INCORRECT RESULT WITH DETERMINISTIC
> FUNCTION IN WHERE C
> +--echo #
> +
> +CREATE TABLE t1 (a INT, b INT, KEY(b));
> +CREATE TABLE t2 (c INT, d INT, KEY(c));
> +INSERT INTO t1 VALUES (1,1),(1,1),(1,2);
> +INSERT INTO t2 VALUES (1,1),(1,2);
> +
> +DELIMITER $;
> +
> +CREATE FUNCTION f1() RETURNS INT DETERMINISTIC
> +BEGIN
> +  DECLARE a int;
> +  -- SQL statement inside
> +  SELECT 1 INTO a;
> +  RETURN a;
> +END $
> +
> +DELIMITER ;$
> +
> +SELECT COUNT(DISTINCT d) FROM t1, t2  WHERE a = c AND b = f1();

According to the bug report the above query worked correctly, and this 
test case does not fail when your patch is not applied. It was the 
following query that gave wrong result:

SELECT COUNT(DISTINCT d) FROM t1, t2  WHERE a = c AND b = f1();

This query should be added to the test case.  I have verified that this 
query gives the correct result with your patch applied.

> +
> +DROP FUNCTION f1;
> +DROP TABLE t1, t2;
> +
> +
>   --echo # ------------------------------------------------------------------
>   --echo # -- End of 5.1 tests
>   --echo # ------------------------------------------------------------------
>

...

> === modified file 'sql/sql_lex.h'
> --- a/sql/sql_lex.h	2011-02-22 21:03:32 +0000
> +++ b/sql/sql_lex.h	2011-06-21 10:03:04 +0000
> @@ -1828,6 +1828,14 @@ typedef struct st_lex : public Query_tab
>     uint create_select_pos;
>     bool create_select_in_comment;
>
> +  /*
> +    The set of those tables whose fields are referenced in all subqueries
> +    of the query.

I guess it should be "The set of tables that has at least one field that 
is referenced in a subquery of this query". And what about tables that 
are referenced in the query itself?

> +    TODO: possibly this it is incorrect to have used tables in THD because
> +    with more than one subquery, it is not clear what does the field mean.

Is this "warning" still valid?  And what does it actually mean?

> +  */
> +  table_map  used_tables;
> +
>     st_lex();
>
>     virtual ~st_lex()
>

...

-- 
Øystein
Thread
bzr commit into mysql-5.1 branch (sergey.glukhov:3649) Bug#11766594Sergey Glukhov21 Jun
  • Re: bzr commit into mysql-5.1 branch (sergey.glukhov:3649) Bug#11766594Øystein Grøvlen23 Jun
  • Re: bzr commit into mysql-5.1 branch (sergey.glukhov:3649) Bug#11766594Roy Lyseng5 Jul
    • Re: bzr commit into mysql-5.1 branch (sergey.glukhov:3649) Bug#11766594Sergey Glukhov7 Jul
      • Re: bzr commit into mysql-5.1 branch (sergey.glukhov:3649) Bug#11766594Roy Lyseng7 Jul