List:Commits« Previous MessageNext Message »
From:Sergey Glukhov Date:June 17 2010 12:42pm
Subject:Re: bzr commit into mysql-5.1-bugteam branch (Sergey.Glukhov:3432)
Bug#48916
View as plain text  
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
Thread
bzr commit into mysql-5.1-bugteam branch (Sergey.Glukhov:3432)Bug#48916Sergey Glukhov16 Jun
  • Re: bzr commit into mysql-5.1-bugteam branch (Sergey.Glukhov:3432)Bug#48916Martin Hansson17 Jun
    • Re: bzr commit into mysql-5.1-bugteam branch (Sergey.Glukhov:3432)Bug#48916Sergey Glukhov17 Jun
      • Re: bzr commit into mysql-5.1-bugteam branch (Sergey.Glukhov:3432)Bug#48916Martin Hansson24 Jun