List:Falcon Storage Engine« Previous MessageNext Message »
From:Vladislav Vaintroub Date:December 2 2008 2:08pm
Subject:RE: Please review fox for bug#34479
View as plain text  
Lars-Erik, while I admit that is it a good job, except for taking slightly
non-MBCS compliant handling of minSortChar.

But, I do believe  there are already too many workarounds in MySQLCollation
.
The original problems are not documented anywhere  and comments are missing
, as usual. Thus I think for sake of portability, readability and other
abilities a rewrite of this stuff using Mr. MySQL-Unicode( aka Bar) as
consultant would  be good. I bet almost all problems could be fixed using
functions strings library already provides (this stuff might be not well
documented as well, but it works) and there is no reasons to
reinvent/reimplement stuff.

-First, why do we need computeKeyLength at all? What does it work around? 
-Which role does minSortChar play? Why are there attempts to trim it
together with the blanks?
-We don't we use charsetinfo->lengthsp() to trim blanks. Why not?
-We use some ad-hoc technique to trim strings. Why?
-why there are so many if (isBinary) in MySQLCollation?



> -----Original Message-----
> From: Lars-Erik.Bjork@stripped [mailto:Lars-Erik.Bjork@stripped]
> Sent: Tuesday, December 02, 2008 2:21 PM
> To: falcon@stripped
> Subject: Please review fox for bug#34479
> 
> The fix is available at:
> 
> http://lists.mysql.com/commits/60379
> 
> The function MySQLCollation::computeKeyLength(...) is no longer
> inlined,
> because it would clutter the header file. Also, some of you (Vlad? :))
> may have problems with using a couple of the CHARSET_INFO struct's
> members 'directly', but this is already done elsewhere :)
> 
> Please enjoy! ;)
> 
> Best regards,
> 
> Lars-Erik
> 
> 
> --
> Falcon Storage Engine Mailing List
> For list archives: http://lists.mysql.com/falcon
> To unsubscribe:    http://lists.mysql.com/falcon?unsub=1


Thread
Please review fox for bug#34479Lars-Erik Bjørk2 Dec
  • RE: Please review fox for bug#34479Vladislav Vaintroub2 Dec
    • RE: Please review fox for bug#34479Lars-Erik Bjørk2 Dec
      • RE: Please review fox for bug#34479Lars-Erik Bjørk2 Dec
        • Re: Please review fox for bug#34479Alexander Barkov3 Dec
          • Re: Please review fox for bug#34479Lars-Erik Bjørk3 Dec
            • Re: Please review fox for bug#34479Kevin Lewis3 Dec
              • Re: Please review fox for bug#34479Ann W. Harrison3 Dec
    • Re: Please review fox for bug#34479Ann W. Harrison2 Dec
      • RE: Please review fox for bug#34479Vladislav Vaintroub2 Dec
Re: Please review fox for bug#34479Kevin Lewis3 Dec
  • Re: Please review fox for bug#34479Alexander Barkov3 Dec
    • Re: Please review fox for bug#34479Ann W. Harrison3 Dec
      • Re: Please review fox for bug#34479Lars-Erik Bjørk4 Dec
        • RE: Please review fox for bug#34479Vladislav Vaintroub4 Dec
        • Re: Please review fox for bug#34479Lars-Erik Bjørk4 Dec
          • RE: Please review fox for bug#34479Vladislav Vaintroub4 Dec
          • Re: Please review fox for bug#34479Ann W. Harrison4 Dec
        • Re: Please review fox for bug#34479Ann W. Harrison4 Dec
          • Re: Please review fox for bug#34479Lars-Erik Bjørk5 Dec
            • Re: Please review fox for bug#34479Lars-Erik Bjørk5 Dec
        • Re: Please review fox for bug#34479Alexander Barkov8 Dec
          • RE: Please review fox for bug#34479Vladislav Vaintroub8 Dec
            • Re: Please review fox for bug#34479Alexander Barkov8 Dec
            • Re: Please review fox for bug#34479Ann W. Harrison8 Dec
Re: Please review fox for bug#34479Ann W. Harrison4 Dec