Re: bk commit into 5.1 tree (gshchepa:1.2670) BUG#28759

[Sergei Golubchik]

Hi!

On Feb 28, gshchepa@stripped wrote:
> ChangeSet@stripped, 2008-02-28 02:11:18+04:00, gshchepa@stripped +7 -0
>   Fixed bug#28759: The MONTHNAME/DAYNAME functions
>   returns binary string, so the LOWER/UPPER functions
>   are not effective on the result of MONTHNAME/DAYNAME
>   call.
>   
>   Character set of the MONTHNAME/DAYNAME function
>   result has been changed to system charset.
>   
> diff -Nrup a/mysql-test/r/func_time.result b/mysql-test/r/func_time.result
> --- a/mysql-test/r/func_time.result	2008-02-25 17:03:27 +04:00
> +++ b/mysql-test/r/func_time.result	2008-02-28 01:41:11 +04:00
> @@ -1318,4 +1318,16 @@ date_sub("0069-01-01 00:00:01",INTERVAL 
>  select date_sub("0169-01-01 00:00:01",INTERVAL 2 SECOND);
>  date_sub("0169-01-01 00:00:01",INTERVAL 2 SECOND)
>  0168-12-31 23:59:59
> +SELECT CHARSET(DAYNAME(19700101));
> +CHARSET(DAYNAME(19700101))
> +utf8
> +SELECT CHARSET(MONTHNAME(19700101));
> +CHARSET(MONTHNAME(19700101))
> +utf8
> +SELECT LOWER(DAYNAME(19700101));
> +LOWER(DAYNAME(19700101))
> +thursday
> +SELECT LOWER(MONTHNAME(19700101));
> +LOWER(MONTHNAME(19700101))
> +january
>  End of 5.1 tests
> diff -Nrup a/mysql-test/t/func_time-master.opt b/mysql-test/t/func_time-master.opt
> --- /dev/null	Wed Dec 31 16:00:00 196900
> +++ b/mysql-test/t/func_time-master.opt	2008-02-28 01:40:31 +04:00
> @@ -0,0 +1 @@
> +--loose-debug=d,test_lc_time_sz
> diff -Nrup a/mysql-test/t/func_time.test b/mysql-test/t/func_time.test
> --- a/mysql-test/t/func_time.test	2008-02-27 19:13:11 +04:00
> +++ b/mysql-test/t/func_time.test	2008-02-28 01:41:12 +04:00
> @@ -826,5 +826,14 @@ select date_sub("90-01-01 00:00:01",INTE
>  select date_sub("0069-01-01 00:00:01",INTERVAL 2 SECOND);
>  select date_sub("0169-01-01 00:00:01",INTERVAL 2 SECOND);
>  
> +#
> +# Bug #28759: DAYNAME() and MONTHNAME() return binary string
> +#
> +
> +SELECT CHARSET(DAYNAME(19700101));
> +SELECT CHARSET(MONTHNAME(19700101));
> +
> +SELECT LOWER(DAYNAME(19700101));
> +SELECT LOWER(MONTHNAME(19700101));
>  
>  --echo End of 5.1 tests
> diff -Nrup a/sql/item_timefunc.cc b/sql/item_timefunc.cc
> --- a/sql/item_timefunc.cc	2008-01-10 15:18:28 +04:00
> +++ b/sql/item_timefunc.cc	2008-02-28 01:41:13 +04:00
> @@ -1041,7 +1041,8 @@ String* Item_func_monthname::val_str(Str
>    }
>    null_value=0;
>    month_name=
> thd->variables.lc_time_names->month_names->type_names[month-1];
> -  str->set(month_name, strlen(month_name), system_charset_info);
> +  str->length(0);
> +  str->append(month_name, strlen(month_name), &my_charset_utf8_unicode_ci);
>    return str;

Do you mean that names aren't valid strings in
my_charset_utf8_general_ci, but valid in my_charset_utf8_unicode_ci ?

If yes - why do you convert them to my_charset_utf8_general_ci, and
don't leave in the charset they're in (my_charset_utf8_unicode_ci) ?

If no - why do you convert them from my_charset_utf8_unicode_ci and
don't leave them in charset they're in (my_charset_utf8_general_ci) ?

In either case - why do you convert ?

>  }
>  
> diff -Nrup a/sql/item_timefunc.h b/sql/item_timefunc.h
> --- a/sql/item_timefunc.h	2008-01-10 15:18:28 +04:00
> +++ b/sql/item_timefunc.h	2008-02-28 01:41:14 +04:00
> @@ -123,9 +123,9 @@ public:
>    enum Item_result result_type () const { return STRING_RESULT; }
>    void fix_length_and_dec() 
>    {
> -    collation.set(&my_charset_bin);
> +    collation.set(system_charset_info);
>      decimals=0;
> -    max_length=10*my_charset_bin.mbmaxlen;
> +    max_length= MAX_MONTH_NAME_LEN * collation.collation->mbmaxlen;
>      maybe_null=1; 
>    }
>    bool check_partition_func_processor(uchar *int_arg) {return TRUE;}
> @@ -298,9 +298,9 @@ class Item_func_dayname :public Item_fun
>    enum Item_result result_type () const { return STRING_RESULT; }
>    void fix_length_and_dec() 
>    { 
> -    collation.set(&my_charset_bin);
> +    collation.set(system_charset_info);
>      decimals=0; 
> -    max_length=9*MY_CHARSET_BIN_MB_MAXLEN;
> +    max_length= MAX_DAY_NAME_LEN * collation.collation->mbmaxlen;
>      maybe_null=1; 
>    }
>    bool check_partition_func_processor(uchar *int_arg) {return TRUE;}
> diff -Nrup a/sql/mysql_priv.h b/sql/mysql_priv.h
> --- a/sql/mysql_priv.h	2007-12-14 17:01:09 +04:00
> +++ b/sql/mysql_priv.h	2008-02-28 01:41:15 +04:00
> @@ -120,6 +120,16 @@ enum Derivation
>  };
>  
>  
> +/*
> +  Maximal number of UTF-8 characters in month/day name
> +  over the locale database (see the sql_locale.cc file).
> +  Both values should be checked/updated after every
> +  modification of that database. The test_lc_time_sz
> +  function (see mysqld.cc) controls such modifications.
> +*/
> +#define MAX_MONTH_NAME_LEN 18
> +#define MAX_DAY_NAME_LEN 14
> +
>  typedef struct my_locale_st
>  {
>    uint  number;
> diff -Nrup a/sql/mysqld.cc b/sql/mysqld.cc
> --- a/sql/mysqld.cc	2008-02-13 13:11:05 +04:00
> +++ b/sql/mysqld.cc	2008-02-28 01:41:16 +04:00
> @@ -3739,6 +3739,34 @@ void decrement_handler_count()
>  #endif /* defined(__NT__) || defined(HAVE_SMEM) */
>  
>  
> +#ifndef DBUG_OFF
> +/*
> +  Debugging helper function to keep the locale database
> +  (see sql_locale.cc) and the MAX_MONTH_NAME_LEN and
> +  MAX_DAY_NAME_LEN variable values in consistent state.
> +*/
> +static void test_lc_time_sz()
> +{
> +  uint max_month_len= 0;
> +  uint max_day_len = 0;
> +  for (MY_LOCALE **loc= my_locales; *loc; loc++)
> +  {
> +    for (const char **month= (*loc)->month_names->type_names; *month;
> month++)
> +      set_if_bigger(max_month_len,
> +                    my_numchars_mb(&my_charset_utf8_general_ci,
> +                                   *month, *month + strlen(*month)));
> +    for (const char **day= (*loc)->day_names->type_names; *day; day++)
> +      set_if_bigger(max_day_len,
> +                    my_numchars_mb(&my_charset_utf8_general_ci,
> +                                   *day, *day + strlen(*day)));
> +  }
> +  DBUG_ASSERT(max_month_len <= MAX_MONTH_NAME_LEN);
> +  DBUG_ASSERT(max_day_len <= MAX_DAY_NAME_LEN);
> +  return;
> +}
> +#endif//DBUG_OFF
> +
> +
>  #ifndef EMBEDDED_LIBRARY
>  #ifdef __WIN__
>  int win_main(int argc, char **argv)
> @@ -3839,6 +3867,8 @@ int main(int argc, char **argv)
>    libwrapName= my_progname+dirname_length(my_progname);
>    openlog(libwrapName, LOG_PID, LOG_AUTH);
>  #endif
> +
> +  DBUG_EXECUTE_IF("test_lc_time_sz", test_lc_time_sz(););

No DBUG_EXECUTE_IF here, call test_lc_time_sz() unconditionally.
And define it without #ifdef DBUG_OFF.
  
>    /*
>      We have enough space for fiddling with the argv, continue
> 
Regards / Mit vielen Grüssen,
Sergei

-- 
   __  ___     ___ ____  __
  /  |/  /_ __/ __/ __ \/ /   Sergei Golubchik <serg@stripped>
 / /|_/ / // /\ \/ /_/ / /__  Principal Software Developer/Server Architect
/_/  /_/\_, /___/\___\_\___/  MySQL GmbH, Dachauer Str. 37, D-80335 München
       <___/                  Geschäftsführer: Kaj Arnö - HRB
München 162140