Hi Sergey, patch approved, just some small things.
Sergey Glukhov wrote:
> #At file:///home/gluh/MySQL/mysql-5.1-bugteam/ based on
> revid:dao-gang.qu@stripped
>
> 3537 Sergey Glukhov 2010-12-30
> Bug#48916 Server incorrectly processing HAVING clauses with an ORDER BY clause
> Before sorting HAVING condition is split into two parts,
> first part is a table related condition and the rest of is
> HAVING part. Extraction of HAVING part does not take into account
> the fact that some of conditions might be non-const but
> have 'used_tables' == 0 (undependent subqueries)
>
Still "undependent" ;-)
> and because of that these conditions are cut off by
> make_cond_for_table() function.
> The fix is to use (table_map) 0 instead of used_tables in
> third argument for make_cond_for_table() function.
> It allows to extract elements which belong to sorted
> table and in addition elements which are undependend
>
Also still misspelled ;-)
> subqueries.
> @ mysql-test/r/having.result
> test result
> @ mysql-test/t/having.test
> test case
> @ sql/sql_select.cc
> The fix is to use (table_map) 0 instead of used_tables in
> third argument for make_cond_for_table() function.
> It allows to extract elements which belong to sorted
> table and in addition elements which are undependend
>
Also still misspelled ;-)
> subqueries.
>
> modified:
> mysql-test/r/having.result
> mysql-test/t/having.test
> sql/sql_select.cc
> === modified file 'mysql-test/r/having.result'
> --- a/mysql-test/r/having.result 2010-07-09 10:39:47 +0000
> +++ b/mysql-test/r/having.result 2010-12-30 11:21:31 +0000
> @@ -545,4 +545,26 @@ FROM t1 JOIN t2 ON t2.f2 LIKE 'x'
> HAVING field1 < 7;
> field1
> DROP TABLE t1,t2;
> +#
> +# Bug#48916 Server incorrectly processing HAVING clauses with an ORDER BY clause
> +#
> +CREATE TABLE t1 (f1 INT, f2 INT);
> +INSERT INTO t1 VALUES (1, 0), (2, 1), (3, 2);
> +CREATE TABLE t2 (f1 INT, f2 INT);
> +SELECT t1.f1
> +FROM t1
> +HAVING (3, 2) IN (SELECT f1, f2 FROM t2) AND t1.f1 >= 0
> +ORDER BY t1.f1;
> +f1
> +SELECT t1.f1
> +FROM t1
> +HAVING (3, 2) IN (SELECT 4, 2) AND t1.f1 >= 0
> +ORDER BY t1.f1;
> +f1
> +SELECT t1.f1
> +FROM t1
> +HAVING 2 IN (SELECT f2 FROM t2) AND t1.f1 >= 0
> +ORDER BY t1.f1;
> +f1
> +DROP TABLE t1,t2;
> End of 5.1 tests
>
> === modified file 'mysql-test/t/having.test'
> --- a/mysql-test/t/having.test 2010-07-09 10:39:47 +0000
> +++ b/mysql-test/t/having.test 2010-12-30 11:21:31 +0000
> @@ -564,4 +564,30 @@ HAVING field1 < 7;
>
> DROP TABLE t1,t2;
>
> +--echo #
> +--echo # Bug#48916 Server incorrectly processing HAVING clauses with an ORDER BY
> clause
> +--echo #
> +
> +CREATE TABLE t1 (f1 INT, f2 INT);
> +INSERT INTO t1 VALUES (1, 0), (2, 1), (3, 2);
> +CREATE TABLE t2 (f1 INT, f2 INT);
> +
> +SELECT t1.f1
> +FROM t1
> +HAVING (3, 2) IN (SELECT f1, f2 FROM t2) AND t1.f1 >= 0
> +ORDER BY t1.f1;
> +
> +SELECT t1.f1
> +FROM t1
> +HAVING (3, 2) IN (SELECT 4, 2) AND t1.f1 >= 0
> +ORDER BY t1.f1;
> +
> +SELECT t1.f1
> +FROM t1
> +HAVING 2 IN (SELECT f2 FROM t2) AND t1.f1 >= 0
> +ORDER BY t1.f1;
> +
> +DROP TABLE t1,t2;
> +
> +
> --echo End of 5.1 tests
>
> === modified file 'sql/sql_select.cc'
> --- a/sql/sql_select.cc 2010-12-28 23:47:05 +0000
> +++ b/sql/sql_select.cc 2010-12-30 11:21:31 +0000
> @@ -2207,7 +2207,7 @@ JOIN::exec()
>
> Item* sort_table_cond= make_cond_for_table(curr_join->tmp_having,
> used_tables,
> - used_tables);
> + (table_map) 0);
> if (sort_table_cond)
> {
> if (!curr_table->select)
> @@ -12814,6 +12814,41 @@ static bool test_if_ref(Item_field *left
> return 0; // keep test
> }
>
> +/**
> + Extract a condition that can be checked after reading given table
> +
> + @param cond Condition to analyze
> + @param tables Tables for which "current field values" are available
> + @param used_table Table that we're extracting the condition for (may
> + also include PSEUDO_TABLE_BITS, and may be zero)
> + @param exclude_expensive_cond Do not push expensive conditions
> +
> + @retval <>NULL Generated condition
> + @retval = NULL Already checked, OR error
>
As I said to Roy earlier, can you please remove the space between = and
NULL above? It looks very weird in Doxygen.
> +
> + @details
> + Extract the condition that can be checked after reading the table
> + specified in 'used_table', given that current-field values for tables
> + specified in 'tables' bitmap are available.
> + If 'used_table' is 0
> + - extract conditions for all tables in 'tables'.
> + - extract uncorrelated conditions(i.e. conditions which have
> + used_tables == 0).
>
Aren't you confusing correlated sub-queries with conditions that don't
refer to tables within the query block? IMU conditions that have
used_tables == 0 either reference no tables at all or they reference
sub-query tables and it doesn't matter if the sub-query is correlated or
not. Correlation is a property of the sub-query, not the parent query.
Great patch!
Best Regards and a happy new year,
Martin
