Hi Øystein,
Øystein Grøvlen wrote:
> I think your new patch looks very good.
> I only have a question on the following change:
>
> > @@ -1795,7 +1795,8 @@ int Field::store_time(MYSQL_TIME *ltime,
> > ASSERT_COLUMN_MARKED_FOR_WRITE;
> > char buff[MAX_DATE_STRING_REP_LENGTH];
> > uint length= (uint) my_TIME_to_str(ltime, buff);
> > - return store(buff, length, &my_charset_bin);
> > + return store(buff, length, (charset()->state & MY_CS_NONASCII) ?
> > + &my_charset_latin1 : &my_charset_bin);
When character set is ASCII-compatible, then you can store
the my_TIME_to_str() result as is, without any conversion.
This is why I send my_charset_bin, to skip conversion.
When character set is not-ASCII-compatible (e.g. ucs2),
then conversion must happen. This is why I send my_charset_latin1
in this case.
Perhaps this version would be clearer to read:
return store(buff, length, (charset()->state & MY_CS_NONASCII) ?
charset() : &my_charset_bin);
(but would use a very little bit more CPU).
> > }
>
> Can you explain the reasoning behind this?
>
> If I apply your patch to current mysql-next-mr, I get a few test
> failures. Please, take a look at attached file.
This is expected. The patch gets not up to date very quickly.
I guess I will clean up the things later, before push.
>
> --
> Øystein
>
