From: Roy Lyseng
Date: February 9 2011 1:28pm
Subject: Re: bzr commit into mysql-trunk branch (olav.sandstaa:3559) Bug#59696
List-Archive: http://lists.mysql.com/commits/130865
Message-Id: <4D52966E.7050405@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

Hi Olav,

thank you for the new patch.

On 09.02.11 13.45, Olav Sandstaa wrote:
> Hi Roy,
>
> Thanks for the review of this patch.
>

>>>        2. Later in JOIN::optimize() we call substitute_for_best_equal_field()
>>>        which uses Item_func::transform() to select the best condition of the
>>>        tables. Item_func::transform() fails to evaluate the different
>>>        conditions that are equal due to these being stored in a Item_ref
>>>        object.
>>>
>>>        The fix for these problems is to implement compile() and transform()
>>>        methods for the Item_ref class that will do the proper evaluation of
>>>        the compile() and transform on the objects the Item_ref object has the
>>>        reference to. This will ensure that when Item_func::compile() and
>>>        Item_func::transform() evaluates Item_ref objects both the Item_ref
>>>        object and the object it references get evaluated.
>> Please try to simplify these two sentences.
>
> Hopefully easier to read now.

Yes, better.
>

>> Proposal: Do
>> let $query= SELECT ...;
>> eval explain $query;
>> eval $query;
>>
>> so that you need to specify the query only once.
>
> Not done (or rather: it was already done in the original patch for the test file
> - I do not know how to get that into the result file :-) )

Sorry, my bad :(


>>> +  old item is substituted for a new one. After this the transformer
>>> +  is applied to the Item_ref object.
>>
>> I think that you can remove references to error handling in the comment
>> without loss of clarity.
>
> Not done. I do not consider what I wrote to be related to error handling.

You're right...

Thanks,
Roy