List:Commits« Previous MessageNext Message »
From:Martin Hansson Date:June 24 2010 10:18am
Subject:Re: bzr commit into mysql-5.1-bugteam branch (Sergey.Glukhov:3432)
Bug#48916
View as plain text  
Hi,
apologies for the late reply, I was on vacation + sick child leave until 
now.

Sergey Glukhov wrote:
> 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.
So what is the table that should be sorted if it's not a temporary 
table? Do you mean 'the table whose columns we sort on'? If so, what if 
we sort on columns from different tables? I.e. which is the table that 
should be sorted in a query like

SELECT * FROM t1 JOIN t2 USING (a) ORDER BY t1.a, t2.b

I still don't quite understand by which criteria the splitting occurs. 
Could you say that the part of the HAVING clause  which refers to 
columns appearing in the  ORDER BY clause are extracted? Or other 
columns as well?
>>
>
>>>       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
Then please enter something like --echo # For code coverage of new code only
>
>>>                        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'
Aaah, I see, thank you for clearing that up.
>>> +        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).
But make_cond_for_table() creates Item's too, and it only calls 
quick_fix_field(). Why doesn't that work for you?

Best Regards

Martin



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