List:Commits« Previous MessageNext Message »
From:Alexander Barkov Date:April 6 2009 12:39pm
Subject:Re: bzr commit into mysql-6.0 branch (bar:3066) WL#1349
View as plain text  
Hello Sergei,


Sergei Golubchik wrote:
> Hi, Alexander!
> 
> On Apr 01, Alexander Barkov wrote:
> See a couple of comments below


Thanks for the comments!

I committed the second version of the patch:

   http://lists.mysql.com/commits/71448


Please find my answers below:



>  
>>  3066 Alexander Barkov	2009-04-01
>>       WL#1349 Use operating system localization to send it as a
>>       default client character set
>> === added file 'mysql-test/t/mysql_locale_posix.test'
>> --- a/mysql-test/t/mysql_locale_posix.test	1970-01-01 00:00:00 +0000
>> +++ b/mysql-test/t/mysql_locale_posix.test	2009-04-01 09:22:21 +0000
>> @@ -0,0 +1,203 @@
>> +--source include/not_windows.inc
>> +--source include/have_case_sensitive_file_system.inc
>> +
>> +#
>> +# Note, please keep this file UTF-8 compatible.
>> +# After editing, make sure that
>> +#  "file mysql_locale_posix.test"
>> +# says
>> +#   "UTF-8 Unicode text"
>> +# or
>> +#   "UTF-8 Unicode English text"
>> +#
>> +
>> +
>> +#
>> +# Check if we're running on a POSIX-locale machine
>> +#
>> +
>> +--disable_query_log
>> +--exec cp /usr/lib/locale/de_DE/LC_IDENTIFICATION var/tmp/LC_IDENTIFICATION
> &>/dev/null || true
> 
> doesn't work for me:
> 

Sorry, I didn't understand why it does not?

Changed to 'locale -a' anyway.


> janus pts/4 ~ % ls -l /usr/lib/locale/
> total 3084K
> -rw-r--r-- 1 root root 3210752 Feb 14 09:43 locale-archive
> janus pts/4 ~ % locale -a
> C
> POSIX
> de_DE
> de_DE.iso88591
> de_DE.iso885915@euro
> de_DE@euro
> deutsch
> en_GB
> en_GB.iso88591
> en_US
> en_US.iso88591
> en_US.utf8
> german
> ru_RU
> ru_RU.koi8r
> ru_RU.utf8
> janus pts/4 ~ %
> 
> perhaps you should use 'locale -a' ?

Looks like 'locale -a' is better. It can be missing though.


> 
>> +set @file=replace(load_file('../../tmp/LC_IDENTIFICATION'), 0x00, 0x20);
>> +--exec rm -f var/tmp/LC_IDENTIFICATION
>> === modified file 'sql-common/client.c'
>> --- a/sql-common/client.c	2008-12-24 10:48:24 +0000
>> +++ b/sql-common/client.c	2009-04-01 09:22:21 +0000
>> @@ -1738,52 +1738,288 @@ static MYSQL_METHODS client_methods=
>>  #endif
>>  };
>>  
>> +/* MySQL and OS charsets are fully compatible */
>> +#define my_cs_exact   1
>> +
>> +/* MySQL charset is very close to OS charset  */
>> +#define my_cs_approx  3
>> +
>> +/*
>> +  MySQL knows this charset, but it is not supported as client character set.
>> +*/
>> +#define my_cs_unsupp  4
> 
> why not a enum ?

Fixed.


> 
>> +
>> +typedef struct str2str_st
>> +{
>> +  const char *os_name;
>> +  const char *my_name;
>> +  int param;
>> +} MY_CSET_OS_NAME;
> ...
>> +static const char *
>> +my_os_charset_to_mysql_charset(const char *csname)
>> +{
>> +  const MY_CSET_OS_NAME *csp;
>> +  for (csp= charsets; csp->os_name; csp++)
>> +  {
>> +    if (!my_strcasecmp(&my_charset_latin1, csp->os_name, csname))
>> +    {
>> +      switch (csp->param)
>> +      {
>> +      case my_cs_exact:
>> +        return csp->my_name;
>> +
>> +      case my_cs_approx:
>> +        /*
>> +          Maybe we should print a warning eventually:
>> +          character set correspondence is not exact.
>> +        */
>> +        return csp->my_name;
>> +
>> +      case my_cs_unsupp:
>> +      default:
>> +        my_printf_error(ER_UNKNOWN_ERROR,
>> +                        "Requested character set '%s'"
>> +                        " is not supported by MySQL client",
>> +                         MYF(0), csp->my_name);
> 
> eh. It's not "requested". It'll be like
> 
> $ mysql
> ERROR 1234: Requested character set 'cp1201' is not supported by MySQL client
> $
> 
> and user thinks "WTF? I didn't request anything!"
> May be "Your locale character set ... is not supported ..." ?
> "locale" isn't a good word here though

Fixed.

> 
>> +        
>> +        goto def;
>> +      }
>> +    }
>> +  }
>> +
>> +  my_printf_error(ER_UNKNOWN_ERROR,
>> +                  "Unknown OS character set '%s'.",
>> +                  MYF(0), csname);
> 
> heh, here you don't write "requested" :)
> Anyway, before '%' it should have the same text as the previous error
> message - because they talk about the same thing.
> 
>> +
>> +def:
>> +  csname= MYSQL_DEFAULT_CHARSET_NAME;
>> +  my_printf_error(ER_UNKNOWN_ERROR,
>> +                  "Switching to the default character set '%s'.",
>> +                  MYF(0), csname);
>> +  return csname;
>> +}
>> +
> 
> Regards / Mit vielen Grüßen,
> Sergei
> 

Thread
bzr commit into mysql-6.0 branch (bar:3066) WL#1349Alexander Barkov1 Apr
  • Re: bzr commit into mysql-6.0 branch (bar:3066) WL#1349Sergei Golubchik1 Apr
    • Re: bzr commit into mysql-6.0 branch (bar:3066) WL#1349Alexander Barkov6 Apr