From: Kevin Lewis
Date: November 23 2008 5:16am
Subject: Re: Request for review bug#40614
List-Archive: http://lists.mysql.com/falcon/204
Message-Id: <4928E71B.1040402@sun.com>
MIME-Version: 1.0
Content-Type: text/plain; format=flowed; charset=ISO-8859-1
Content-Transfer-Encoding: 8BIT

Vlad,

That is a fantastic review comment. I totally agree, but I'm not sure I 
would have noticed it.  Well done.

Lars-Erik Bjørk wrote:
> Vlad,
> 
> Thanks for the comments, you do have a valid point there. There is no 
> need to take the extra step of computing the intermediate result. 
> Especially when it may lead to ambiguous results. I will change my 
> implementation accordingly :)
> 
> Btw, that was a fast review on a Saturday!
> 
> /Lars-Erik
> 
> 
> On Nov 22, 2008, at 4:24 PM, Vladislav Vaintroub wrote:
> 
>> Lars-Erik, I like the comments and test-case very impressive. Very good:)
>>
>> There is one subtlety though. Unlike in math, relying on difference 
>> between
>> values when making comparisons does not always work, specifically in 
>> corner
>> cases, where difference would overflow the range for the data type.
>>
>> Illustration is the program below. It will incorrectly think that
>> INT_MAX(==2147483647) < INT_MIN(-2147483648) and assert ,because 
>> INT_MAX -
>> INT_MIN is -1.
>>
>> #include <limits.h>
>> #include <assert.h>
>>
>> int compare(int a, int b)
>> {
>>     return (a-b);
>> }
>>
>> int main(int argc, char argv[])
>> {
>>     int result = compare(INT_MAX,INT_MIN);
>>     assert(result > 0);
>>     return 0;
>> }
>>
>> I propose move away from computing difference and consistently do 
>> something
>> like
>>
>> if (a < b)
>>   return -1;
>> if (a > b)
>>   return 1;
>> return 0;
>>
>> for all datatypes instead.
>>
>>
>>
>>
>>> -----Original Message-----
>>> From: Lars-Erik.Bjork@stripped [mailto:Lars-Erik.Bjork@stripped]
>>> Sent: Saturday, November 22, 2008 1:35 PM
>>> To: falcon@stripped
>>> Subject: Request for review bug#40614
>>>
>>> Hi all,
>>>
>>> The patch is described in the commit, available at
>>> http://lists.mysql.com/commits/59617
>>>
>>> Have a nice weekend!
>>>
>>> -- 
>>> Falcon Storage Engine Mailing List
>>> For list archives: http://lists.mysql.com/falcon
>>> To unsubscribe:    http://lists.mysql.com/falcon?unsub=wlad@stripped
>>
>>
>>
>> -- 
>> Falcon Storage Engine Mailing List
>> For list archives: http://lists.mysql.com/falcon
>> To unsubscribe:    
>> http://lists.mysql.com/falcon?unsub=lars-erik.bjork@stripped
>>
> 
>