maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Alexander!

On Jul 02, Alexander Barkov wrote:
> On 06/26/2013 11:11 PM, Sergei Golubchik wrote:
> >> === modified file 'sql/field.cc'
> >> --- sql/field.cc	2013-06-06 19:32:29 +0000
> >> +++ sql/field.cc	2013-06-19 04:51:38 +0000
> >> @@ -87,6 +87,7 @@ const char field_separator=',';
> >>   #define FIELDTYPE_NUM (FIELDTYPE_TEAR_FROM + (255 - FIELDTYPE_TEAR_TO))
> >>   static inline int field_type2index (enum_field_types field_type)
> >>   {
> >> +  field_type= real_type_to_type(field_type);
> >
> > I don't like that MYSQL_TYPE_*2 types (which are, really, now only a
> > migration types, for mysql-5.6 data files) are leaking into the server.
> > I'd prefer if they'd stay in their corresponding Field objects
> > and the rest of the server wouldn't need to know they exist.
> 
> I tried to reduce the amount of MYSQL_TYPE_*2 appearance.
> 
> They now appear in:
> 
> field.h
> field.cc
> log_event.cc
> rpl_utility.cc
> sql_partition.cc
> sql_table.cc
> which I think is OK.

I thought you'll make real_type() methods to return MYSQL_TYPE_xxx
values, not MYSQL_TYPE_xxx2 values. Why wouldn't that work?

> Also, I had to add them into two places:
> 
> item.cc:          Item::cmp_type()
> item_strfunc.cc:  Item_func_dyncol_create::prepare_arguments()
> 
> to avoid the "enumeration value is not handled" warning.
> I don't think that adding "default" is a good idea.

Agree with that.

> >> -String *Field_time::val_str(String *val_buffer,
> >> -			    String *val_ptr __attribute__((unused)))
> >> +String *Field_time::val_str(String *str,
> >> +			    String *unused __attribute__((unused)))
> >>   {
...
> >> +  str->length(my_time_to_str(&ltime, const_cast<char*>(str->ptr()), decimals()));
> >> +  str->set_charset(&my_charset_numeric);
> >> +  return str;
> > Well. I intentionally kept the old "optimized" version.
> > I'd expect a generic one to be slower.
> > May be it's ok, though...
> It seems we agreed that the old version was not
> really that much "optimized" to deserve so much
> duplicate code :)
> (I wrote about it in a separate letter and you did not argue).

Right.

> === modified file 'include/my_time.h'
> --- include/my_time.h	2013-05-24 15:09:59 +0000
> +++ include/my_time.h	2013-06-27 11:24:23 +0000
> @@ -177,6 +181,22 @@ static inline ulong sec_part_truncate(ul
>    return second_part - second_part % (ulong)log_10_int[TIME_SECOND_PART_DIGITS - digits];
>  }

Why you didn't remove sec_part_truncate()?

>  
> +/* Date/time rounding and truncation functions */
> +static inline long my_time_fraction_remainder(long nr, uint decimals)
> +{
> +  DBUG_ASSERT(decimals <= TIME_SECOND_PART_DIGITS);
> +  return nr % (long) log_10_int[TIME_SECOND_PART_DIGITS - decimals];
> +}
> +static inline void my_time_trunc(MYSQL_TIME *ltime, uint decimals)
> +{
> +  ltime->second_part-= my_time_fraction_remainder(ltime->second_part, decimals);
> +}
> +static inline void my_timeval_trunc(struct timeval *tv, uint decimals)
> +{
> +  tv->tv_usec-= my_time_fraction_remainder(tv->tv_usec, decimals);
> +}
> +
> +
>  #define hrtime_to_my_time(X) ((my_time_t)hrtime_to_time(X))
>  
>  /* 
> 
> === modified file 'include/mysql_com.h'
> --- include/mysql_com.h	2013-06-06 19:32:29 +0000
> +++ include/mysql_com.h	2013-06-27 10:54:59 +0000
> @@ -400,6 +400,9 @@ enum enum_field_types { MYSQL_TYPE_DECIM
>  			MYSQL_TYPE_DATETIME, MYSQL_TYPE_YEAR,
>  			MYSQL_TYPE_NEWDATE, MYSQL_TYPE_VARCHAR,
>  			MYSQL_TYPE_BIT,
> +			MYSQL_TYPE_TIMESTAMP2, /* MySQL-5.6 TIMESTAMP */
> +			MYSQL_TYPE_DATETIME2,  /* MySQL-5.6 DATETIME  */
> +			MYSQL_TYPE_TIME2,      /* MySQL-5.6 TIME      */

better to have a more verbose one multi-line comment for all three types,
something like

 /*
   mysql-5.6 compatibility types. they're only used internally
   for reading RBR mysql-5.6 events and mysql-5.6 frm files
   and they're are never sent to the client
 */

>                          MYSQL_TYPE_NEWDECIMAL=246,
>  			MYSQL_TYPE_ENUM=247,
>  			MYSQL_TYPE_SET=248,
> 
> === modified file 'sql-common/my_time.c'
> --- sql-common/my_time.c	2013-04-07 12:00:16 +0000
> +++ sql-common/my_time.c	2013-06-27 12:42:14 +0000
> @@ -1101,6 +1129,27 @@ int my_TIME_to_str(const MYSQL_TIME *l_t
>  }
>  
>  
> +/**
> +  Print a timestamp with an optional fractional part: XXXXX[.YYYYY]
> +
> +  @param      tm  The timestamp value to print.
> +  @param  OUT to  The string pointer to print at. 
> +  @param      dec Precision, in the range 0..6.
> +  @return         The length of the result string.
> +*/
> +int my_timeval_to_str(const struct timeval *tm, char *to, uint dec)
> +{
> +  char *pos= to + sprintf(to, "%d", (int) tm->tv_sec);

why sprintf?

> +  if (dec)
> +  {
> +    *pos++= '.';
> +    pos= fmt_number((uint) sec_part_shift(tm->tv_usec, dec), pos, dec);
> +  }
> +  *pos= '\0';
> +  return (int) (pos - to);
> +}
> +
> +
>  /*
>    Convert datetime value specified as number to broken-down TIME
>    representation and form value of DATETIME type as side-effect.
> 

Regards,
Sergei