Hi Sergei, Thanks for review. This is a new version of the patch. I tried to address most of your suggestions. Please see comments inline. (I wrote some of them in a earlier letter, but will repeat just in case) Thanks. On 06/26/2013 11:11 PM, Sergei Golubchik wrote:
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
Good idea. Moved to sql/compat56.cc and sql/compat56.h
+ 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.
Done.
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
Fixed.
+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/
Done.
+ +/* + 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.
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. 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. This should probably be fixed separately. Perhaps, we need a separate enum consisting of real types, like this: enum enum_field_real_types { MYSQL_REAL_TYPE_DATE, MYSQL_REAL_TYPE_NEWDATE, MYSQL_REAL_TYPE_TIME, MYSQL_REAL_TYPE_TIME2 ... }; Then have enum_field_types defined using enum_field_real_types, like this: enum enum_field_types { MYSQL_TYPE_DATE= MYSQL_REAL_TYPE_DATE, MYSQL_TYPE_TIME= MYSQL_REAL_TYPE_TIME ... // No codes like MYSQL_REAL_TYPE_NEWDATE or MYSQL_REAL_TYPE_TIME2 here } Then, all the functions in should use either enum_field_real_types or enum_field_types, depending on what they actually need, For example, the mentioned "value is not handled" should be gone, because these functions need type (not real type!).
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.
Fixed.
+ { + 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
Done.
+ 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(<ime, - const_cast<char*>(val_buffer->ptr()), 0); - val_buffer->length(length); - val_buffer->set_charset(&my_charset_numeric); - - return val_buffer; + get_date(<ime, TIME_TIME_ONLY); + str->alloc(field_length + 1); + str->length(my_time_to_str(<ime, 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).
}
@@ -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
Moved to the parent classes.
+ +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?
Because Field_temporal does not need a charset any more, as it is not a child of Field_str now.
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.
Fixed the comment.
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.
It seems we agreed to try to do something around this in a separate changeset.
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() ?
Changed to pos_in_interval_val_real(). Note, mysql-test/r/statistics.result has changed slightly. But I think it's Ok.
+ } +}; + + +/** + 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.
We decided to take a look into this later.
+ 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
We agreed to try to do something around this in a separate changeset.
- ((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())
Done.
+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)
Done.
+ #endif /* SQL_TIME_INCLUDED */
Regards, Sergei