Hi,
On 06/17/2010 03:55 PM, Martin Hansson wrote:
> Hi,
> I don't quite understand how the fix works. I inlined some questions
> below.
>
> Sergey Glukhov wrote:
>> #At file:///home/gluh/MySQL/mysql-5.1-bugteam/ based on
>> revid:bjorn.munch@stripped
>>
>> 3432 Sergey Glukhov 2010-06-16
>> Bug#48916 Server incorrectly processing HAVING clauses with an
>> ORDER BY clause
>> Before sorting HAVING condition is splitted into two parts,
>> first past is a table related condition and the rest of is
>> HAVING part. ...
> What is the 'table related' condition? Do you mean the SELECT
> condition? Or related to the temporary table that results from the
> GROUP BY/ORDER BY operation?
'table related' condition means condition which pushed down to the table
which
should be sorted. And It's not always a temporary table. This condition is
evaluated before sorting and filters out unnecessary records.
>
> past -> part
>> the fact that some of conditions might be non-const but
>> have 'used_tables' equal to zero(particulary IN subselect)
>> and because of that these conditions are cut off by
>> make_cond_for_table() function.
>> The fix is to split extraction of HAVING part into two steps:
>> 1. extract condition which does not belong to the sorted table
> What is the sorted table? Is there always such a table?
'Sorted' means 'table we are going to sort'.
>> 2. extract condition which has 'used_tables' equal to zero
>> and depending on result use one or union of them.
>> @ mysql-test/r/having.result
>> test case
>> @ mysql-test/t/having.test
>> test case
>> @ sql/sql_select.cc
>> The fix is to split extraction of HAVING part into two steps:
>> 1. extract condition which does not belong to sorted table
>> 2. extract condition which has 'used_tables' equal to zero
>> and depending on result use one or union of them.
>>
>> 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-04-12 10:12:20 +0000
>>
>> +End of 5.1 tests
> Most of these tests pass even without applying the fix. Is there a
> reason (e.g. code coverage) for having them here?
right, the reason is the code coverage
>> OUTER_REF_TABLE_BIT), (table_map) 0);
> What does make_cond_for_table do actually? There is no documentation
> and the parameter names are completely useless.
extract parts of condition which use 'used_table' tables where
'used_table' tables belong to 'tables' set.
For example:
1.used_table= 2(010)
tables= 7(111)
result: condition for table 2
2.used_table= 2(010)
tables= 5(101)
result: NULL
Note: There is a special case for tables= 0
which means 'extact all condtions with used_tables()==0'
>> + if (having_non_const_cond && having_const_cond)
>> + {
>> + curr_join->tmp_having= new
>> Item_cond_and(having_non_const_cond,
>> + having_const_cond);
>> + curr_join->tmp_having->fix_fields(thd, 0);
> What is the reason for calling fix_fields on the new expression tree?
> This was not done in previous code.
if we extract a part of already fixed condition we don't need fix_fields,
but if we create new one we should do fix_fields(top AND is not fixed).
>> + }
>> + else if (having_non_const_cond)
>> + curr_join->tmp_having= having_non_const_cond;
>> + else if (having_const_cond)
>> + curr_join->tmp_having= having_const_cond;
>> + else
>> + {
>> + DBUG_ASSERT(!(curr_join->tmp_having->used_tables() &
>> ~used_tables) &&
>> + !having_const_cond);
>> + curr_join->tmp_having= 0;
> Please use NULL instead of 0 for NULL pointers.
ok
Regards,
Gluh
