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
>