List:Falcon Storage Engine« Previous MessageNext Message »
From:Lars-Erik Bjørk Date:November 24 2008 9:00am
Subject:Re: Request for review bug#40614
View as plain text  
Modified one:
http://lists.mysql.com/commits/59660

Includes comments from Vlad and Hakan


On Sat, 2008-11-22 at 16:56 +0100, 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
> >
> 
> 

Thread
Request for review bug#40614Lars-Erik Bjørk22 Nov
  • RE: Request for review bug#40614Vladislav Vaintroub22 Nov
    • Re: Request for review bug#40614Lars-Erik Bjørk22 Nov
      • Re: Request for review bug#40614Kevin Lewis23 Nov
      • Re: Request for review bug#40614Lars-Erik Bjørk24 Nov