← Back to team overview

maria-developers team mailing list archive

Re: merge for MySQL56 FSP data types

 

Hi, Alexander!

On Jun 19, Alexander Barkov wrote:
> Hi Serg,
> 
> Please review the patch merging MySQL-5.6 fractional second precision 
> enabled data types.
> 
> Thanks.


Here you are, my comments are below:

> === modified file 'include/my_time.h'
> --- include/my_time.h	2013-05-24 15:09:59 +0000
> +++ include/my_time.h	2013-06-19 11:13:13 +0000
> @@ -107,10 +107,37 @@ ulonglong TIME_to_ulonglong_time(const M
>  ulonglong TIME_to_ulonglong(const MYSQL_TIME *);
>  double TIME_to_double(const MYSQL_TIME *my_time);
>  
> +/** MySQL56 routines and macros **/
> +#define MY_PACKED_TIME_GET_INT_PART(x)     ((x) >> 24)
> +#define MY_PACKED_TIME_GET_FRAC_PART(x)    ((x) % (1LL << 24))
> +#define MY_PACKED_TIME_MAKE(i, f)          ((((longlong) (i)) << 24) + (f))
> +#define MY_PACKED_TIME_MAKE_INT(i)         ((((longlong) (i)) << 24))
> +
> +longlong TIME_to_longlong_datetime_packed(const MYSQL_TIME *);
> +longlong TIME_to_longlong_time_packed(const MYSQL_TIME *);
> +
> +void TIME_from_longlong_datetime_packed(MYSQL_TIME *ltime, longlong nr);
> +void TIME_from_longlong_time_packed(MYSQL_TIME *ltime, longlong nr);
> +
> +void my_datetime_packed_to_binary(longlong nr, uchar *ptr, uint dec);
> +longlong my_datetime_packed_from_binary(const uchar *ptr, uint dec);
> +uint my_datetime_binary_length(uint dec);
> +
> +void my_time_packed_to_binary(longlong nr, uchar *ptr, uint dec);
> +longlong my_time_packed_from_binary(const uchar *ptr, uint dec);
> +uint my_time_binary_length(uint dec);
> +
> +void my_timestamp_to_binary(const struct timeval *tm, uchar *ptr, uint dec);
> +void my_timestamp_from_binary(struct timeval *tm, const uchar *ptr, uint dec);
> +uint my_timestamp_binary_length(uint dec);
> +/** End of MySQL routines and macros **/

I don't think we need these macros and related functions in include/ and
sql-commont (that is, in the client). These belong in sql/
as they're only used for reading MySQL data files and for RBR.

if you'd like you can even put them in a separate file, sql/compat65.cc

> +
>  longlong pack_time(MYSQL_TIME *my_time);
>  MYSQL_TIME *unpack_time(longlong packed, MYSQL_TIME *my_time);
>  
>  int check_time_range(struct st_mysql_time *my_time, uint dec, int *warning);
> +my_bool check_datetime_range(const MYSQL_TIME *ltime);
> +
>  
>  long calc_daynr(uint year,uint month,uint day);
>  uint calc_days_in_year(uint year);
> === modified file 'include/mysql_com.h'
> --- include/mysql_com.h	2013-06-06 19:32:29 +0000
> +++ include/mysql_com.h	2013-06-18 10:35:35 +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_TYPE_DATETIME2,
> +			MYSQL_TYPE_TIME2,

Add a comment about these types please.

>                          MYSQL_TYPE_NEWDECIMAL=246,
>  			MYSQL_TYPE_ENUM=247,
>  			MYSQL_TYPE_SET=248,
> === added file 'mysql-test/t/type_temporal_mysql56.test'
> --- mysql-test/t/type_temporal_mysql56.test	1970-01-01 00:00:00 +0000
> +++ mysql-test/t/type_temporal_mysql56.test	2013-06-19 09:01:02 +0000
> @@ -0,0 +1,23 @@
> +
> +let $MYSQLD_DATADIR= `select @@datadir`;
> +--copy_file std_data/mysql56time.frm $MYSQLD_DATADIR/test/mysql56time.frm
> +--copy_file std_data/mysql56time.MYD $MYSQLD_DATADIR/test/mysql56time.MYD
> +--copy_file std_data/mysql56time.MYI $MYSQLD_DATADIR/test/mysql56time.MYI
> +SHOW CREATE TABLE mysql56time;
> +--query_vertical SELECT * FROM mysql56time;

generally, you either start mysqltest command with -- (dash-dash)
OR you end it with a semicolon.
If you do both - as above - you end up with two semicolons,
see the result file

> +DROP TABLE mysql56time;
> +
> +--copy_file std_data/mysql56datetime.frm $MYSQLD_DATADIR/test/mysql56datetime.frm
> +--copy_file std_data/mysql56datetime.MYD $MYSQLD_DATADIR/test/mysql56datetime.MYD
> +--copy_file std_data/mysql56datetime.MYI $MYSQLD_DATADIR/test/mysql56datetime.MYI
> +SHOW CREATE TABLE mysql56datetime;
> +--query_vertical SELECT * FROM mysql56datetime;
> +DROP TABLE mysql56datetime;
> +
> +--copy_file std_data/mysql56timestamp.frm $MYSQLD_DATADIR/test/mysql56timestamp.frm
> +--copy_file std_data/mysql56timestamp.MYD $MYSQLD_DATADIR/test/mysql56timestamp.MYD
> +--copy_file std_data/mysql56timestamp.MYI $MYSQLD_DATADIR/test/mysql56timestamp.MYI
> +SET TIME_ZONE='+00:00';
> +SHOW CREATE TABLE mysql56timestamp;
> +--query_vertical SELECT * FROM mysql56timestamp;
> +DROP TABLE mysql56timestamp;
> === 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-19 11:59:45 +0000
> @@ -1346,6 +1396,432 @@ ulonglong TIME_to_ulonglong(const MYSQL_
>    return 0;
>  }
>  
> +
> +/*** MySQL56 TIME low-level memory and disk representation routines ***/

As I wrote above, I'd prefer to remove them from the client library
and move to sql/

> +
> +/*
> +  In-memory format:
> +
> +   1  bit sign          (Used for sign, when on disk)
> +   1  bit unused        (Reserved for wider hour range, e.g. for intervals)
> +   10 bit hour          (0-836)
> +   6  bit minute        (0-59)
> +   6  bit second        (0-59)
> +  24  bits microseconds (0-999999)
> +
> + Total: 48 bits = 6 bytes
> +   Suhhhhhh.hhhhmmmm.mmssssss.ffffffff.ffffffff.ffffffff
> +*/
> +
> +
> +/**
> +  Convert time value to MySQL56 numeric packed representation.
> +  
> +  @param    ltime   The value to convert.
> +  @return           Numeric packed representation.
> +*/
> +longlong TIME_to_longlong_time_packed(const MYSQL_TIME *ltime)
> +{
> +  /* If month is 0, we mix day with hours: "1 00:10:10" -> "24:00:10" */
> +  long hms= (((ltime->month ? 0 : ltime->day * 24) + ltime->hour) << 12) |
> +            (ltime->minute << 6) | ltime->second;
> +  longlong tmp= MY_PACKED_TIME_MAKE(hms, ltime->second_part);
> +  return ltime->neg ? -tmp : tmp;
> +}
> +
> +
> +
> +/**
> +  Convert MySQL56 time packed numeric representation to time.
> +
> +  @param  OUT ltime  The MYSQL_TIME variable to set.
> +  @param      tmp    The packed numeric representation.
> +*/
> +void TIME_from_longlong_time_packed(MYSQL_TIME *ltime, longlong tmp)
> +{
> +  long hms;
> +  if ((ltime->neg= (tmp < 0)))
> +    tmp= -tmp;
> +  hms= MY_PACKED_TIME_GET_INT_PART(tmp);
> +  ltime->year=   (uint) 0;
> +  ltime->month=  (uint) 0;
> +  ltime->day=    (uint) 0;
> +  ltime->hour=   (uint) (hms >> 12) % (1 << 10); /* 10 bits starting at 12th */
> +  ltime->minute= (uint) (hms >> 6)  % (1 << 6);  /* 6 bits starting at 6th   */
> +  ltime->second= (uint)  hms        % (1 << 6);  /* 6 bits starting at 0th   */
> +  ltime->second_part= MY_PACKED_TIME_GET_FRAC_PART(tmp);
> +  ltime->time_type= MYSQL_TIMESTAMP_TIME;
> +}
> +
> +
> +/**
> +  Calculate binary size of MySQL56 packed numeric time representation.
> +  
> +  @param   dec   Precision.
> +*/
> +uint my_time_binary_length(uint dec)
> +{
> +  DBUG_ASSERT(dec <= TIME_SECOND_PART_DIGITS);
> +  return 3 + (dec + 1) / 2;
> +}
> +
> +
> +/*
> +  On disk we convert from signed representation to unsigned
> +  representation using TIMEF_OFS, so all values become binary comparable.
> +*/
> +#define TIMEF_OFS 0x800000000000LL
> +#define TIMEF_INT_OFS 0x800000LL
> +
> +
> +/**
> +  Convert MySQL56 in-memory numeric time representation to on-disk representation
> +  
> +  @param       nr   Value in packed numeric time format.
> +  @param   OUT ptr  The buffer to put value at.
> +  @param       dec  Precision.
> +*/
> +void my_time_packed_to_binary(longlong nr, uchar *ptr, uint dec)
> +{
> +  DBUG_ASSERT(dec <= TIME_SECOND_PART_DIGITS);
> +  /* Make sure the stored value was previously properly rounded or truncated */
> +  DBUG_ASSERT((MY_PACKED_TIME_GET_FRAC_PART(nr) % 
> +              (int) log_10_int[TIME_SECOND_PART_DIGITS - dec]) == 0);
> +
> +  switch (dec)
> +  {
> +  case 0:
> +  default:
> +    mi_int3store(ptr, TIMEF_INT_OFS + MY_PACKED_TIME_GET_INT_PART(nr));
> +    break;
> +
> +  case 1:
> +  case 2:
> +    mi_int3store(ptr, TIMEF_INT_OFS + MY_PACKED_TIME_GET_INT_PART(nr));
> +    ptr[3]= (unsigned char) (char) (MY_PACKED_TIME_GET_FRAC_PART(nr) / 10000);
> +    break;
> +
> +  case 4:
> +  case 3:
> +    mi_int3store(ptr, TIMEF_INT_OFS + MY_PACKED_TIME_GET_INT_PART(nr));
> +    mi_int2store(ptr + 3, MY_PACKED_TIME_GET_FRAC_PART(nr) / 100);
> +    break;
> +
> +  case 5:
> +  case 6:
> +    mi_int6store(ptr, nr + TIMEF_OFS);
> +    break;
> +  }
> +}
> +
> +
> +/**
> +  Convert MySQL56 on-disk time representation to in-memory packed numeric 
> +  representation.
> +  
> +  @param   ptr  The pointer to read the value at.
> +  @param   dec  Precision.
> +  @return       Packed numeric time representation.
> +*/
> +longlong my_time_packed_from_binary(const uchar *ptr, uint dec)
> +{
> +  DBUG_ASSERT(dec <= TIME_SECOND_PART_DIGITS);
> +
> +  switch (dec)
> +  {
> +  case 0:
> +  default:
> +    {
> +      longlong intpart= mi_uint3korr(ptr) - TIMEF_INT_OFS;
> +      return MY_PACKED_TIME_MAKE_INT(intpart);
> +    }
> +  case 1:
> +  case 2:
> +    {
> +      longlong intpart= mi_uint3korr(ptr) - TIMEF_INT_OFS;
> +      int frac= (uint) ptr[3];
> +      if (intpart < 0 && frac)
> +      {
> +        /*
> +          Negative values are stored with reverse fractional part order,
> +          for binary sort compatibility.
> +
> +            Disk value  intpart frac   Time value   Memory value
> +            800000.00    0      0      00:00:00.00  0000000000.000000
> +            7FFFFF.FF   -1      255   -00:00:00.01  FFFFFFFFFF.FFD8F0
> +            7FFFFF.9D   -1      99    -00:00:00.99  FFFFFFFFFF.F0E4D0
> +            7FFFFF.00   -1      0     -00:00:01.00  FFFFFFFFFF.000000
> +            7FFFFE.FF   -1      255   -00:00:01.01  FFFFFFFFFE.FFD8F0
> +            7FFFFE.F6   -2      246   -00:00:01.10  FFFFFFFFFE.FE7960
> +
> +            Formula to convert fractional part from disk format
> +            (now stored in "frac" variable) to absolute value: "0x100 - frac".
> +            To reconstruct in-memory value, we shift
> +            to the next integer value and then substruct fractional part.
> +        */
> +        intpart++;    /* Shift to the next integer value */
> +        frac-= 0x100; /* -(0x100 - frac) */
> +      }
> +      return MY_PACKED_TIME_MAKE(intpart, frac * 10000);
> +    }
> +
> +  case 3:
> +  case 4:
> +    {
> +      longlong intpart= mi_uint3korr(ptr) - TIMEF_INT_OFS;
> +      int frac= mi_uint2korr(ptr + 3);
> +      if (intpart < 0 && frac)
> +      {
> +        /*
> +          Fix reverse fractional part order: "0x10000 - frac".
> +          See comments for FSP=1 and FSP=2 above.
> +        */
> +        intpart++;      /* Shift to the next integer value */
> +        frac-= 0x10000; /* -(0x10000-frac) */
> +      }
> +      return MY_PACKED_TIME_MAKE(intpart, frac * 100);
> +    }
> +
> +  case 5:
> +  case 6:
> +    return ((longlong) mi_uint6korr(ptr)) - TIMEF_OFS;
> +  }
> +}
> +
> +
> +/*** MySQL56 DATETIME low-level memory and disk representation routines ***/
> +
> +/*
> +    1 bit  sign            (used when on disk)
> +   17 bits year*13+month   (year 0-9999, month 0-12)
> +    5 bits day             (0-31)
> +    5 bits hour            (0-23)
> +    6 bits minute          (0-59)
> +    6 bits second          (0-59)
> +   24 bits microseconds    (0-999999)
> +
> +   Total: 64 bits = 8 bytes
> +
> +   SYYYYYYY.YYYYYYYY.YYdddddh.hhhhmmmm.mmssssss.ffffffff.ffffffff.ffffffff
> +*/
> +
> +/**
> +  Convert datetime to MySQL56 packed numeric datetime representation.
> +  @param ltime  The value to convert.
> +  @return       Packed numeric representation of ltime.
> +*/
> +longlong TIME_to_longlong_datetime_packed(const MYSQL_TIME *ltime)
> +{
> +  longlong ymd= ((ltime->year * 13 + ltime->month) << 5) | ltime->day;
> +  longlong hms= (ltime->hour << 12) | (ltime->minute << 6) | ltime->second;
> +  longlong tmp= MY_PACKED_TIME_MAKE(((ymd << 17) | hms), ltime->second_part);
> +  DBUG_ASSERT(!check_datetime_range(ltime)); /* Make sure no overflow */
> +  return ltime->neg ? -tmp : tmp;
> +}
> +
> +
> +/**
> +  Convert MySQL56 packed numeric datetime representation to MYSQL_TIME.
> +  @param OUT  ltime The datetime variable to convert to.
> +  @param      tmp   The packed numeric datetime value.
> +*/
> +void TIME_from_longlong_datetime_packed(MYSQL_TIME *ltime, longlong tmp)
> +{
> +  longlong ymd, hms;
> +  longlong ymdhms, ym;
> +  if ((ltime->neg= (tmp < 0)))
> +    tmp= -tmp;
> +
> +  ltime->second_part= MY_PACKED_TIME_GET_FRAC_PART(tmp);
> +  ymdhms= MY_PACKED_TIME_GET_INT_PART(tmp);
> +
> +  ymd= ymdhms >> 17;
> +  ym= ymd >> 5;
> +  hms= ymdhms % (1 << 17);
> +
> +  ltime->day= ymd % (1 << 5);
> +  ltime->month= ym % 13;
> +  ltime->year= ym / 13;
> +
> +  ltime->second= hms % (1 << 6);
> +  ltime->minute= (hms >> 6) % (1 << 6);
> +  ltime->hour= (hms >> 12);
> +  
> +  ltime->time_type= MYSQL_TIMESTAMP_DATETIME;
> +}
> +
> +
> +/**
> +  Calculate binary size of MySQL56 packed datetime representation.
> +  @param dec  Precision.
> +*/
> +uint my_datetime_binary_length(uint dec)
> +{
> +  DBUG_ASSERT(dec <= TIME_SECOND_PART_DIGITS);
> +  return 5 + (dec + 1) / 2;
> +}
> +
> +
> +/*
> +  On disk we store as unsigned number with DATETIMEF_INT_OFS offset,
> +  for HA_KETYPE_BINARY compatibilty purposes.
> +*/
> +#define DATETIMEF_INT_OFS 0x8000000000LL
> +
> +
> +/**
> +  Convert MySQL56 on-disk datetime representation
> +  to in-memory packed numeric representation.
> +
> +  @param ptr   The pointer to read value at.
> +  @param dec   Precision.
> +  @return      In-memory packed numeric datetime representation.
> +*/
> +longlong my_datetime_packed_from_binary(const uchar *ptr, uint dec)
> +{
> +  longlong intpart= mi_uint5korr(ptr) - DATETIMEF_INT_OFS;
> +  int frac;
> +  DBUG_ASSERT(dec <= TIME_SECOND_PART_DIGITS);
> +  switch (dec)
> +  {
> +  case 0:
> +  default:
> +    return MY_PACKED_TIME_MAKE_INT(intpart);
> +  case 1:
> +  case 2:
> +    frac= ((int) (signed char) ptr[5]) * 10000;
> +    break;
> +  case 3:
> +  case 4:
> +    frac= mi_sint2korr(ptr + 5) * 100;
> +    break;
> +  case 5:
> +  case 6:
> +    frac= mi_sint3korr(ptr + 5);
> +    break;
> +  }
> +  return MY_PACKED_TIME_MAKE(intpart, frac);
> +}
> +
> +
> +/**
> +  Store MySQL56 in-memory numeric packed datetime representation to disk.
> +
> +  @param      nr  In-memory numeric packed datetime representation.
> +  @param OUT  ptr The pointer to store at.
> +  @param      dec Precision, 1-6.
> +*/
> +void my_datetime_packed_to_binary(longlong nr, uchar *ptr, uint dec)
> +{
> +  DBUG_ASSERT(dec <= TIME_SECOND_PART_DIGITS);
> +  /* The value being stored must have been properly rounded or truncated */
> +  DBUG_ASSERT((MY_PACKED_TIME_GET_FRAC_PART(nr) %
> +              (int) log_10_int[TIME_SECOND_PART_DIGITS - dec]) == 0);
> +
> +  mi_int5store(ptr, MY_PACKED_TIME_GET_INT_PART(nr) + DATETIMEF_INT_OFS);
> +  switch (dec)
> +  {
> +  case 0:
> +  default:
> +    break;
> +  case 1:
> +  case 2:
> +    ptr[5]= (unsigned char) (char) (MY_PACKED_TIME_GET_FRAC_PART(nr) / 10000);
> +    break;
> +  case 3:
> +  case 4:
> +    mi_int2store(ptr + 5, MY_PACKED_TIME_GET_FRAC_PART(nr) / 100);
> +    break;
> +  case 5:
> +  case 6:
> +    mi_int3store(ptr + 5, MY_PACKED_TIME_GET_FRAC_PART(nr));
> +  }
> +}
> +
> +
> +/*** MySQL56 TIMESTAMP low-level memory and disk representation routines ***/
> +
> +/**
> +  Calculate on-disk size of a timestamp value.
> +
> +  @param  dec  Precision.
> +*/
> +uint my_timestamp_binary_length(uint dec)
> +{
> +  DBUG_ASSERT(dec <= TIME_SECOND_PART_DIGITS);
> +  return 4 + (dec + 1) / 2;
> +}
> +
> +
> +/**
> +  Convert MySQL56 binary timestamp representation to in-memory representation.
> +
> +  @param  OUT tm  The variable to convert to.
> +  @param      ptr The pointer to read the value from.
> +  @param      dec Precision.
> +*/
> +void my_timestamp_from_binary(struct timeval *tm, const uchar *ptr, uint dec)
> +{
> +  DBUG_ASSERT(dec <= TIME_SECOND_PART_DIGITS);
> +  tm->tv_sec= mi_uint4korr(ptr);
> +  switch (dec)
> +  {
> +    case 0:
> +    default:
> +      tm->tv_usec= 0;
> +      break;
> +    case 1:
> +    case 2:
> +      tm->tv_usec= ((int) ptr[4]) * 10000;
> +      break;
> +    case 3:
> +    case 4:
> +      tm->tv_usec= mi_sint2korr(ptr + 4) * 100;
> +      break;
> +    case 5:
> +    case 6:
> +      tm->tv_usec= mi_sint3korr(ptr + 4);
> +  }
> +}
> +
> +
> +/**
> +  Convert MySQL56 in-memory timestamp representation to on-disk representation.
> +
> +  @param        tm   The value to convert.
> +  @param  OUT   ptr  The pointer to store the value to.
> +  @param        dec  Precision.
> +*/
> +void my_timestamp_to_binary(const struct timeval *tm, uchar *ptr, uint dec)
> +{
> +  DBUG_ASSERT(dec <= TIME_SECOND_PART_DIGITS);
> +  /* Stored value must have been previously properly rounded or truncated */
> +  DBUG_ASSERT((tm->tv_usec %
> +               (int) log_10_int[TIME_SECOND_PART_DIGITS - dec]) == 0);
> +  mi_int4store(ptr, tm->tv_sec);
> +  switch (dec)
> +  {
> +    case 0:
> +    default:
> +      break;
> +    case 1:
> +    case 2:
> +      ptr[4]= (unsigned char) (char) (tm->tv_usec / 10000);
> +      break;
> +    case 3:
> +    case 4:
> +      mi_int2store(ptr + 4, tm->tv_usec / 100);
> +      break;
> +      /* Impossible second precision. Fall through */
> +    case 5:
> +    case 6:
> +      mi_int3store(ptr + 4, tm->tv_usec);
> +  }
> +}
> +
> +/****************************************/
> +
> +
>  double TIME_to_double(const MYSQL_TIME *my_time)
>  {
>    double d= (double)TIME_to_ulonglong(my_time);
> === 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.

>    return (field_type < FIELDTYPE_TEAR_FROM ?
>            field_type :
>            ((int)FIELDTYPE_TEAR_FROM) + (field_type - FIELDTYPE_TEAR_TO) - 1);
> @@ -4690,6 +4767,16 @@ String *Field_timestamp::val_str(String
>    *to++= (char) ('0'+(char) (temp));
>    *to= 0;
>    val_buffer->set_charset(&my_charset_numeric);
> +
> +  if (uint dec= decimals())

eh? We never declare variables inside if(). A question of a consistent
coding style.

> +  {
> +    ulong sec_part= (ulong) sec_part_shift(ltime.second_part, dec);
> +    char *buf= const_cast<char*>(val_buffer->ptr() + MAX_DATETIME_WIDTH);
> +    for (int i= dec; i > 0; i--, sec_part/= 10)
> +    buf[i]= (char)(sec_part % 10) + '0';
> +    buf[0]= '.';
> +    buf[dec + 1]= 0;
> +  }
>    return val_buffer;
>  }
>  
> @@ -5092,7 +5198,18 @@ int Field_temporal::store(double nr)
>  }
>  
>  
> -int Field_temporal::store(longlong nr, bool unsigned_val)
> +int Field_temporal::store_decimal(const my_decimal *d)
> +{
> +  ASSERT_COLUMN_MARKED_FOR_WRITE_OR_COMPUTED;
> +  double val;
> +  /* TODO: use decimal2string? */
> +  int err= warn_if_overflow(my_decimal2double(E_DEC_FATAL_ERROR &
> +                                            ~E_DEC_OVERFLOW, d, &val));

decimal2string is slow, decimal2double is lossy.
Use my_decimal2seconds

> +  return err | store(val);
> +}
> +
> +
> +int Field_temporal_with_date::store(longlong nr, bool unsigned_val)
>  {
>    int error;
>    MYSQL_TIME ltime;
> @@ -5229,32 +5375,16 @@ longlong Field_time::val_int(void)
>    my_charset_bin
>  */
>  
> -String *Field_time::val_str(String *val_buffer,
> -			    String *val_ptr __attribute__((unused)))
> +String *Field_time::val_str(String *str,
> +			    String *unused __attribute__((unused)))
>  {
>    ASSERT_COLUMN_MARKED_FOR_READ;
>    MYSQL_TIME ltime;
> -  long tmp=(long) sint3korr(ptr);
> -  ltime.neg= 0;
> -  if (tmp < 0)
> -  {
> -    tmp= -tmp;
> -    ltime.neg= 1;
> -  }
> -  ltime.year= ltime.month= 0;
> -  ltime.day= (uint) 0;
> -  ltime.hour= (uint) (tmp/10000);
> -  ltime.minute= (uint) (tmp/100 % 100);
> -  ltime.second= (uint) (tmp % 100);
> -  ltime.second_part= 0;
> -
> -  val_buffer->alloc(MAX_DATE_STRING_REP_LENGTH);
> -  uint length= (uint) my_time_to_str(&ltime,
> -                                     const_cast<char*>(val_buffer->ptr()), 0);
> -  val_buffer->length(length);
> -  val_buffer->set_charset(&my_charset_numeric);
> -
> -  return val_buffer;
> +  get_date(&ltime, TIME_TIME_ONLY);
> +  str->alloc(field_length + 1);
> +  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...

>  }
>  
>  
> @@ -5431,13 +5541,51 @@ void Field_time_hires::sql_type(String &
>                                  "time(%u)", dec));
>  }
>  
> -void Field_time_hires::make_field(Send_field *field)
> +void Field_time_with_dec::make_field(Send_field *field)
>  {
>    Field::make_field(field);
>    field->decimals= dec;
>  }
>  
>  /****************************************************************************
> +** time type with fsp (MySQL-5.6 version)
> +** In string context: HH:MM:SS.FFFFFF
> +** In number context: HHMMSS.FFFFFF
> +****************************************************************************/
> +
> +void Field_timef::sql_type(String &res) const
> +{
> +  if (dec == 0)
> +  {
> +    res.set_ascii(STRING_WITH_LEN("time"));
> +    return;
> +  }
> +  const CHARSET_INFO *cs= res.charset();
> +  res.length(cs->cset->snprintf(cs, (char*) res.ptr(), res.alloced_length(),
> +                               "time(%d)", dec));
> +}

Why would your mysql-5.6 types need a separate sql_type() method?
It doesn't seem to be anything that couldn't go into the base class

> +
> +int Field_timef::reset()
> +{
> +  my_time_packed_to_binary(0, ptr, dec);
> +  return 0;
> +}
> +
> +void Field_timef::store_TIME(MYSQL_TIME *ltime)
> +{
> +  my_time_trunc(ltime, decimals());
> +  longlong tmp= TIME_to_longlong_time_packed(ltime);
> +  my_time_packed_to_binary(tmp, ptr, dec);
> +}
> +
> +bool Field_timef::get_date(MYSQL_TIME *ltime, ulonglong fuzzydate)
> +{
> +  longlong tmp= my_time_packed_from_binary(ptr, dec);
> +  TIME_from_longlong_time_packed(ltime, tmp);
> +  return false;
> +}
> +
> +/****************************************************************************
>  ** year type
>  ** Save in a byte the year 0, 1901->2155
>  ** Can handle 2 byte or 4 byte years!
> @@ -9413,17 +9494,6 @@ bool Create_field::init(THD *thd, char *
>      DBUG_RETURN(TRUE);
>    }
>  
> -  switch (fld_type) {
> -  case MYSQL_TYPE_DATE:
> -  case MYSQL_TYPE_NEWDATE:
> -  case MYSQL_TYPE_TIME:
> -  case MYSQL_TYPE_DATETIME:
> -  case MYSQL_TYPE_TIMESTAMP:
> -    charset= &my_charset_numeric;
> -    flags|= BINARY_FLAG;
> -  default: break;
> -  }
> -

why?

>    DBUG_RETURN(FALSE); /* success */
>  }
>  
> === modified file 'sql/field.h'
> --- sql/field.h	2013-06-06 15:51:28 +0000
> +++ sql/field.h	2013-06-19 09:46:34 +0000
> @@ -428,6 +464,27 @@ class Field
>    virtual uint32 key_length() const { return pack_length(); }
>    virtual enum_field_types type() const =0;
>    virtual enum_field_types real_type() const { return type(); }
> +  virtual enum_field_types binlog_type() const
> +  {
> +    /*
> +      Binlog stores field->type() as type code by default.
> +      This puts MYSQL_TYPE_STRING in case of CHAR, VARCHAR, SET and ENUM,
> +      with extra data type details put into metadata.
> +
> +      We cannot store field->type() in case of temporal types with
> +      fractional seconds: TIME(n), DATETIME(n) and TIMESTAMP(n),
> +      because binlog records with MYSQL_TYPE_TIME, MYSQL_TYPE_DATETIME
> +      type codes do not have metadata.
> +      So for MySQL-5.6 temporal data types with fractional seconds we'll store
> +      real_type() type codes instead, i.e.
> +      MYSQL_TYPE_TIME2, MYSQL_TYPE_DATETIME2, MYSQL_TYPE_TIMESTAMP2,
> +      and put precision into metatada.
> +
> +      Note: perhaps binlog should eventually be modified to store
> +      real_type() instead of type() for all column types.
> +    */
> +    return type();
> +  }

Eh, I don't like it. So, to replicate with RBR between tables with
TIME columns of different precision, one needs to create tables
in MySQL 5.6? That's very lame thing to say to users :(

I'd rather fix it to work for all temporal types, both 5.6 and ours.
Perhaps, indeed, store real_type() as you write above.

But in a separate changeset. And here let's just say that it doesn't
work at all, and we always put MYSQL_TYPE_TIME/etc in the binlog.

>    inline  int cmp(const uchar *str) { return cmp(ptr,str); }
>    virtual int cmp_max(const uchar *a, const uchar *b, uint max_len)
>      { return cmp(a, b); }
> @@ -661,6 +718,16 @@ class Field
>    { return binary() ? &my_charset_bin : charset(); }
>    virtual CHARSET_INFO *sort_charset(void) const { return charset(); }
>    virtual bool has_charset(void) const { return FALSE; }
> +  /*
> +    match_collation_to_optimize_range() is to distinguish in
> +    range optimizer (see opt_range.cc) between real string types:
> +      CHAR, VARCHAR, TEXT
> +    and the other string-alike types with result_type() == STRING_RESULT:
> +      DATE, TIME, DATETIME, TIMESTAMP
> +    We need it to decide whether to test if collation of the operation
> +    matches collation of the field (needed only for real string types).
> +  */
> +  virtual bool match_collation_to_optimize_range() const { return false; }

Not needed. temporal types are not string-alike,
they have have cmp_type() == TIME_RESULT,
string types have STRING_RESULT.

>    virtual void set_charset(CHARSET_INFO *charset_arg) { }
>    virtual enum Derivation derivation(void) const
>    { return DERIVATION_IMPLICIT; }
> @@ -1345,7 +1407,67 @@ class Field_null :public Field_str {
>  };
>  
>  
> -class Field_timestamp :public Field_str {
> +class Field_temporal: public Field {
> +public:
> +  Field_temporal(uchar *ptr_arg,uint32 len_arg, uchar *null_ptr_arg,
> +                 uchar null_bit_arg, utype unireg_check_arg,
> +                 const char *field_name_arg)
> +    :Field(ptr_arg, len_arg, null_ptr_arg, null_bit_arg, unireg_check_arg,
> +               field_name_arg)
> +    { flags|= BINARY_FLAG; }
> +  Item_result result_type () const { return STRING_RESULT; }   
> +  uint32 max_display_length() { return field_length; }
> +  bool str_needs_quotes() { return TRUE; }
> +  enum Derivation derivation(void) const { return DERIVATION_NUMERIC; }
> +  uint repertoire(void) const { return MY_REPERTOIRE_NUMERIC; }
> +  CHARSET_INFO *charset(void) const { return &my_charset_numeric; }
> +  const CHARSET_INFO *sort_charset(void) const { return &my_charset_bin; }
> +  bool binary() const { return true; }
> +  enum Item_result cmp_type () const { return TIME_RESULT; }
> +  uint is_equal(Create_field *new_field);
> +  bool eq_def(Field *field)
> +  {
> +    return (Field::eq_def(field) && decimals() == field->decimals());
> +  }
> +  int  store_decimal(const my_decimal *);
> +  my_decimal *val_decimal(my_decimal*);
> +  void set_warnings(MYSQL_ERROR::enum_warning_level trunc_level,
> +                    const ErrConv *str, int was_cut, timestamp_type ts_type);
> +  double pos_in_interval(Field *min, Field *max)
> +  {
> +    return pos_in_interval_val_str(min, max, 0);

Really? This would first convert a temporal value to a string,
and then use a quite complicated procedure of placing a string
in the interval. Wouldn't it be much cheaper to use
pos_in_interval_val_real() ?

> +  }
> +};
> +
> +
> +/**
> +  Abstract class for:
> +  - DATE
> +  - DATETIME
> +  - DATETIME(1..6)
> +  - DATETIME(0..6) - MySQL56 version
> +*/
> +class Field_temporal_with_date: public Field_temporal {
> +protected:
> +  int store_TIME_with_warning(MYSQL_TIME *ltime, const ErrConv *str,
> +                              int was_cut, int have_smth_to_conv);

Eh? So you'll have *three* different store_TIME_with_warning() methods?
I didn't like my version, because I had *two*,
there should've been only one.

> +  virtual void store_TIME(MYSQL_TIME *ltime) = 0;
> +public:
> +  Field_temporal_with_date(uchar *ptr_arg, uint32 len_arg,
> +                           uchar *null_ptr_arg, uchar null_bit_arg,
> +                           utype unireg_check_arg,
> +                           const char *field_name_arg)
> +    :Field_temporal(ptr_arg, len_arg, null_ptr_arg, null_bit_arg,
> +                    unireg_check_arg, field_name_arg)
> +    {}
> +  int  store(const char *to, uint length, CHARSET_INFO *charset);
> +  int  store(double nr);
> +  int  store(longlong nr, bool unsigned_val);
> +  int  store_time_dec(MYSQL_TIME *ltime, uint dec);
> +};
> +
> +
> +class Field_timestamp :public Field_temporal {
>  protected:
>    int store_TIME_with_warning(THD *, MYSQL_TIME *, const ErrConv *,
>                                bool, bool);
> === modified file 'sql/opt_range.cc'
> --- sql/opt_range.cc	2013-06-06 19:32:29 +0000
> +++ sql/opt_range.cc	2013-06-18 10:35:35 +0000
> @@ -8014,10 +8014,10 @@ get_mm_leaf(RANGE_OPT_PARAM *param, COND
>  
>    */
>    if (field->result_type() == STRING_RESULT &&

this and below should probably use cmp_type, not result_type

> -      ((Field_str*) field)->match_collation_to_optimize_range() &&
> +      field->match_collation_to_optimize_range() &&
>        value->result_type() == STRING_RESULT &&
>        key_part->image_type == Field::itRAW &&
> -      ((Field_str*)field)->charset() != conf_func->compare_collation() &&
> +      field->charset() != conf_func->compare_collation() &&
>        !(conf_func->compare_collation()->state & MY_CS_BINSORT &&
>          (type == Item_func::EQUAL_FUNC || type == Item_func::EQ_FUNC)))
>      goto end;
> === modified file 'sql/sql_time.h'
> --- sql/sql_time.h	2012-08-31 12:15:52 +0000
> +++ sql/sql_time.h	2013-06-18 10:35:36 +0000
> @@ -110,4 +110,23 @@ extern DATE_TIME_FORMAT global_time_form
>  extern KNOWN_DATE_TIME_FORMAT known_date_time_formats[];
>  extern LEX_STRING interval_type_to_name[];
>  
> +/* Date/time rounding and truncation functions */
> +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];
> +}
> +inline void my_time_trunc(MYSQL_TIME *ltime, uint decimals)
> +{
> +  ltime->second_part-= my_time_fraction_remainder(ltime->second_part, decimals);
> +}

good idea, but
1. put it in my_time.h instead of sec_part_truncate()
2. change item_timefunc.cc (and other files too) to use it 
(this will remove all uses of sec_part_truncate())

> +inline void my_datetime_trunc(MYSQL_TIME *ltime, uint decimals)
> +{
> +  return my_time_trunc(ltime, decimals);
> +}
> +inline void my_timeval_trunc(struct timeval *tv, uint decimals)
> +{
> +  tv->tv_usec-= my_time_fraction_remainder(tv->tv_usec, decimals);
> +}

please move all these four functions to my_time.h
(although, I'd delete my_datetime_trunc() completely)

> +
>  #endif /* SQL_TIME_INCLUDED */

Regards,
Sergei


Follow ups

References