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=1
>>
>>
>>
>> --
>> Falcon Storage Engine Mailing List
>> For list archives: http://lists.mysql.com/falcon
>> To unsubscribe:
>> http://lists.mysql.com/falcon?unsub=1
>>
>
>
